perf: remove unnecessary events to re-render Header Grouping

- Header Grouping requires us to subscribe to a few events to make sure that the Header Grouping always follow the width of all columns that are contained in each group. However the `grid.onRendered` is already being called by many use cases, so there's no need to subscribe to so many events when we know the `onRendered` will be called anyway. - for example `onColumnsResized`, `onColumnsReordered`, `onColumnPickerColumnsChanged`, .... will all trigger the `onRendered`, so there's really no need to duplicate the work when we can subscribe to a lot less events to do the same work
ghiscoding · Sep 30, 2024 · ce02f0f · ce02f0f
1 parent 5ee077d
commit ce02f0f
Show file tree

Hide file tree

Showing 2 changed files with 1 addition and 143 deletions.
diff --git a/packages/common/src/services/__tests__/groupingAndColspan.service.spec.ts b/packages/common/src/services/__tests__/groupingAndColspan.service.spec.ts
@@ -167,18 +167,6 @@ describe('GroupingAndColspanService', () => {
       expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
     });
 
-    it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid "onSort"', () => {
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      gridStub.onSort.notify({ columnId: 'lastName', sortAsc: true, sortCol: mockColumns[0] }, new SlickEventData(), gridStub);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
     it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid "onAutosizeColumns"', () => {
       const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
 
@@ -203,103 +191,6 @@ describe('GroupingAndColspanService', () => {
       expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
     });
 
-    it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid "onColumnsResized"', () => {
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      gridStub.onColumnsResized.notify({ triggeredByColumn: 'lastName', grid: gridStub }, new SlickEventData(), gridStub);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
-    it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid "onColumnsReordered"', () => {
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      gridStub.onColumnsReordered.notify({ impactedColumns: [], grid: gridStub }, new SlickEventData(), gridStub);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
-    it('should call the "renderPreHeaderRowGroupingTitles" after triggering a dataView "onColumnsResized"', () => {
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      dataViewStub.onRowCountChanged.notify({ previous: 1, current: 2, dataView: dataViewStub, callingOnRowsChanged: false, itemCount: 1 }, new SlickEventData(), gridStub);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(2);
-      expect(setTimeoutSpy).toHaveBeenNthCalledWith(1, expect.any(Function), 75);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 0);
-    });
-
-    it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid resize', () => {
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      resizerPluginStub.onGridAfterResize.notify({}, new SlickEventData(), gridStub);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
-    it('should call the "renderPreHeaderRowGroupingTitles" after changing column visibility from column picker', () => {
-      const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
-      const divHeaderColumns = document.getElementsByClassName('slick-header-columns');
-      vi.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      fnCallbacks['onColumnPickerColumnsChanged'](columnsMock);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2); // 1x for init, 1x for event
-      expect(divHeaderColumns.length).toBeGreaterThan(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
-    it('should call the "renderPreHeaderRowGroupingTitles" after changing column visibility from grid menu', () => {
-      const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
-      const divHeaderColumns = document.getElementsByClassName('slick-header-columns');
-      vi.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      fnCallbacks['onGridMenuColumnsChanged'](columnsMock);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2); // 1x for init, 1x for event
-      expect(divHeaderColumns.length).toBeGreaterThan(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
-    it('should call the "renderPreHeaderRowGroupingTitles" after changing column visibility & closing the grid menu', () => {
-      const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
-      const divHeaderColumns = document.getElementsByClassName('slick-header-columns');
-      vi.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      fnCallbacks['onGridMenuMenuClose'](columnsMock);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2); // 1x for init, 1x for event
-      expect(divHeaderColumns.length).toBeGreaterThan(2);
-      expect(setTimeoutSpy).toHaveBeenCalledTimes(1);
-      expect(setTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Function), 75);
-    });
-
     it('should call the "renderPreHeaderRowGroupingTitles" after calling the "translateGroupingAndColSpan" method', () => {
       gridOptionMock.enableTranslate = true;
       const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
@@ -366,19 +257,5 @@ describe('GroupingAndColspanService', () => {
       expect(divHeaderColumns.length).toBeGreaterThan(2);
       expect(divHeaderColumns[0].outerHTML).toEqual(`<div style="width: 2815px; left: -1000px;" class="slick-header-columns">All your colums div here</div>`);
     });
-
-    it('should  call the "renderPreHeaderRowGroupingTitles" when "onHeaderMenuHideColumns" is triggered', () => {
-      const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
-      const divHeaderColumns = document.getElementsByClassName('slick-header-columns');
-      vi.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
-      const renderSpy = vi.spyOn(service, 'renderPreHeaderRowGroupingTitles');
-
-      service.init(gridStub);
-      fnCallbacks['onHeaderMenuHideColumns'](columnsMock);
-      vi.runAllTimers(); // fast-forward timer
-
-      expect(renderSpy).toHaveBeenCalledTimes(2); // 1x for init, 1x for event
-      expect(divHeaderColumns.length).toBeGreaterThan(2);
-    });
   });
 });
diff --git a/packages/common/src/services/groupingAndColspan.service.ts b/packages/common/src/services/groupingAndColspan.service.ts
@@ -1,7 +1,7 @@
 import type { BasePubSubService, EventSubscription } from '@slickgrid-universal/event-pub-sub';
 import { createDomElement, emptyElement } from '@slickgrid-universal/utils';
 
-import type { Column, GridOption, SlickResizer, } from './../interfaces/index';
+import type { Column, GridOption, } from './../interfaces/index';
 import type { ExtensionUtility } from '../extensions/extensionUtility';
 import { type SlickDataView, SlickEventHandler, type SlickGrid } from '../core/index';
 
@@ -37,7 +37,6 @@ export class GroupingAndColspanService {
   /**
    * Initialize the Service
    * @param {object} grid
-   * @param {object} resizerPlugin
    */
   init(grid: SlickGrid): void {
     this._grid = grid;
@@ -51,28 +50,10 @@ export class GroupingAndColspanService {
           this.translateGroupingAndColSpan();
         }
 
-        this._eventHandler.subscribe(grid.onSort, () => this.renderPreHeaderRowGroupingTitles());
         this._eventHandler.subscribe(grid.onRendered, () => this.renderPreHeaderRowGroupingTitles());
         this._eventHandler.subscribe(grid.onAutosizeColumns, () => this.renderPreHeaderRowGroupingTitles());
-        this._eventHandler.subscribe(grid.onColumnsResized, () => this.renderPreHeaderRowGroupingTitles());
-        this._eventHandler.subscribe(grid.onColumnsReordered, () => this.renderPreHeaderRowGroupingTitles());
         this._eventHandler.subscribe(this._dataView.onRowCountChanged, () => this.delayRenderPreHeaderRowGroupingTitles(0));
 
-        // for both picker (columnPicker/gridMenu) we also need to re-create after hiding/showing columns
-        this._subscriptions.push(
-          this.pubSubService.subscribe(
-            ['onColumnPickerColumnsChanged', 'onGridMenuColumnsChanged', 'onGridMenuMenuClose'],
-            () => this.renderPreHeaderRowGroupingTitles()
-          ),
-          this.pubSubService.subscribe('onHeaderMenuHideColumns', () => this.delayRenderPreHeaderRowGroupingTitles(0)),
-        );
-
-        // we also need to re-create after a grid resize
-        const resizerPlugin = grid.getPluginByName<SlickResizer>('Resizer');
-        if (resizerPlugin?.onGridAfterResize) {
-          this._eventHandler.subscribe(resizerPlugin.onGridAfterResize, () => this.renderPreHeaderRowGroupingTitles());
-        }
-
         // and finally we need to re-create after user calls the Grid "setOptions" when changing from regular to frozen grid (and vice versa)
         this._eventHandler.subscribe(grid.onSetOptions, (_e, args) => {
           // when user changes frozen columns dynamically (e.g. from header menu), we need to re-render the pre-header of the grouping titles