Skip to content

Commit

Permalink
fix(components/ag-grid): improve row selector performance (#2691) (#2710
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 1da3110 commit 34ee162
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 133 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)"
/>
169 changes: 95 additions & 74 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,19 +1,16 @@
import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
import {
ComponentFixture,
TestBed,
fakeAsync,
tick,
} from '@angular/core/testing';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { expect, expectAsync } from '@skyux-sdk/testing';
import { SkyCheckboxHarness } from '@skyux/forms/testing';

import {
BeanCollection,
ColDef,
ICellRendererParams,
RowClickedEvent,
RowNode,
} from 'ag-grid-community';
import { AgRowNodeEventListener } from 'ag-grid-community/dist/types/core/interfaces/iRowNode';
import { Observable, of } from 'rxjs';

import { SkyAgGridFixtureComponent } from '../../fixtures/ag-grid.component.fixture';
Expand All @@ -23,9 +20,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 @@ -42,13 +36,18 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
);
rowSelectorCellNativeElement = rowSelectorCellFixture.nativeElement;
rowSelectorCellComponent = rowSelectorCellFixture.componentInstance;
const rowNode = new RowNode({ frameworkOverrides: {} } as BeanCollection);
spyOn(rowNode, 'isSelected').and.returnValue(false);
spyOn(rowNode, 'setSelected');
rowNode.data = {};
cellRendererParams = {
colDef: {
field: dataField,
cellRendererParams: {
label: 'Select',
},
},
node: rowNode,
};
});

Expand All @@ -66,41 +65,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 BeanCollection);
cellRendererParams.value = checked;
cellRendererParams.node = rowNode;
spyOn(rowNode, 'setSelected');

expect(rowSelectorCellComponent.checked).toBeUndefined();
const checkbox = await TestbedHarnessEnvironment.loader(
rowSelectorCellFixture,
).getHarness(SkyCheckboxHarness.with({ dataSkyId: 'row-checkbox' }));

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(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 BeanCollection);
value since no column field provided`, async () => {
cellRendererParams.value = true;
cellRendererParams.node = rowNode;
(cellRendererParams.colDef as ColDef).field = undefined;
spyOn(rowNode, 'setSelected');

expect(rowSelectorCellComponent.checked).toBeUndefined();
expect(rowSelectorCellComponent.rowNode).toBeUndefined();
const checkbox = await TestbedHarnessEnvironment.loader(
rowSelectorCellFixture,
).getHarness(SkyCheckboxHarness.with({ dataSkyId: 'row-checkbox' }));

rowSelectorCellComponent.agInit({
...cellRendererParams,
Expand All @@ -112,80 +110,68 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
} as ICellRendererParams);

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

expect(rowSelectorCellComponent.checked).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 BeanCollection);
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 BeanCollection);
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 BeanCollection);
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 @@ -195,11 +181,10 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {

describe('refresh', () => {
it('returns true', () => {
const rowNode = new RowNode({ frameworkOverrides: {} } as BeanCollection);
expect(
rowSelectorCellComponent.refresh({
node: {
isSelected: () => true,
} as unknown as RowNode,
node: rowNode,
} as unknown as ICellRendererParams),
).toBe(true);
});
Expand All @@ -212,45 +197,64 @@ describe('SkyAgGridCellRendererRowSelectorComponent', () => {
dataPropertySet = false,
selectable = true,
): Promise<void> {
let rowClickListener: ((event: RowClickedEvent) => void) | undefined;
const rowNode = new RowNode({ frameworkOverrides: {} } as BeanCollection);
rowNode.data = {};
rowNode.selectable = selectable;
const rowClickedEvent: Partial<RowClickedEvent> = {
node: rowNode,
data: undefined,
rowIndex: undefined,
rowPinned: undefined,
context: undefined,
api: undefined,
type: undefined,
};

cellRendererParams.value = false;
cellRendererParams.colDef = colDefinition;
cellRendererParams.node = rowNode;

spyOn(rowNode, 'setSelected');
spyOn(rowNode, 'isSelected').and.returnValues(...isSelectedValues);
spyOn(rowNode, 'addEventListener').and.stub();
spyOn(rowNode, 'addEventListener').and.callFake(
(event: any, listener: AgRowNodeEventListener<any>): void => {
// set event listener
rowClickListener = listener;
},
);

rowSelectorCellFixture.detectChanges();

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

rowSelectorCellComponent.agInit(
cellRendererParams as ICellRendererParams,
);
rowSelectorCellFixture.detectChanges();
expect(rowNode.addEventListener).toHaveBeenCalledWith(
'rowSelected',
jasmine.any(Function),
);

const loader = TestbedHarnessEnvironment.loader(rowSelectorCellFixture);
const harness = await loader.getHarness(
SkyCheckboxHarness.with({ dataSkyId: 'row-checkbox' }),
);
expect(await harness.isChecked()).toBe(false);
expect(await harness.isDisabled()).toBe(!selectable);
if (selectable) {
await harness.check();
expect(await checkbox.isChecked()).toBeFalsy();
expect(await checkbox.isDisabled()).toBe(!selectable);

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

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

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

if (dataPropertySet) {
Expand Down Expand Up @@ -297,4 +301,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 34ee162

Please sign in to comment.