Skip to content

Commit

Permalink
fix(components/ag-grid): improve row selector performance (#2691)
Browse files Browse the repository at this point in the history 
* fix(components/ag-grid): improve row selector performance

* Simplify changes.
  • Loading branch information
@johnhwhite
johnhwhite committed Sep 9, 2024
1 parent 0de9fff commit e1d391b
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 158 deletions.
4 changes: 2 additions & 2 deletions ...-grid/cell-renderers/cell-renderer-row-selector/cell-renderer-row-selector.component.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
[tabindex]="0"
[labelHidden]="true"
[labelText]="(label | async) || ''"
[(ngModel)]="checked"
(change)="updateRow()"
[checked]="(checked | async) || false"
(change)="updateRow(!!$event.checked)"
/>
174 changes: 79 additions & 95 deletions ...id/cell-renderers/cell-renderer-row-selector/cell-renderer-row-selector.component.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import {
ComponentFixture,
TestBed,
fakeAsync,
tick,
} from '@angular/core/testing';
import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { expect, expectAsync } from '@skyux-sdk/testing';
// eslint-disable-next-line @nx/enforce-module-boundaries
import { SkyCheckboxFixture } from '@skyux/forms/testing';
import { SkyCheckboxHarness } from '@skyux/forms/testing';

import {
Beans,
Expand All @@ -24,9 +19,6 @@ import { SkyCellClass } from '../../types/cell-class';
import { SkyAgGridCellRendererRowSelectorComponent } from './cell-renderer-row-selector.component';

describe('SkyAgGridCellRendererRowSelectorComponent', () => {
// We've had some issue with grid rendering causing the specs to timeout in IE. Extending it slightly to help.
jasmine.DEFAULT_TIMEOUT_INTERVAL = 7500;

let rowSelectorCellFixture: ComponentFixture<SkyAgGridCellRendererRowSelectorComponent>;
let rowSelectorCellComponent: SkyAgGridCellRendererRowSelectorComponent;
let rowSelectorCellNativeElement: HTMLElement;
Expand All @@ -43,13 +35,18 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
);
rowSelectorCellNativeElement = rowSelectorCellFixture.nativeElement;
rowSelectorCellComponent = rowSelectorCellFixture.componentInstance;
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
spyOn(rowNode, 'isSelected').and.returnValue(false);
spyOn(rowNode, 'setSelected');
rowNode.data = {};
cellRendererParams = {
colDef: {
field: dataField,
cellRendererParams: {
label: 'Select',
},
},
node: rowNode,
};
});

Expand All @@ -67,54 +64,40 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {

describe('agInit', () => {
it(`initializes the SkyuxCheckboxGridCellComponent
properties and sets the checkbox to the value of the column field provided`, fakeAsync(() => {
properties and sets the checkbox to the value of the column field provided`, async () => {
const checked = true;
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
cellRendererParams.value = checked;
cellRendererParams.node = rowNode;
spyOn(rowNode, 'setSelected');

const checkbox = new SkyCheckboxFixture(
const checkbox = await TestbedHarnessEnvironment.loader(
rowSelectorCellFixture,
'row-checkbox',
);
).getHarness(SkyCheckboxHarness.with({ dataSkyId: 'row-checkbox' }));

expect(rowSelectorCellComponent.checked).toBeUndefined();
expect(checkbox.selected).toBe(false);
expect(await checkbox.isChecked()).toBe(false);
expect(rowSelectorCellComponent.rowNode).toBeUndefined();
expect(() => rowSelectorCellComponent.updateRow(checked)).not.toThrow();

rowSelectorCellComponent.agInit(
cellRendererParams as ICellRendererParams,
);

rowSelectorCellFixture.detectChanges();
tick();
rowSelectorCellFixture.detectChanges();
await rowSelectorCellFixture.whenStable();

expect(rowSelectorCellComponent.checked).toBe(checked);
expect(checkbox.selected).toBe(true);
expect(rowSelectorCellComponent.rowNode).toEqual(rowNode);
expect(await checkbox.isChecked()).toBe(true);
expect(rowSelectorCellComponent.rowNode).toEqual(cellRendererParams.node);
expect(
rowSelectorCellComponent.rowNode?.setSelected,
).toHaveBeenCalledWith(true);
}));
});

it(`initializes the SkyuxCheckboxGridCellComponent properties and sets the checkbox to the node's selected
value since no column field provided`, fakeAsync(() => {
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
value since no column field provided`, async () => {
cellRendererParams.value = true;
cellRendererParams.node = rowNode;
(cellRendererParams.colDef as ColDef).field = undefined;
spyOn(rowNode, 'setSelected');

const checkbox = new SkyCheckboxFixture(
const checkbox = await TestbedHarnessEnvironment.loader(
rowSelectorCellFixture,
'row-checkbox',
);

expect(rowSelectorCellComponent.checked).toBeUndefined();
expect(checkbox.selected).toBe(false);
expect(rowSelectorCellComponent.rowNode).toBeUndefined();
).getHarness(SkyCheckboxHarness.with({ dataSkyId: 'row-checkbox' }));

rowSelectorCellComponent.agInit({
...cellRendererParams,
Expand All @@ -126,81 +109,68 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
} as ICellRendererParams);

rowSelectorCellFixture.detectChanges();
tick();
rowSelectorCellFixture.detectChanges();
await rowSelectorCellFixture.whenStable();

expect(rowSelectorCellComponent.checked).toBe(false);
expect(checkbox.selected).toBe(false);
expect(rowSelectorCellComponent.rowNode).toEqual(rowNode);
expect(await checkbox.isChecked()).toBe(false);
expect(rowSelectorCellComponent.rowNode).toEqual(cellRendererParams.node);
expect(
rowSelectorCellComponent.rowNode?.setSelected,
).not.toHaveBeenCalled();
}));
});
});

describe('updateRow', () => {
it(`should set the rowNode selected property and the row data's column-defined field property
to the component's checked property value if column field provided`, () => {
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
rowNode.data = {};
cellRendererParams.value = true;
cellRendererParams.node = rowNode;

spyOn(rowNode, 'setSelected');
to the component's checked property value if column field provided`, async () => {
cellRendererParams.value = false;

rowSelectorCellFixture.detectChanges();
rowSelectorCellComponent.agInit(
cellRendererParams as ICellRendererParams,
);

rowSelectorCellComponent.updateRow();
rowSelectorCellComponent.updateRow(true);

expect(
rowSelectorCellComponent.rowNode?.setSelected,
).toHaveBeenCalledWith(true);
).toHaveBeenCalledWith(true, undefined, 'checkboxSelected');
expect(rowSelectorCellComponent.rowNode?.data[dataField]).toBe(true);
});

it(`should set the rowNode selected property to the component's checked property value if no column field provided`, () => {
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
rowNode.data = {};
cellRendererParams.node = rowNode;
cellRendererParams.colDef = {
field: undefined,
};
spyOn(rowNode, 'isSelected').and.returnValues(true, false);
(cellRendererParams.node?.isSelected as jasmine.Spy).and.returnValues(
true,
false,
);

rowSelectorCellFixture.detectChanges();
rowSelectorCellComponent.agInit(
cellRendererParams as ICellRendererParams,
);

spyOn(rowNode, 'setSelected');

rowSelectorCellComponent.updateRow();
rowSelectorCellComponent.updateRow(true);

expect(
rowSelectorCellComponent.rowNode?.setSelected,
).toHaveBeenCalledWith(true);
).toHaveBeenCalledWith(true, undefined, 'checkboxSelected');
});

it(`should not set the rowNode selected property to the component's checked property value if no column field provided and value is not changed`, () => {
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
rowNode.data = {};
cellRendererParams.node = rowNode;
cellRendererParams.colDef = {
field: undefined,
};
spyOn(rowNode, 'isSelected').and.returnValues(true, true);
(cellRendererParams.node?.isSelected as jasmine.Spy).and.returnValues(
true,
true,
);

rowSelectorCellFixture.detectChanges();
rowSelectorCellComponent.agInit(
cellRendererParams as ICellRendererParams,
);

spyOn(rowNode, 'setSelected');

rowSelectorCellComponent.updateRow();
rowSelectorCellComponent.updateRow(true);

expect(
rowSelectorCellComponent.rowNode?.setSelected,
Expand All @@ -210,23 +180,22 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {

describe('refresh', () => {
it('returns true', () => {
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
expect(
rowSelectorCellComponent.refresh({
node: {
isSelected: () => true,
} as unknown as RowNode,
node: rowNode,
} as unknown as ICellRendererParams),
).toBe(true);
});
});

describe('row selection', () => {
function testRowSelected(
async function testRowSelected(
colDefinition: ColDef | undefined,
isSelectedValues: boolean[],
dataPropertySet = false,
selectable = true,
): void {
): Promise<void> {
let rowClickListener: ((event: RowClickedEvent) => void) | undefined;
const rowNode = new RowNode({ frameworkOverrides: {} } as Beans);
rowNode.data = {};
Expand All @@ -238,7 +207,6 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
rowPinned: undefined,
context: undefined,
api: undefined,
columnApi: {} as never,
type: undefined,
};

Expand All @@ -261,36 +229,35 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {

rowSelectorCellFixture.detectChanges();

const checkbox = new SkyCheckboxFixture(
const checkbox = await TestbedHarnessEnvironment.loader(
rowSelectorCellFixture,
'row-checkbox',
);
).getHarness(SkyCheckboxHarness.with({ dataSkyId: 'row-checkbox' }));

rowSelectorCellComponent.agInit(
cellRendererParams as ICellRendererParams,
);
rowSelectorCellFixture.detectChanges();

expect(rowSelectorCellComponent.checked).toBeFalsy();
expect(checkbox.selected).toBe(false);
expect(checkbox.disabled).toBe(!selectable);
expect(await checkbox.isChecked()).toBeFalsy();
expect(await checkbox.isDisabled()).toBe(!selectable);

// trigger the rowClickEventListener
if (rowClickListener && selectable) {
rowClickListener(rowClickedEvent as RowClickedEvent);
}

rowSelectorCellFixture.detectChanges();
tick();
rowSelectorCellFixture.detectChanges();
await rowSelectorCellFixture.whenStable();

if (selectable) {
expect(rowNode.addEventListener).toHaveBeenCalledWith(
RowNode.EVENT_ROW_SELECTED,
'rowSelected',
jasmine.any(Function),
);
expect(rowSelectorCellComponent.checked).toBe(true);
expect(checkbox.selected).toBe(true);
expect(await checkbox.isChecked()).toBe(true);
expect(await checkbox.isDisabled()).toBe(false);
} else {
expect(await checkbox.isDisabled()).toBe(true);
}

if (dataPropertySet) {
Expand All @@ -302,24 +269,24 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
}

it(`should set the checkbox's selected value and the row data's column-defined field property
to the component's checked property value if the data field is provided`, fakeAsync(() => {
testRowSelected(cellRendererParams.colDef, [true, true], true);
}));
to the component's checked property value if the data field is provided`, async () => {
await testRowSelected(cellRendererParams.colDef, [true, true], true);
});

it(`should set the checkbox's selected value to the component's checked property value if the data field is provided or the default is used`, fakeAsync(() => {
it(`should set the checkbox's selected value to the component's checked property value if the data field is provided or the default is used`, async () => {
const columnWithoutDataField = {};
testRowSelected(columnWithoutDataField, [false, true, true]);
}));
await testRowSelected(columnWithoutDataField, [false, true, true]);
});

it(`should disable the checkbox`, fakeAsync(() => {
it(`should disable the checkbox`, async () => {
const columnWithoutDataField = {};
testRowSelected(
await testRowSelected(
columnWithoutDataField,
[false, false, false],
false,
false,
);
}));
});
});

it('should pass accessibility', async () => {
Expand All @@ -337,4 +304,21 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {

await expectAsync(rowSelectorCellNativeElement).toBeAccessible();
});

it('should pass accessibility when collapsed', async () => {
(cellRendererParams.node as RowNode).rowIndex = null;
rowSelectorCellComponent.agInit({
...cellRendererParams,
colDef: {
cellRendererParams: {
label: (): Observable<string> => of('Select'),
},
},
} as ICellRendererParams);

rowSelectorCellFixture.detectChanges();
await rowSelectorCellFixture.whenStable();

await expectAsync(rowSelectorCellNativeElement).toBeAccessible();
});
});
Loading

0 comments on commit e1d391b

Please sign in to comment.