fix: hideColumnByIds() should call setColumn() only once (#1736)

- we should only ever call `setColumn()` once when hiding multiple columns to avoid perf issues - also add `showColumnByIds()` to align with `hideColumnByIds()`
ghiscoding · Nov 9, 2024 · 0ba1a93 · 0ba1a93
1 parent 976bd23
commit 0ba1a93
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 20 deletions.
diff --git a/examples/vite-demo-vanilla-bundle/src/examples/example07.html b/examples/vite-demo-vanilla-bundle/src/examples/example07.html
@@ -28,6 +28,10 @@ <h3 class="title is-size-3">
     <span class="mdi mdi-eye-off-outline"></span>
     <span>Dynamically Hide "Finish"</span>
   </button>
+  <button class="button is-small" data-test="show-column-subset-btn" onclick.delegate="showColumnSubset()">
+    <span class="mdi mdi-eye-outline"></span>
+    <span>Show Certain Columns</span>
+  </button>
   <button class="button is-small" data-test="add-item-btn" onclick.delegate="addItem()"
     title="Clear Filters &amp; Sorting to see it better">
     <span class="mdi mdi-plus"></span>

diff --git a/examples/vite-demo-vanilla-bundle/src/examples/example07.ts b/examples/vite-demo-vanilla-bundle/src/examples/example07.ts
@@ -638,6 +638,11 @@ export default class Example07 {
     // this.sgb.gridService.hideColumnByIds(['duration', 'finish'], { autoResizeColumns: false, hideFromColumnPicker: true, hideFromGridMenu: false });
   }
 
+  showColumnSubset() {
+    // note that calling this function will NOT include dynamically created columns like row selection & row move, you need to include them yourself
+    this.sgb.gridService.showColumnByIds(['_move', '_checkbox_selector', 'title', 'action', 'percentComplete', 'start', 'finish']);
+  }
+
   // Disable/Enable Filtering/Sorting functionalities
   // --------------------------------------------------
 

diff --git a/packages/common/src/interfaces/hideColumnOption.interface.ts b/packages/common/src/interfaces/hideColumnOption.interface.ts
@@ -1,5 +1,8 @@
 export interface HideColumnOption {
-  /** Defaults to true, do we want to auto-reize the columns in the grid after hidding the column(s)? */
+  /** Defaults to true, do we want to call `grid.setColumns()` */
+  applySetColumns?: boolean;
+
+  /** Defaults to true, do we want to auto-resize (by calling `grid.autosizeColumns()`) the columns in the grid after hidding the column(s)? */
   autoResizeColumns?: boolean;
 
   /** Defaults to false, do we want to hide the column name from the column picker after hidding the column from the grid? */
@@ -8,6 +11,14 @@ export interface HideColumnOption {
   /** Defaults to false, do we want to hide the column name from the grid menu after hidding the column from the grid? */
   hideFromGridMenu?: boolean;
 
-  /** Defaults to true, do we want to trigger an event "onHeaderMenuHideColumns" after hidding the column(s)? */
+  /** Defaults to true, do we want to trigger an event "onHideColumns" after hidding the column(s)? */
+  triggerEvent?: boolean;
+}
+
+export interface ShowColumnOption {
+  /** Defaults to true, do we want to auto-resize (by calling `grid.autosizeColumns()`) the columns in the grid after hidding the column(s)? */
+  autoResizeColumns?: boolean;
+
+  /** Defaults to true, do we want to trigger an event "onShowColumns" after showing the column(s)? */
   triggerEvent?: boolean;
 }
diff --git a/packages/common/src/services/__tests__/grid.service.spec.ts b/packages/common/src/services/__tests__/grid.service.spec.ts
@@ -1505,10 +1505,12 @@ describe('Grid Service', () => {
     it('should return -1 when the column id is not found in the list of loaded column definitions', () => {
       const mockColumns = [{ id: 'field1', width: 100 }, { id: 'field2', width: 150 }, { id: 'field3', field: 'field3' }] as Column[];
       vi.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
+      const setColSpy = vi.spyOn(gridStub, 'setColumns');
 
       const output = service.hideColumnById('xyz');
 
       expect(output).toBe(-1);
+      expect(setColSpy).not.toHaveBeenCalled();
     });
 
     it('should set new columns minus the column to hide and it should keep new set as the new "visibleColumns"', () => {
@@ -1519,6 +1521,7 @@ describe('Grid Service', () => {
       const autoSizeSpy = vi.spyOn(gridStub, 'autosizeColumns');
       const setColsSpy = vi.spyOn(gridStub, 'setColumns');
       const pubSubSpy = vi.spyOn(pubSubServiceStub, 'publish');
+      const setColSpy = vi.spyOn(gridStub, 'setColumns');
 
       const output = service.hideColumnById('field2');
 
@@ -1527,6 +1530,7 @@ describe('Grid Service', () => {
       expect(setVisibleSpy).toHaveBeenCalledWith(mockWithoutColumns);
       expect(setColsSpy).toHaveBeenCalledWith(mockWithoutColumns);
       expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuHideColumns', { columns: mockWithoutColumns });
+      expect(setColSpy).toHaveBeenCalledTimes(1);
     });
 
     it('should set new columns minus the column to hide but without triggering an event when set to False', () => {
@@ -1605,14 +1609,17 @@ describe('Grid Service', () => {
       const autoSizeSpy = vi.spyOn(gridStub, 'autosizeColumns');
       const pubSubSpy = vi.spyOn(pubSubServiceStub, 'publish');
       const hideByIdSpy = vi.spyOn(service, 'hideColumnById');
+      const setColSpy = vi.spyOn(gridStub, 'setColumns');
 
       service.hideColumnByIds(['field2', 'field3']);
 
       expect(hideByIdSpy).toHaveBeenCalledTimes(2);
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
       expect(autoSizeSpy).toHaveBeenCalled();
       expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuHideColumns', { columns: expect.any(Array) });
+      expect(pubSubSpy).toHaveBeenCalledWith('onHideColumns', { columns: expect.any(Array) });
+      expect(setColSpy).toHaveBeenCalledTimes(1);
     });
 
     it('should loop through the Ids provided and call hideColumnById on each of them with same options BUT not auto size columns neither trigger when both are disabled', () => {
@@ -1621,14 +1628,16 @@ describe('Grid Service', () => {
       const autoSizeSpy = vi.spyOn(gridStub, 'autosizeColumns');
       const pubSubSpy = vi.spyOn(pubSubServiceStub, 'publish');
       const hideByIdSpy = vi.spyOn(service, 'hideColumnById');
+      const setColSpy = vi.spyOn(gridStub, 'setColumns');
 
       service.hideColumnByIds(['field2', 'field3'], { autoResizeColumns: false, triggerEvent: false });
 
       expect(hideByIdSpy).toHaveBeenCalledTimes(2);
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: false, triggerEvent: false });
       expect(autoSizeSpy).not.toHaveBeenCalled();
       expect(pubSubSpy).not.toHaveBeenCalled();
+      expect(setColSpy).toHaveBeenCalledTimes(1);
     });
 
     it('should loop through the Ids provided and call hideColumnById on each of them with same options and hide from column picker when "hideFromColumnPicker" is enabled', () => {
@@ -1639,8 +1648,8 @@ describe('Grid Service', () => {
       service.hideColumnByIds(['field2', 'field3'], { hideFromColumnPicker: true });
 
       expect(hideByIdSpy).toHaveBeenCalledTimes(2);
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { autoResizeColumns: false, hideFromColumnPicker: true, hideFromGridMenu: false, triggerEvent: false });
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { autoResizeColumns: false, hideFromColumnPicker: true, hideFromGridMenu: false, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: true, hideFromGridMenu: false, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: true, hideFromGridMenu: false, triggerEvent: false });
     });
 
     it('should loop through the Ids provided and call hideColumnById on each of them with same options and hide from column picker when "hideFromColumnPicker" is enabled', () => {
@@ -1651,8 +1660,26 @@ describe('Grid Service', () => {
       service.hideColumnByIds(['field2', 'field3'], { hideFromGridMenu: true });
 
       expect(hideByIdSpy).toHaveBeenCalledTimes(2);
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: true, triggerEvent: false });
-      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: true, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(1, 'field2', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: true, triggerEvent: false });
+      expect(hideByIdSpy).toHaveBeenNthCalledWith(2, 'field3', { applySetColumns: false, autoResizeColumns: false, hideFromColumnPicker: false, hideFromGridMenu: true, triggerEvent: false });
+    });
+  });
+
+  describe('showColumnByIds method', () => {
+    it('should loop through the Ids provided and call setColumns() with columns found from allColumns reference', () => {
+      const mockAllColumns = [{ id: 'field1', width: 100 }, { id: 'field2', width: 150 }, { id: 'field3', field: 'field3' }] as Column[];
+      vi.spyOn(gridStub, 'getColumns').mockReturnValue(mockAllColumns);
+      const pubSubSpy = vi.spyOn(pubSubServiceStub, 'publish');
+      const setColSpy = vi.spyOn(gridStub, 'setColumns');
+
+      service.showColumnByIds(['field2', 'field3']);
+
+      expect(pubSubSpy).toHaveBeenCalledWith('onShowColumns', { columns: expect.any(Array) });
+      expect(setColSpy).toHaveBeenCalledTimes(1);
+      expect(setColSpy).toHaveBeenCalledWith([
+        { excludeFromColumnPicker: true, excludeFromGridMenu: true, id: 'field2', width: 150 },
+        { excludeFromColumnPicker: true, excludeFromGridMenu: true, field: 'field3', id: 'field3' },
+      ]);
     });
   });
 

diff --git a/packages/common/src/services/grid.service.ts b/packages/common/src/services/grid.service.ts
@@ -11,6 +11,7 @@ import type {
   GridServiceUpdateOption,
   HideColumnOption,
   OnEventArgs,
+  ShowColumnOption,
 } from '../interfaces/index.js';
 import type { FilterService } from './filter.service.js';
 import type { GridStateService } from './gridState.service.js';
@@ -24,7 +25,8 @@ import { SlickRowSelectionModel } from '../extensions/slickRowSelectionModel.js'
 const GridServiceDeleteOptionDefaults: GridServiceDeleteOption = { skipError: false, triggerEvent: true };
 const GridServiceInsertOptionDefaults: GridServiceInsertOption = { highlightRow: true, resortGrid: false, selectRow: false, scrollRowIntoView: true, skipError: false, triggerEvent: true };
 const GridServiceUpdateOptionDefaults: GridServiceUpdateOption = { highlightRow: false, selectRow: false, scrollRowIntoView: false, skipError: false, triggerEvent: true };
-const HideColumnOptionDefaults: HideColumnOption = { autoResizeColumns: true, triggerEvent: true, hideFromColumnPicker: false, hideFromGridMenu: false };
+const HideColumnOptionDefaults: HideColumnOption = { applySetColumns: true, autoResizeColumns: true, triggerEvent: true, hideFromColumnPicker: false, hideFromGridMenu: false };
+const ShowColumnOptionDefaults: ShowColumnOption = { autoResizeColumns: true, triggerEvent: true };
 
 export class GridService {
   protected _grid!: SlickGrid;
@@ -190,21 +192,25 @@ export class GridService {
   }
 
   /**
-   * Hide a Column from the Grid by its column definition id, the column will just become hidden and will still show up in columnPicker/gridMenu
+   * Hide a Column from the Grid by its id, the column will just become hidden and will still show up in columnPicker/gridMenu
    * @param {string | number} columnId - column definition id
    * @param {boolean} triggerEvent - do we want to trigger an event (onHeaderMenuHideColumns) when column becomes hidden? Defaults to true.
    * @return {number} columnIndex - column index position when found or -1
    */
   hideColumnById(columnId: string | number, options?: HideColumnOption): number {
-    options = { ...HideColumnOptionDefaults, ...options };
-    if (this._grid && this._grid.getColumns && this._grid.setColumns) {
+    if (this._grid) {
+      options = { ...HideColumnOptionDefaults, ...options };
       const currentColumns = this._grid.getColumns();
       const colIndexFound = currentColumns.findIndex(col => col.id === columnId);
 
       if (colIndexFound >= 0) {
         const visibleColumns = arrayRemoveItemByIndex<Column>(currentColumns, colIndexFound);
         this.sharedService.visibleColumns = visibleColumns;
-        this._grid.setColumns(visibleColumns);
+
+        // do we want to apply the new columns in the grid
+        if (options?.applySetColumns) {
+          this._grid.setColumns(visibleColumns);
+        }
 
         const columnIndexFromAllColumns = this.sharedService.allColumns.findIndex(col => col.id === columnId);
         if (columnIndexFromAllColumns) {
@@ -232,24 +238,55 @@ export class GridService {
   }
 
   /**
-   * Hide a Column from the Grid by its column definition id(s), the column will just become hidden and will still show up in columnPicker/gridMenu
-   * @param {Array<string | number>} columnIds - column definition ids, can be a single string and an array of strings
-   * @param {boolean} triggerEvent - do we want to trigger an event (onHeaderMenuHideColumns) when column becomes hidden? Defaults to true.
+   * Hide multiple columns by their Ids, the column will just become hidden and will still show up in columnPicker/gridMenu
+   * @param {Array<string | number>} columnIds - column ids to hide
+   * @param {boolean} triggerEvent - do we want to trigger an event "onHideColumns" when column becomes hidden? Defaults to true.
    */
   hideColumnByIds(columnIds: Array<string | number>, options?: HideColumnOption): void {
-    options = { ...HideColumnOptionDefaults, ...options };
     if (Array.isArray(columnIds)) {
+      options = { ...HideColumnOptionDefaults, ...options };
       for (const columnId of columnIds) {
         // hide each column by its id but wait after the for loop to auto resize columns in the grid
-        this.hideColumnById(columnId, { ...options, triggerEvent: false, autoResizeColumns: false });
+        this.hideColumnById(columnId, { ...options, triggerEvent: false, applySetColumns: false, autoResizeColumns: false });
       }
+
+      // after looping through all columns, we can apply the new columns in the grid
+      this._grid.setColumns(this.sharedService.visibleColumns);
+
       // do we want to auto-resize the columns in the grid after hidding some? most often yes
       if (options?.autoResizeColumns) {
         this._grid.autosizeColumns();
       }
+
       // do we want to trigger an event after hidding
       if (options?.triggerEvent) {
+        // @deprecate `onHeaderMenuHideColumns` event, we should keep only `onHideColumns`
         this.pubSubService.publish('onHeaderMenuHideColumns', { columns: this.sharedService.visibleColumns });
+        this.pubSubService.publish('onHideColumns', { columns: this.sharedService.visibleColumns });
+      }
+    }
+  }
+
+  /**
+   * Show multiple columns by their Ids, any column outside the provided array will be considered hidden but will still show up in columnPicker/gridMenu
+   * @param {Array<string | number>} columnIds - column ids to show
+   * @param {boolean} triggerEvent - do we want to trigger an event (onShowColumns) when column becomes hidden? Defaults to true.
+   */
+  showColumnByIds(columnIds: Array<string | number>, options?: ShowColumnOption): void {
+    if (this._grid) {
+      options = { ...ShowColumnOptionDefaults, ...options };
+      const columns = this.sharedService.allColumns.filter(c => columnIds.includes(c.id));
+      this._grid.setColumns(columns);
+      this.sharedService.visibleColumns = columns;
+
+      // do we want to auto-resize the columns in the grid after hidding some? most often yes
+      if (options?.autoResizeColumns) {
+        this._grid.autosizeColumns();
+      }
+
+      // do we want to trigger an event after showing
+      if (options?.triggerEvent) {
+        this.pubSubService.publish('onShowColumns', { columns: this.sharedService.visibleColumns });
       }
     }
   }

diff --git a/test/cypress/e2e/example07.cy.ts b/test/cypress/e2e/example07.cy.ts
@@ -1188,4 +1188,15 @@ describe('Example 07 - Row Move & Checkbox Selector Selector Plugins', () => {
       .click()
       .then(() => expect(stub.getCall(0)).to.be.calledWith('Command: contact-chat'));
   });
+
+  it('should be able to show certain defined columns', () => {
+    const expectedTitles = ['', '', 'Title', 'Action', '% Complete', 'Start', 'Finish'];
+
+    cy.get('[data-test="show-column-subset-btn"]').click();
+
+    cy.get('.grid7')
+      .find('.slick-header-columns')
+      .children()
+      .each(($child, index) => expect($child.text()).to.eq(expectedTitles[index]));
+  });
 });