Skip to content

Commit 0ab9f41

Browse files
ghiscodingghiscoding
authored
perf(common): improve Select All row selections toggling on large dataset by 9x faster (#2785)
* perf(common): improve Select All row selections toggling on large dataset by 9x faster
1 parent 8581275 commit 0ab9f41

11 files changed

Lines changed: 243 additions & 31 deletions

File tree

‎demos/aurelia/test/cypress/e2e/example32.cy.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ describe('Example 32 - Columns Resize by Content', () => {
115115
cy.get('#filter-checkbox-selectall-container input[type=checkbox]').click({ force: true });
116116

117117
cy.window().then((win) => {
118-
expect(win.console.log).to.have.callCount(3);
118+
const selectedIdsCalls = (win.console.log as any).getCalls().filter((call: any) => call.args[0] === 'Selected Ids:');
119119
expect(win.console.log).to.be.calledWith('Selected Ids:', expectedRowIds);
120+
expect(selectedIdsCalls.some((call: any) => call.args[1]?.length === 401)).to.be.true;
120121
});
121122
});
122123

‎demos/react/test/cypress/e2e/example32.cy.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ describe('Example 32 - Columns Resize by Content', () => {
115115
cy.get('#filter-checkbox-selectall-container input[type=checkbox]').click({ force: true });
116116

117117
cy.window().then((win) => {
118-
expect(win.console.log).to.have.callCount(3);
118+
const selectedIdsCalls = (win.console.log as any).getCalls().filter((call: any) => call.args[0] === 'Selected Ids:');
119119
expect(win.console.log).to.be.calledWith('Selected Ids:', expectedRowIds);
120+
expect(selectedIdsCalls.some((call: any) => call.args[1]?.length === 401)).to.be.true;
120121
});
121122
});
122123

‎demos/vue/test/cypress/e2e/example32.cy.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ describe('Example 32 - Columns Resize by Content', () => {
115115
cy.get('#filter-checkbox-selectall-container input[type=checkbox]').click({ force: true });
116116

117117
cy.window().then((win) => {
118-
expect(win.console.log).to.have.callCount(3);
118+
const selectedIdsCalls = (win.console.log as any).getCalls().filter((call: any) => call.args[0] === 'Selected Ids:');
119119
expect(win.console.log).to.be.calledWith('Selected Ids:', expectedRowIds);
120+
expect(selectedIdsCalls.some((call: any) => call.args[1]?.length === 401)).to.be.true;
120121
});
121122
});
122123

‎frameworks/angular-slickgrid/test/cypress/e2e/example32.cy.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ describe('Example 32 - Columns Resize by Content', () => {
115115
cy.get('#filter-checkbox-selectall-container input[type=checkbox]').click({ force: true });
116116

117117
cy.window().then((win) => {
118-
expect(win.console.log).to.have.callCount(3);
118+
const selectedIdsCalls = (win.console.log as any).getCalls().filter((call: any) => call.args[0] === 'Selected Ids:');
119119
expect(win.console.log).to.be.calledWith('Selected Ids:', expectedRowIds);
120+
expect(selectedIdsCalls.some((call: any) => call.args[1]?.length === 401)).to.be.true;
120121
});
121122
});
122123

‎packages/common/src/core/__tests__/slickDataView.spec.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2197,6 +2197,85 @@ describe('SlickDatView core file', () => {
21972197
]);
21982198
});
21992199

2200+
it('should report all filtered selected IDs when selection changes through the grid', () => {
2201+
const columns = [
2202+
{ id: 'name', field: 'name', name: 'Name' },
2203+
{ id: 'age', field: 'age', name: 'Age' },
2204+
];
2205+
const gridOptions = { enableCellNavigation: true, multiSelect: true, devMode: { ownerNodeIndex: 0 } } as GridOption;
2206+
dv = new SlickDataView({});
2207+
const grid = new SlickGrid('#myGrid', dv, columns, gridOptions);
2208+
const onSelectedRowIdsSpy = vi.spyOn(dv.onSelectedRowIdsChanged, 'notify');
2209+
grid.setSelectionModel(new SlickHybridSelectionModel({ selectActiveRow: false, selectionType: 'row' }));
2210+
dv.setItems(items);
2211+
dv.setPagingOptions({ dataView: dv, pageNum: 0, pageSize: 4 });
2212+
dv.syncGridSelection(grid, false, true);
2213+
dv.setSelectedIds([3, 4, 8], { isRowBeingAdded: true, applyRowSelectionToGrid: true });
2214+
onSelectedRowIdsSpy.mockClear();
2215+
2216+
grid.setSelectedRows([0, 1, 2]);
2217+
2218+
expect(onSelectedRowIdsSpy).toHaveBeenLastCalledWith(expect.objectContaining({ filteredIds: [4, 3, 1, 8] }), expect.anything(), dv);
2219+
});
2220+
2221+
it('should normalize an existing unsorted selection when the same IDs are selected again', () => {
2222+
const columns = [
2223+
{ id: 'name', field: 'name', name: 'Name' },
2224+
{ id: 'age', field: 'age', name: 'Age' },
2225+
];
2226+
const gridOptions = { enableCellNavigation: true, multiSelect: true, devMode: { ownerNodeIndex: 0 } } as GridOption;
2227+
dv = new SlickDataView({});
2228+
const grid = new SlickGrid('#myGrid', dv, columns, gridOptions);
2229+
grid.setSelectionModel(new SlickHybridSelectionModel({ selectActiveRow: false, selectionType: 'row' }));
2230+
dv.setItems(items);
2231+
grid.setSelectedRows([0, 1]);
2232+
dv.syncGridSelection(grid, false, true);
2233+
2234+
dv.setSelectedIds([3, 4], { isRowBeingAdded: true, applyRowSelectionToGrid: false });
2235+
2236+
expect(dv.getAllSelectedIds()).toEqual([3, 4]);
2237+
});
2238+
2239+
it('should reuse pending filtered IDs for bulk grid selection', () => {
2240+
const columns = [
2241+
{ id: 'name', field: 'name', name: 'Name' },
2242+
{ id: 'age', field: 'age', name: 'Age' },
2243+
];
2244+
const gridOptions = { enableCellNavigation: true, multiSelect: true, devMode: { ownerNodeIndex: 0 } } as GridOption;
2245+
dv = new SlickDataView({});
2246+
const grid = new SlickGrid('#myGrid', dv, columns, gridOptions);
2247+
const onSelectedRowIdsSpy = vi.spyOn(dv.onSelectedRowIdsChanged, 'notify');
2248+
grid.setSelectionModel(new SlickHybridSelectionModel({ selectActiveRow: false, selectionType: 'row' }));
2249+
dv.setItems(items);
2250+
dv.setPagingOptions({ dataView: dv, pageNum: 0, pageSize: 4 });
2251+
dv.syncGridSelection(grid, false, true);
2252+
dv.setSelectedIds([3, 4, 8], { isRowBeingAdded: true, shouldTriggerEvent: false, applyRowSelectionToGrid: false });
2253+
2254+
grid.setSelectedRows([0, 1], 'click.selectAll');
2255+
2256+
expect(onSelectedRowIdsSpy).toHaveBeenLastCalledWith(expect.objectContaining({ filteredIds: [3, 4, 8] }), expect.anything(), dv);
2257+
});
2258+
2259+
it('should ignore pending filtered IDs when bulk grid selection changes the selected IDs', () => {
2260+
const columns = [
2261+
{ id: 'name', field: 'name', name: 'Name' },
2262+
{ id: 'age', field: 'age', name: 'Age' },
2263+
];
2264+
const gridOptions = { enableCellNavigation: true, multiSelect: true, devMode: { ownerNodeIndex: 0 } } as GridOption;
2265+
dv = new SlickDataView({});
2266+
const grid = new SlickGrid('#myGrid', dv, columns, gridOptions);
2267+
const onSelectedRowIdsSpy = vi.spyOn(dv.onSelectedRowIdsChanged, 'notify');
2268+
grid.setSelectionModel(new SlickHybridSelectionModel({ selectActiveRow: false, selectionType: 'row' }));
2269+
dv.setItems(items);
2270+
dv.setPagingOptions({ dataView: dv, pageNum: 0, pageSize: 4 });
2271+
dv.syncGridSelection(grid, false, true);
2272+
dv.setSelectedIds([8], { isRowBeingAdded: true, shouldTriggerEvent: false, applyRowSelectionToGrid: false });
2273+
2274+
grid.setSelectedRows([0, 1], 'click.selectAll');
2275+
2276+
expect(onSelectedRowIdsSpy).toHaveBeenLastCalledWith(expect.objectContaining({ filteredIds: [4, 3, 8] }), expect.anything(), dv);
2277+
});
2278+
22002279
it('should not expect row selections to be preserved when using "multiSelect:false" and setSelectedIds() even when either preseve is enabled ("preserveHidden" or "preserveHiddenOnSelectionChange")', () => {
22012280
const columns = [
22022281
{ id: 'name', field: 'name', name: 'Name' },

‎packages/common/src/core/__tests__/slickGrid.spec.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,15 @@ class TestGrid extends SlickGrid<any, Column> {
3030
public callGetFormatter(row: number, column: Column) {
3131
return this.getFormatter(row, column);
3232
}
33+
public callRowsToRanges(rows: number[], compactRows = false) {
34+
return this.rowsToRanges(rows, compactRows);
35+
}
36+
public callAppendSelectedCellHtml(row: number, cell: number) {
37+
const divRow = document.createElement('div');
38+
this.selectedRanges = [new SlickRange(row, cell)];
39+
this.appendCellHtml(divRow, row, cell, 1, 1, null, this.getDataItem(row));
40+
return divRow.firstElementChild as HTMLDivElement;
41+
}
3342
public setCurrentEditorNull() {
3443
(this as any).currentEditor = null;
3544
}
@@ -969,6 +978,38 @@ describe('SlickGrid core file', () => {
969978
expect(secondRowItemCell.classList.contains('selected')).toBeTruthy();
970979
});
971980

981+
it('should compact contiguous row ranges when Select All is clicked', () => {
982+
const rowSelectionModel = new SlickHybridSelectionModel({ selectionType: 'row' });
983+
const setRangeSpy = vi.spyOn(rowSelectionModel, 'setSelectedRanges');
984+
985+
grid = new SlickGrid<any, Column>(container, data, columns, defaultOptions);
986+
grid.setSelectionModel(rowSelectionModel);
987+
vi.spyOn(grid.getEditorLock(), 'isActive').mockReturnValueOnce(false);
988+
989+
grid.setSelectedRows([0, 1], 'click.selectAll');
990+
991+
expect(setRangeSpy).toHaveBeenCalledWith([new SlickRange(0, 0, 1, 0)], 'click.selectAll');
992+
});
993+
994+
it('should compact non-contiguous ascending rows and allow an empty selection', () => {
995+
grid = new TestGrid(container, data, columns, defaultOptions);
996+
997+
expect((grid as TestGrid).callRowsToRanges([0, 2], true)).toEqual([new SlickRange(0, 0, 0, 0), new SlickRange(2, 0, 2, 0)]);
998+
expect((grid as TestGrid).callRowsToRanges([], true)).toEqual([]);
999+
});
1000+
1001+
it('should retain legacy selection order when Select All receives non-ascending rows', () => {
1002+
const rowSelectionModel = new SlickHybridSelectionModel({ selectionType: 'row' });
1003+
1004+
grid = new SlickGrid<any, Column>(container, data, columns, defaultOptions);
1005+
grid.setSelectionModel(rowSelectionModel);
1006+
vi.spyOn(grid.getEditorLock(), 'isActive').mockReturnValueOnce(false);
1007+
1008+
grid.setSelectedRows([2, 1], 'click.selectAll');
1009+
1010+
expect(grid.getSelectedRows()).toEqual([1, 2]);
1011+
});
1012+
9721013
it('should select rows when the last column is hidden', () => {
9731014
const columnsWithHiddenLastColumn = [
9741015
{ id: 'firstName', field: 'firstName', name: 'First Name' },
@@ -984,6 +1025,24 @@ describe('SlickGrid core file', () => {
9841025
expect(grid.getSelectedRows()).toEqual([1]);
9851026
});
9861027

1028+
it('should add selected CSS when rendering a selected cell without a CSS hash', () => {
1029+
grid = new TestGrid(container, data, columns, defaultOptions);
1030+
1031+
const cell = (grid as TestGrid).callAppendSelectedCellHtml(1, 0);
1032+
1033+
expect(cell.classList.contains('selected')).toBeTruthy();
1034+
});
1035+
1036+
it('should add selected CSS alongside an existing cell CSS hash', () => {
1037+
grid = new TestGrid(container, data, columns, defaultOptions);
1038+
grid.setCellCssStyles('highlight', { 1: { firstName: 'highlight' } });
1039+
1040+
const cell = (grid as TestGrid).callAppendSelectedCellHtml(1, 0);
1041+
1042+
expect(cell.classList).toContain('highlight');
1043+
expect(cell.classList).toContain('selected');
1044+
});
1045+
9871046
it('should call SlickHybridSelectionModel.onDragReplaceCells() when selection mode is REP and range is expanding', () => {
9881047
const hybridSelectionModel = new SlickHybridSelectionModel();
9891048
hybridSelectionModel.activeSelectionIsRow = true;

‎packages/common/src/core/slickDataView.ts‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
156156
protected toggledGroupsByLevel: any[] = [];
157157
protected groupingDelimiter = ':|:';
158158
protected selectedRowIds: DataIdType[] = [];
159+
protected pendingSelectedFilteredIds?: { ids: DataIdType[]; selectedRowIds: DataIdType[] };
159160
protected preSelectedRowIdsChangeFn?: (args?: any) => void;
160161

161162
protected pagesize = 0;
@@ -1392,8 +1393,21 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
13921393
if (rowIds === false) {
13931394
this.selectedRowIds = [];
13941395
} else {
1395-
if (this.selectedRowIds!.sort().join(',') !== rowIds.sort().join(',')) {
1396-
this.selectedRowIds = rowIds;
1396+
const sortedRowIds = rowIds.every((id, index) => index === 0 || `${rowIds[index - 1]}` <= `${id}`) ? rowIds : rowIds.slice().sort();
1397+
const sortedSelectedRowIds = this.selectedRowIds!.every(
1398+
(id, index) => index === 0 || `${this.selectedRowIds![index - 1]}` <= `${id}`
1399+
)
1400+
? this.selectedRowIds
1401+
: this.selectedRowIds!.slice().sort();
1402+
let selectedRowIdsChanged = this.selectedRowIds!.length !== sortedRowIds.length;
1403+
if (!selectedRowIdsChanged) {
1404+
const selectedRowIdsSet = new Set(this.selectedRowIds);
1405+
selectedRowIdsChanged = sortedRowIds.some((id) => !selectedRowIdsSet.has(id));
1406+
}
1407+
if (selectedRowIdsChanged) {
1408+
this.selectedRowIds = sortedRowIds;
1409+
} else if (sortedSelectedRowIds !== this.selectedRowIds) {
1410+
this.selectedRowIds = sortedSelectedRowIds;
13971411
}
13981412
}
13991413
};
@@ -1435,10 +1449,17 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
14351449
dataView: this,
14361450
};
14371451
this.preSelectedRowIdsChangeFn!(selectedRowsChangedArgs);
1452+
const isBulkSelection = args.caller === 'click.selectAll' || args.caller === 'click.unselectAll';
1453+
const pendingSelectedFilteredIds = this.pendingSelectedFilteredIds;
1454+
const filteredIds =
1455+
isBulkSelection && pendingSelectedFilteredIds?.selectedRowIds === this.selectedRowIds
1456+
? pendingSelectedFilteredIds.ids
1457+
: (this.getAllSelectedFilteredIds() as DataIdType[]);
1458+
this.pendingSelectedFilteredIds = undefined;
14381459
this.onSelectedRowIdsChanged.notify(
14391460
Object.assign(selectedRowsChangedArgs, {
14401461
selectedRowIds: this.selectedRowIds,
1441-
filteredIds: this.getAllSelectedFilteredIds() as DataIdType[],
1462+
filteredIds,
14421463
}),
14431464
new SlickEventData(),
14441465
this
@@ -1532,6 +1553,10 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
15321553
};
15331554
this.preSelectedRowIdsChangeFn?.(selectedRowsChangedArgs);
15341555

1556+
if (shouldTriggerEvent === false && applyRowSelectionToGrid === false) {
1557+
this.pendingSelectedFilteredIds = { ids: isRowBeingAdded ? selectedIds.slice() : [], selectedRowIds: this.selectedRowIds };
1558+
}
1559+
15351560
if (shouldTriggerEvent !== false) {
15361561
this.onSelectedRowIdsChanged.notify(
15371562
Object.assign(selectedRowsChangedArgs, {

0 commit comments

Comments
 (0)