fix(core): header columns grouping misbehave after hiding column (#164)

- some header group titles disappear after hiding a column

.eslintignore

@@ -9,4 +9,5 @@
 *.md
 *.zip
 *.spec.ts
+**/__tests__/*.*
 **/dist/*.*

.eslintrc

@@ -21,6 +21,9 @@
   "plugins": [
     "@typescript-eslint"
   ],
+  "ignorePatterns": [
+    "**/__tests__/*.ts"
+  ],
   "rules": {
     "@typescript-eslint/adjacent-overload-signatures": "error",
     "@typescript-eslint/ban-ts-comment": "off",

examples/webpack-demo-vanilla-bundle/src/examples/icons.ts

@@ -113,6 +113,7 @@ export class Icons {
       '.mdi.mdi-filter-outline',
       '.mdi.mdi-filter-plus-outline',
       '.mdi.mdi-filter-remove-outline',
+      '.mdi.mdi-fire',
       '.mdi.mdi-flip-vertical',
       '.mdi.mdi-folder',
       '.mdi.mdi-folder-open',
@@ -123,6 +124,12 @@ export class Icons {
       '.mdi.mdi-history',
       '.mdi.mdi-information',
       '.mdi.mdi-information-outline',
+      '.mdi.mdi-lightbulb',
+      '.mdi.mdi-lightbulb-off',
+      '.mdi.mdi-lightbulb-off-outline',
+      '.mdi.mdi-lightbulb-on',
+      '.mdi.mdi-lightbulb-on-outline',
+      '.mdi.mdi-lightbulb-outline',
       '.mdi.mdi-link',
       '.mdi.mdi-link-variant',
       '.mdi.mdi-load',
@@ -146,6 +153,7 @@ export class Icons {
       '.mdi.mdi-redo',
       '.mdi.mdi-refresh',
       '.mdi.mdi-shape-square-plus',
+      '.mdi.mdi-snowflake',
       '.mdi.mdi-sort-ascending',
       '.mdi.mdi-sort-descending',
       '.mdi.mdi-sort-variant-remove',

packages/common/src/extensions/__tests__/cellExternalCopyManagerExtension.spec.ts

@@ -287,7 +287,7 @@ describe('cellExternalCopyManagerExtension', () => {
       extension.register() as SlickCellExternalCopyManager;
       const spy = jest.spyOn(extension.undoRedoBuffer, 'queueAndExecuteCommand');
 
-      extension.addonOptions.clipboardCommandHandler(queueCallback);
+      extension.addonOptions!.clipboardCommandHandler!(queueCallback);
 
       expect(spy).toHaveBeenCalled();
     });
@@ -298,7 +298,7 @@ describe('cellExternalCopyManagerExtension', () => {
       const getDataSpy = jest.spyOn(gridStub, 'getData').mockReturnValue(mockGetData);
       const addItemSpy = jest.spyOn(mockGetData, 'addItem');
 
-      extension.addonOptions.newRowCreator(2);
+      extension.addonOptions!.newRowCreator!(2);
 
       expect(getDataSpy).toHaveBeenCalled();
       expect(addItemSpy).toHaveBeenCalledWith(expect.objectContaining({ id: 'newRow_0' }));
@@ -307,7 +307,7 @@ describe('cellExternalCopyManagerExtension', () => {
 
     it('should expect a formatted output after calling "dataItemColumnValueExtractor" callback', () => {
       extension.register();
-      const output = extension.addonOptions.dataItemColumnValueExtractor({ firstName: 'John', lastName: 'Doe' }, { id: 'firstName', field: 'firstName', exportWithFormatter: true, formatter: Formatters.bold });
+      const output = extension.addonOptions!.dataItemColumnValueExtractor!({ firstName: 'John', lastName: 'Doe' }, { id: 'firstName', field: 'firstName', exportWithFormatter: true, formatter: Formatters.bold });
       expect(output).toBe('<b>John</b>');
     });
 
@@ -318,7 +318,7 @@ describe('cellExternalCopyManagerExtension', () => {
       jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       extension.register();
 
-      const output = extension.addonOptions.dataItemColumnValueExtractor({ firstName: '<b>John</b>', lastName: null }, { id: 'lastName', field: 'lastName', exportWithFormatter: true, formatter: myBoldFormatter });
+      const output = extension.addonOptions!.dataItemColumnValueExtractor!({ firstName: '<b>John</b>', lastName: null }, { id: 'lastName', field: 'lastName', exportWithFormatter: true, formatter: myBoldFormatter });
 
       expect(output).toBe('');
     });
@@ -328,7 +328,7 @@ describe('cellExternalCopyManagerExtension', () => {
       jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       extension.register();
 
-      const output = extension.addonOptions.dataItemColumnValueExtractor({ firstName: '<b>John</b>', lastName: 'Doe' }, { id: 'firstName', field: 'firstName', exportWithFormatter: true, formatter: Formatters.bold });
+      const output = extension.addonOptions!.dataItemColumnValueExtractor!({ firstName: '<b>John</b>', lastName: 'Doe' }, { id: 'firstName', field: 'firstName', exportWithFormatter: true, formatter: Formatters.bold });
 
       expect(output).toBe('John');
     });
@@ -339,7 +339,7 @@ describe('cellExternalCopyManagerExtension', () => {
       jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       extension.register();
 
-      const output = extension.addonOptions.dataItemColumnValueExtractor({ firstName: '<b>John</b>', lastName: 'Doe' }, { id: 'firstName', field: 'firstName', exportWithFormatter: true, formatter: myBoldFormatter });
+      const output = extension.addonOptions!.dataItemColumnValueExtractor!({ firstName: '<b>John</b>', lastName: 'Doe' }, { id: 'firstName', field: 'firstName', exportWithFormatter: true, formatter: myBoldFormatter });
 
       expect(output).toBe('John');
     });
@@ -349,7 +349,7 @@ describe('cellExternalCopyManagerExtension', () => {
       jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       extension.register();
 
-      const output = extension.addonOptions.dataItemColumnValueExtractor({ firstName: '<b>John</b>', lastName: 'Doe' }, { id: 'firstName', field: 'firstName' });
+      const output = extension.addonOptions!.dataItemColumnValueExtractor!({ firstName: '<b>John</b>', lastName: 'Doe' }, { id: 'firstName', field: 'firstName' });
 
       expect(output).toBeNull();
     });

packages/common/src/extensions/headerMenuExtension.ts

@@ -236,7 +236,7 @@ export class HeaderMenuExtension implements Extension {
       const visibleColumns = arrayRemoveItemByIndex<Column>(currentColumns, columnIndex);
       this.sharedService.visibleColumns = visibleColumns;
       this.sharedService.slickGrid.setColumns(visibleColumns);
-      this.pubSubService.publish('onHeaderMenuColumnsChanged', { columns: visibleColumns });
+      this.pubSubService.publish('onHeaderMenuHideColumns', { columns: visibleColumns, hiddenColumn: column });
     }
   }
 

packages/common/src/interfaces/hideColumnOption.interface.ts

@@ -8,6 +8,6 @@ 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 even "onHeaderMenuColumnsChanged" after hidding the column(s)? */
+  /** Defaults to true, do we want to trigger an event "onHeaderMenuHideColumns" after hidding the column(s)? */
   triggerEvent?: boolean;
 }

packages/common/src/services/__tests__/grid.service.spec.ts

@@ -1293,7 +1293,7 @@ describe('Grid Service', () => {
 
       expect(setVisibleSpy).toHaveBeenCalledWith(mockWithoutColumns);
       expect(setColsSpy).toHaveBeenCalledWith(mockWithoutColumns);
-      expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuColumnsChanged', { columns: mockWithoutColumns });
+      expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuHideColumns', { columns: mockWithoutColumns });
     });
 
     it('should set new columns minus the column to hide but without triggering an event when set to False', () => {
@@ -1337,7 +1337,7 @@ describe('Grid Service', () => {
       expect(autoSizeSpy).toHaveBeenCalled();
       expect(setVisibleSpy).toHaveBeenCalledWith(mockWithoutColumns);
       expect(setColsSpy).toHaveBeenCalledWith(mockWithoutColumns);
-      expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuColumnsChanged', { columns: mockWithoutColumns });
+      expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuHideColumns', { columns: mockWithoutColumns });
     });
 
     it('should set new columns minus the column to hide but without triggering an event when set to False', () => {
@@ -1423,7 +1423,7 @@ describe('Grid Service', () => {
       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(autoSizeSpy).toHaveBeenCalled();
-      expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuColumnsChanged', { columns: expect.toBeArray() });
+      expect(pubSubSpy).toHaveBeenCalledWith('onHeaderMenuHideColumns', { columns: expect.toBeArray() });
     });
 
     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', () => {

packages/common/src/services/__tests__/gridState.service.spec.ts

@@ -708,6 +708,7 @@ describe('GridStateService', () => {
     });
 
     it('should return null when no BackendService is used and FilterService is missing the "getCurrentLocalFilters" method', () => {
+      // @ts-ignore
       gridStub.getOptions = undefined;
       const output = service.getCurrentFilters();
       expect(output).toBeNull();
@@ -760,7 +761,7 @@ describe('GridStateService', () => {
 
   describe('needToPreserveRowSelection method', () => {
     it('should return false when there are no "dataView" property defined in the grid options', () => {
-      const gridOptionsMock = { dataView: null } as GridOption;
+      const gridOptionsMock = { dataView: null } as unknown as GridOption;
       jest.spyOn(gridStub, 'getOptions').mockReturnValue(gridOptionsMock);
 
       const output = service.needToPreserveRowSelection();
@@ -769,7 +770,7 @@ describe('GridStateService', () => {
     });
 
     it('should return false when "dataView" property is defined in the grid options with "syncGridSelection" property', () => {
-      const gridOptionsMock = { dataView: null } as GridOption;
+      const gridOptionsMock = { dataView: null } as unknown as GridOption;
       jest.spyOn(gridStub, 'getOptions').mockReturnValue(gridOptionsMock);
 
       const output = service.needToPreserveRowSelection();
@@ -945,7 +946,7 @@ describe('GridStateService', () => {
       expect(pubSubSpy).toHaveBeenCalledWith(`onGridStateChanged`, stateChangeMock);
     });
 
-    it('should trigger a "onGridStateChanged" event when "onHeaderMenuColumnsChanged" is triggered', () => {
+    it('should trigger a "onGridStateChanged" event when "onHeaderMenuHideColumns" is triggered', () => {
       const columnsMock1 = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
       const currentColumnsMock1 = [{ columnId: 'field1', cssClass: 'red', headerCssClass: '', width: 100 }] as CurrentColumn[];
       const gridStateMock = { columns: currentColumnsMock1, filters: [], sorters: [] } as GridState;
@@ -954,7 +955,7 @@ describe('GridStateService', () => {
       const getCurGridStateSpy = jest.spyOn(service, 'getCurrentGridState').mockReturnValue(gridStateMock);
       const getAssocCurColSpy = jest.spyOn(service, 'getAssociatedCurrentColumns').mockReturnValue(currentColumnsMock1);
 
-      fnCallbacks['onHeaderMenuColumnsChanged'](columnsMock1);
+      fnCallbacks['onHeaderMenuHideColumns'](columnsMock1);
 
       expect(getCurGridStateSpy).toHaveBeenCalled();
       expect(getAssocCurColSpy).toHaveBeenCalled();

packages/common/src/services/__tests__/groupingAndColspan.service.spec.ts

@@ -1,3 +1,4 @@
+// @ts-nocheck
 import { GroupingAndColspanService } from '../groupingAndColspan.service';
 import { Column, SlickDataView, GridOption, SlickEventHandler, SlickGrid, SlickNamespace, SlickColumnPicker, SlickGridMenu } from '../../interfaces/index';
 import { ExtensionUtility } from '../../extensions/extensionUtility';
@@ -9,6 +10,17 @@ const gridId = 'grid1';
 const gridUid = 'slickgrid_124343';
 const containerId = 'demo-container';
 
+const fnCallbacks = {};
+const mockPubSub = {
+  publish: jest.fn(),
+  subscribe: (eventName, fn) => fnCallbacks[eventName] = fn,
+  unsubscribe: jest.fn(),
+  unsubscribeAll: jest.fn(),
+};
+jest.mock('../pubSub.service', () => ({
+  PubSubService: () => mockPubSub
+}));
+
 const gridOptionMock = {
   createPreHeaderPanel: true,
   enablePagination: true,
@@ -85,7 +97,7 @@ describe('GroupingAndColspanService', () => {
     div.innerHTML = template;
     document.body.appendChild(div);
 
-    service = new GroupingAndColspanService(mockExtensionUtility, extensionServiceStub);
+    service = new GroupingAndColspanService(mockExtensionUtility, extensionServiceStub, mockPubSub);
     slickgridEventHandler = service.eventHandler;
   });
 
@@ -325,5 +337,19 @@ 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');
+      jest.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
+      const spy = jest.spyOn(service, 'renderPreHeaderRowGroupingTitles');
+
+      service.init(gridStub);
+      fnCallbacks['onHeaderMenuHideColumns'](columnsMock);
+      jest.runAllTimers(); // fast-forward timer
+
+      expect(spy).toHaveBeenCalledTimes(2); // 1x for init, 1x for event
+      expect(divHeaderColumns.length).toBeGreaterThan(2);
+    });
   });
 });

packages/common/src/services/grid.service.ts

@@ -205,7 +205,7 @@ export class GridService {
    * @deprecated Hide a Column from the Grid by its column definition index (the column will just become hidden and will still show up in columnPicker/gridMenu)
    * @see hideColumnById Please use "hideColumnById(id)" or "hideColumnByIds([ids])" instead since it has a lot more options
    * @param columnIndex - column definition index
-   * @param triggerEvent - do we want to trigger an event (onHeaderMenuColumnsChanged) when column becomes hidden? Defaults to true.
+   * @param triggerEvent - do we want to trigger an event (onHeaderMenuHideColumns) when column becomes hidden? Defaults to true.
    */
   hideColumnByIndex(columnIndex: number, triggerEvent = true) {
     if (this._grid && this._grid.getColumns && this._grid.setColumns) {
@@ -214,15 +214,15 @@ export class GridService {
       this.sharedService.visibleColumns = visibleColumns;
       this._grid.setColumns(visibleColumns);
       if (triggerEvent) {
-        this.pubSubService.publish('onHeaderMenuColumnsChanged', { columns: visibleColumns });
+        this.pubSubService.publish('onHeaderMenuHideColumns', { columns: visibleColumns });
       }
     }
   }
 
   /**
    * 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
    * @param {string | number} columnId - column definition id
-   * @param {boolean} triggerEvent - do we want to trigger an event (onHeaderMenuColumnsChanged) when column becomes hidden? Defaults to true.
+   * @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 {
@@ -253,7 +253,7 @@ export class GridService {
 
         // do we want to trigger an event after hidding
         if (options?.triggerEvent) {
-          this.pubSubService.publish('onHeaderMenuColumnsChanged', { columns: visibleColumns });
+          this.pubSubService.publish('onHeaderMenuHideColumns', { columns: visibleColumns });
         }
         return colIndexFound;
       }
@@ -264,7 +264,7 @@ 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 (onHeaderMenuColumnsChanged) when column becomes hidden? Defaults to true.
+   * @param {boolean} triggerEvent - do we want to trigger an event (onHeaderMenuHideColumns) when column becomes hidden? Defaults to true.
    */
   hideColumnByIds(columnIds: Array<string | number>, options?: HideColumnOption) {
     options = { ...HideColumnOptionDefaults, ...options };
@@ -279,7 +279,7 @@ export class GridService {
       }
       // do we want to trigger an event after hidding
       if (options?.triggerEvent) {
-        this.pubSubService.publish('onHeaderMenuColumnsChanged', { columns: this.sharedService.visibleColumns });
+        this.pubSubService.publish('onHeaderMenuHideColumns', { columns: this.sharedService.visibleColumns });
       }
     }
   }

packages/common/src/services/gridState.service.ts

@@ -361,7 +361,7 @@ export class GridStateService {
 
     // subscribe to HeaderMenu (hide column)
     this._subscriptions.push(
-      this.pubSubService.subscribe('onHeaderMenuColumnsChanged', (visibleColumns: Column[]) => {
+      this.pubSubService.subscribe('onHeaderMenuHideColumns', (visibleColumns: Column[]) => {
         const currentColumns: CurrentColumn[] = this.getAssociatedCurrentColumns(visibleColumns);
         this.pubSubService.publish('onGridStateChanged', { change: { newValues: currentColumns, type: GridStateType.columns }, gridState: this.getCurrentGridState() });
       })

packages/common/src/services/groupingAndColspan.service.ts

@@ -12,6 +12,7 @@ import {
 import { ExtensionName } from '../enums/index';
 import { ExtensionUtility } from '../extensions/extensionUtility';
 import { ExtensionService } from '../services/extension.service';
+import { PubSubService } from './pubSub.service';
 
 // using external non-typed js libraries
 declare const Slick: SlickNamespace;
@@ -20,7 +21,7 @@ export class GroupingAndColspanService {
   private _eventHandler: SlickEventHandler;
   private _grid: SlickGrid;
 
-  constructor(private extensionUtility: ExtensionUtility, private extensionService: ExtensionService) {
+  constructor(private extensionUtility: ExtensionUtility, private extensionService: ExtensionService, private pubSubService: PubSubService,) {
     this._eventHandler = new Slick.EventHandler();
   }
 
@@ -71,6 +72,9 @@ export class GroupingAndColspanService {
         if (columnPickerExtension?.instance?.onColumnsChanged) {
           this._eventHandler.subscribe(columnPickerExtension.instance.onColumnsChanged, () => this.renderPreHeaderRowGroupingTitles());
         }
+        this.pubSubService.subscribe('onHeaderMenuHideColumns', () => {
+          this.delayRenderPreHeaderRowGroupingTitles(0);
+        });
 
         const gridMenuExtension = this.extensionService.getExtensionByName(ExtensionName.gridMenu);
         if (gridMenuExtension && gridMenuExtension.instance && gridMenuExtension.instance.onColumnsChanged && gridMenuExtension.instance.onMenuClose) {
@@ -89,13 +93,13 @@ export class GroupingAndColspanService {
         (this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onSetOptionsHandler>>).subscribe(onSetOptionsHandler, (_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
           if (args?.optionsBefore?.frozenColumn !== args?.optionsAfter?.frozenColumn) {
-            setTimeout(() => this.renderPreHeaderRowGroupingTitles(), 0);
+            this.delayRenderPreHeaderRowGroupingTitles(0);
           }
         });
 
         // also not sure why at this point, but it seems that I need to call the 1st create in a delayed execution
         // probably some kind of timing issues and delaying it until the grid is fully ready fixes this problem
-        setTimeout(() => this.renderPreHeaderRowGroupingTitles(), 75);
+        this.delayRenderPreHeaderRowGroupingTitles(75);
       }
     }
   }
@@ -105,6 +109,11 @@ export class GroupingAndColspanService {
     this._eventHandler.unsubscribeAll();
   }
 
+  /** call "renderPreHeaderRowGroupingTitles()" with a setTimeout delay */
+  delayRenderPreHeaderRowGroupingTitles(delay = 0) {
+    setTimeout(() => this.renderPreHeaderRowGroupingTitles(), delay);
+  }
+
   /** Create or Render the Pre-Header Row Grouping Titles */
   renderPreHeaderRowGroupingTitles() {
     if (this._gridOptions && this._gridOptions.frozenColumn !== undefined && this._gridOptions.frozenColumn >= 0) {