Merge pull request #1680 from ghiscoding/perf/grouping-rows-changed

perf: don't invalidate grid rows more than once, fixes #1678
ghiscoding · Sep 14, 2024 · 075d191 · 075d191
2 parents ac7e6f9 + c01164c
commit 075d191
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 38 deletions.
diff --git a/examples/vite-demo-vanilla-bundle/src/examples/example02.ts b/examples/vite-demo-vanilla-bundle/src/examples/example02.ts
@@ -295,6 +295,8 @@ export default class Example02 {
   }
 
   groupByDuration() {
+    // you need to manually add the sort icon(s) in UI
+    this.sgb?.slickGrid?.setSortColumns([{ columnId: 'duration', sortAsc: true }]);
     this.sgb?.dataView?.setGrouping({
       getter: 'duration',
       formatter: (g) => `Duration: ${g.value} <span class="text-green">(${g.count} items)</span>`,
@@ -306,9 +308,6 @@ export default class Example02 {
       aggregateCollapsed: false,
       lazyTotalsCalculation: true
     } as Grouping);
-
-    // you need to manually add the sort icon(s) in UI
-    this.sgb?.slickGrid?.setSortColumns([{ columnId: 'duration', sortAsc: true }]);
     this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
   }
 
@@ -329,7 +328,9 @@ export default class Example02 {
   }
 
   groupByDurationEffortDriven() {
-    this.sgb?.slickGrid?.setSortColumns([]);
+    // you need to manually add the sort icon(s) in UI
+    const sortColumns = [{ columnId: 'duration', sortAsc: true }, { columnId: 'effortDriven', sortAsc: true }];
+    this.sgb?.slickGrid?.setSortColumns(sortColumns);
     this.sgb?.dataView?.setGrouping([
       {
         getter: 'duration',
@@ -352,15 +353,17 @@ export default class Example02 {
         lazyTotalsCalculation: true
       }
     ] as Grouping[]);
-
-    // you need to manually add the sort icon(s) in UI
-    const sortColumns = [{ columnId: 'duration', sortAsc: true }, { columnId: 'effortDriven', sortAsc: true }];
-    this.sgb?.slickGrid?.setSortColumns(sortColumns);
     this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
   }
 
   groupByDurationEffortDrivenPercent() {
-    this.sgb?.slickGrid?.setSortColumns([]);
+    // you need to manually add the sort icon(s) in UI
+    const sortColumns = [
+      { columnId: 'duration', sortAsc: true },
+      { columnId: 'effortDriven', sortAsc: true },
+      { columnId: 'percentComplete', sortAsc: true }
+    ];
+    this.sgb?.slickGrid?.setSortColumns(sortColumns);
     this.sgb?.dataView?.setGrouping([
       {
         getter: 'duration',
@@ -392,14 +395,6 @@ export default class Example02 {
         lazyTotalsCalculation: true
       }
     ] as Grouping[]);
-
-    // you need to manually add the sort icon(s) in UI
-    const sortColumns = [
-      { columnId: 'duration', sortAsc: true },
-      { columnId: 'effortDriven', sortAsc: true },
-      { columnId: 'percentComplete', sortAsc: true }
-    ];
-    this.sgb?.slickGrid?.setSortColumns(sortColumns);
     this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
   }
 }
diff --git a/examples/vite-demo-vanilla-bundle/src/examples/example03.ts b/examples/vite-demo-vanilla-bundle/src/examples/example03.ts
@@ -416,9 +416,11 @@ export default class Example03 {
     this.selectedGroupingFields = [...this.selectedGroupingFields]; // force dirty checking
   }
 
-  clearGrouping() {
+  clearGrouping(invalidateRows = true) {
     this.draggableGroupingPlugin?.clearDroppedGroups();
-    this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
+    if (invalidateRows) {
+      this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
+    }
   }
 
   collapseAllGroups() {
@@ -436,28 +438,23 @@ export default class Example03 {
     });
   }
 
-  groupByDuration() {
-    this.clearGrouping();
+  groupByDurationOrderByCount(sortedByCount = false) {
+    this.durationOrderByCount = sortedByCount;
+    this.clearGrouping(false);
+
     if (this.draggableGroupingPlugin?.setDroppedGroups) {
       this.showTopHeader();
       this.draggableGroupingPlugin.setDroppedGroups('duration');
+
+      // you need to manually add the sort icon(s) in UI
+      const sortColumns = sortedByCount ? [] : [{ columnId: 'duration', sortAsc: true }];
+      this.sgb?.slickGrid?.setSortColumns(sortColumns);
       this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
     }
   }
 
-  groupByDurationOrderByCount(sortedByCount = false) {
-    this.durationOrderByCount = sortedByCount;
-    this.clearGrouping();
-    this.groupByDuration();
-
-    // you need to manually add the sort icon(s) in UI
-    const sortColumns = sortedByCount ? [] : [{ columnId: 'duration', sortAsc: true }];
-    this.sgb?.slickGrid?.setSortColumns(sortColumns);
-    this.sgb?.slickGrid?.invalidate(); // invalidate all rows and re-render
-  }
-
   groupByDurationEffortDriven() {
-    this.clearGrouping();
+    this.clearGrouping(false);
     if (this.draggableGroupingPlugin?.setDroppedGroups) {
       this.showTopHeader();
       this.draggableGroupingPlugin.setDroppedGroups(['duration', 'effortDriven']);

diff --git a/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts b/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts
@@ -247,6 +247,7 @@ const mockGrid = {
   getContainerNode: vi.fn(),
   getGridPosition: vi.fn(),
   getOptions: vi.fn(),
+  getRenderedRange: vi.fn(),
   getSelectionModel: vi.fn(),
   getScrollbarDimensions: vi.fn(),
   updateRow: vi.fn(),
@@ -1806,6 +1807,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
       it('should update each row and re-render the grid when filtering and DataView "onRowsChanged" event is triggered', () => {
         const renderSpy = vi.spyOn(mockGrid, 'render');
         const updateRowSpy = vi.spyOn(mockGrid, 'updateRow');
+        vi.spyOn(mockGrid, 'getRenderedRange').mockReturnValue({ bottom: 10, top: 0, leftPx: 0, rightPx: 890 });
 
         component.gridOptions = { enableFiltering: true };
         component.initialization(divContainer, slickEventHandler);

diff --git a/packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts b/packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts
@@ -831,12 +831,16 @@ export class SlickVanillaGridBundle<TData = any> {
         }
       });
 
-      // when filtering data with local dataset, we need to update each row else it will not always show correctly in the UI
-      // also don't use "invalidateRows" since it destroys the entire row and as bad user experience when updating a row
       if (gridOptions?.enableFiltering && !gridOptions.enableRowDetailView) {
-        this._eventHandler.subscribe(dataView.onRowsChanged, (_e, args) => {
-          if (args?.rows && Array.isArray(args.rows)) {
-            args.rows.forEach((row: number) => grid.updateRow(row));
+        this._eventHandler.subscribe(dataView.onRowsChanged, (_e, { calledOnRowCountChanged, rows }) => {
+          // filtering data with local dataset will not always show correctly unless we call this updateRow/render
+          // also don't use "invalidateRows" since it destroys the entire row and as bad user experience when updating a row
+          // see commit: https://github.com/ghiscoding/aurelia-slickgrid/commit/8c503a4d45fba11cbd8d8cc467fae8d177cc4f60
+          if (!calledOnRowCountChanged && Array.isArray(rows)) {
+            const ranges = grid.getRenderedRange();
+            rows
+              .filter(row => row >= ranges.top && row <= ranges.bottom)
+              .forEach((row: number) => grid.updateRow(row));
             grid.render();
           }
         });