fix(core): showing/hiding column shouldn't affect its freezing positi…

…on (#185)

* fix(core): showing/hiding a column shouldn't affect freezing position
- when calling "hide column" we need to readjust the freezingColumn by (-1) when on left container
- when showing/hiding a column from ColumnPicker/GridMenu, we need to check if we need to also readjust when column is on the left container

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

@@ -110,7 +110,7 @@ export class Example12 {
     this._bindingEventService.bind(this.gridContainerElm, 'onbeforeeditcell', this.handleOnBeforeEditCell.bind(this));
     this._bindingEventService.bind(this.gridContainerElm, 'oncellchange', this.handleOnCellChange.bind(this));
     this._bindingEventService.bind(this.gridContainerElm, 'onclick', this.handleOnCellClicked.bind(this));
-    this._bindingEventService.bind(this.gridContainerElm, 'ongridstatechanged', this.handleOnSelectedRowsChanged.bind(this));
+    this._bindingEventService.bind(this.gridContainerElm, 'ongridstatechanged', this.handleOnGridStateChanged.bind(this));
     this._bindingEventService.bind(this.gridContainerElm, 'ondblclick', () => this.openCompositeModal('edit', 50));
     this._bindingEventService.bind(this.gridContainerElm, 'oncompositeeditorchange', this.handleOnCompositeEditorChange.bind(this));
     this._bindingEventService.bind(this.gridContainerElm, 'onpaginationchanged', this.handlePaginationChanged.bind(this));
@@ -518,7 +518,8 @@ export class Example12 {
     this.renderUnsavedStylingOnAllVisibleCells();
   }
 
-  handleOnSelectedRowsChanged(event) {
+  handleOnGridStateChanged(event) {
+    // console.log('handleOnGridStateChanged', event?.detail ?? '')
     const gridState = event && event.detail && event.detail.gridState;
     if (Array.isArray(gridState?.rowSelection.dataContextIds)) {
       this.isMassSelectionDisabled = gridState.rowSelection.dataContextIds.length === 0;

package.json

@@ -73,4 +73,4 @@
     "node": ">=12.0.0",
     "npm": ">=6.14.0"
   }
-}
+}

packages/common/package.json

@@ -70,7 +70,7 @@
     "lodash.isequal": "^4.5.0",
     "moment-mini": "^2.24.0",
     "multiple-select-modified": "^1.3.8",
-    "slickgrid": "^2.4.31"
+    "slickgrid": "^2.4.32"
   },
   "devDependencies": {
     "@types/dompurify": "^2.0.4",

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

@@ -9,6 +9,8 @@ declare const Slick: SlickNamespace;
 const gridStub = {
   getOptions: jest.fn(),
   registerPlugin: jest.fn(),
+  setColumns: jest.fn(),
+  setOptions: jest.fn(),
 } as unknown as SlickGrid;
 
 const mockAddon = jest.fn().mockImplementation(() => ({
@@ -74,41 +76,59 @@ describe('columnPickerExtension', () => {
       expect(mockAddon).toHaveBeenCalledWith(columnsMock, gridStub, gridOptionsMock);
     });
 
-    it('should call internal event handler subscribe and expect the "onColumnSpy" option to be called when addon notify is called', () => {
+    it('should call internal event handler subscribe and expect the "onColumnsChanged" grid option to be called when addon notify is called', () => {
       const handlerSpy = jest.spyOn(extension.eventHandler, 'subscribe');
       const onColumnSpy = jest.spyOn(SharedService.prototype.gridOptions.columnPicker as ColumnPicker, 'onColumnsChanged');
       const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
+      const readjustSpy = jest.spyOn(extensionUtility, 'readjustFrozenColumnIndexWhenNeeded');
 
       const instance = extension.register() as SlickColumnPicker;
-      instance.onColumnsChanged.notify({ allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
+      instance.onColumnsChanged.notify({ columnId: 'field1', showing: false, allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
 
+      expect(readjustSpy).not.toHaveBeenCalled();
       expect(handlerSpy).toHaveBeenCalledTimes(1);
       expect(handlerSpy).toHaveBeenCalledWith(
         { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
         expect.anything()
       );
-      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub });
+      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: false, allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub });
       expect(visibleColsSpy).not.toHaveBeenCalled();
     });
 
-    it(`should call internal event handler subscribe and expect the "onColumnSpy" option to be called when addon notify is called
+    it(`should call internal event handler subscribe and expect the "onColumnsChanged" grid option to be called when addon notify is called
     and it should override "visibleColumns" when array passed as arguments is bigger than previous visible columns`, () => {
       const handlerSpy = jest.spyOn(extension.eventHandler, 'subscribe');
       const onColumnSpy = jest.spyOn(SharedService.prototype.gridOptions.columnPicker as ColumnPicker, 'onColumnsChanged');
       const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
 
       const instance = extension.register() as SlickColumnPicker;
-      instance.onColumnsChanged.notify({ allColumns: columnsMock, columns: columnsMock, grid: gridStub }, new Slick.EventData(), gridStub);
+      instance.onColumnsChanged.notify({ columnId: 'field1', showing: true, allColumns: columnsMock, columns: columnsMock, grid: gridStub }, new Slick.EventData(), gridStub);
 
       expect(handlerSpy).toHaveBeenCalledTimes(1);
       expect(handlerSpy).toHaveBeenCalledWith(
         { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
         expect.anything()
       );
-      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { allColumns: columnsMock, columns: columnsMock, grid: gridStub });
+      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: true, allColumns: columnsMock, columns: columnsMock, grid: gridStub });
       expect(visibleColsSpy).toHaveBeenCalledWith(columnsMock);
     });
 
+    it('should call internal "onColumnsChanged" event and expect "readjustFrozenColumnIndexWhenNeeded" method to be called when the grid is detected to be a frozen grid', () => {
+      gridOptionsMock.frozenColumn = 0;
+      const handlerSpy = jest.spyOn(extension.eventHandler, 'subscribe');
+      const readjustSpy = jest.spyOn(extensionUtility, 'readjustFrozenColumnIndexWhenNeeded');
+
+      const instance = extension.register() as SlickColumnPicker;
+      instance.onColumnsChanged.notify({ columnId: 'field1', showing: false, allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
+
+      expect(handlerSpy).toHaveBeenCalledTimes(1);
+      expect(handlerSpy).toHaveBeenCalledWith(
+        { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
+        expect.anything()
+      );
+      expect(readjustSpy).toHaveBeenCalledWith('field1', 0, false, columnsMock, columnsMock.slice(0, 1));
+    });
+
     it('should dispose of the addon', () => {
       const instance = extension.register() as SlickColumnPicker;
       const destroySpy = jest.spyOn(instance, 'destroy');

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

@@ -1,9 +1,16 @@
 import { ExtensionName } from '../../enums/index';
-import { GridOption } from '../../interfaces/index';
+import { Column, GridOption, SlickGrid } from '../../interfaces/index';
 import { ExtensionUtility } from '../extensionUtility';
 import { SharedService } from '../../services/shared.service';
 import { TranslateServiceStub } from '../../../../../test/translateServiceStub';
 
+const gridStub = {
+  getOptions: jest.fn(),
+  setColumns: jest.fn(),
+  setOptions: jest.fn(),
+  registerPlugin: jest.fn(),
+} as unknown as SlickGrid;
+
 const mockAddon = jest.fn().mockImplementation(() => ({
   init: jest.fn(),
   destroy: jest.fn()
@@ -197,6 +204,71 @@ describe('extensionUtility', () => {
         expect(output).toBe('Commandes');
       });
     });
+
+    describe('readjustFrozenColumnIndexWhenNeeded method', () => {
+      let gridOptionsMock: GridOption;
+
+      beforeEach(() => {
+        gridOptionsMock = { frozenColumn: 1 } as GridOption;
+        jest.spyOn(SharedService.prototype, 'slickGrid', 'get').mockReturnValue(gridStub);
+        jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
+        jest.spyOn(SharedService.prototype, 'frozenVisibleColumnId', 'get').mockReturnValue('field2');
+      });
+
+      afterEach(() => {
+        jest.clearAllMocks();
+      });
+
+      it('should increase "frozenColumn" from 0 to 1 when showing a column that was previously hidden and its index is lower or equal to provided argument (2nd arg, frozenColumnIndex)', () => {
+        const allColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
+        const visibleColumns = [{ id: 'field1' }, { id: 'field2' }] as Column[];
+        const setOptionSpy = jest.spyOn(SharedService.prototype.slickGrid, 'setOptions');
+
+        utility.readjustFrozenColumnIndexWhenNeeded('field1', 0, true, allColumns, visibleColumns);
+
+        expect(setOptionSpy).toHaveBeenCalledWith({ frozenColumn: 1 });
+      });
+
+      it('should keep "frozenColumn" at 0 when showing a column that was previously hidden and its index is greater than provided argument (2nd arg, frozenColumnIndex)', () => {
+        const allColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
+        const visibleColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
+        const setOptionSpy = jest.spyOn(SharedService.prototype.slickGrid, 'setOptions');
+
+        utility.readjustFrozenColumnIndexWhenNeeded('field3', 0, true, allColumns, visibleColumns);
+
+        expect(setOptionSpy).not.toHaveBeenCalled();
+      });
+
+      it('should decrease "frozenColumn" from 1 to 0 when hiding a column that was previously shown and its index is lower or equal to provided argument (2nd arg, frozenColumnIndex)', () => {
+        const allColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
+        const visibleColumns = [{ id: 'field1' }, { id: 'field2' }] as Column[];
+        const setOptionSpy = jest.spyOn(SharedService.prototype.slickGrid, 'setOptions');
+
+        utility.readjustFrozenColumnIndexWhenNeeded('field1', 1, false, allColumns, visibleColumns);
+
+        expect(setOptionSpy).toHaveBeenCalledWith({ frozenColumn: 0 });
+      });
+
+      it('should keep "frozenColumn" at 1 when hiding a column that was previously hidden and its index is greater than provided argument (2nd arg, frozenColumnIndex)', () => {
+        const allColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
+        const visibleColumns = [{ id: 'field1' }, { id: 'field2' }] as Column[];
+        const setOptionSpy = jest.spyOn(SharedService.prototype.slickGrid, 'setOptions');
+
+        utility.readjustFrozenColumnIndexWhenNeeded('field3', 1, false, allColumns, visibleColumns);
+
+        expect(setOptionSpy).not.toHaveBeenCalled();
+      });
+
+      it('should not change "frozenColumn" when showing a column that was not found in the visibleColumns columns array', () => {
+        const allColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
+        const visibleColumns = [{ id: 'field1' }, { field: 'field2' }] as Column[];
+        const setOptionSpy = jest.spyOn(SharedService.prototype.slickGrid, 'setOptions');
+
+        utility.readjustFrozenColumnIndexWhenNeeded('fiel3', 0, true, allColumns, visibleColumns);
+
+        expect(setOptionSpy).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('without Translate Service', () => {

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

@@ -169,16 +169,18 @@ describe('gridMenuExtension', () => {
         const onCloseSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu as GridMenu, 'onMenuClose');
         const onCommandSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu as GridMenu, 'onCommand');
         const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
+        const readjustSpy = jest.spyOn(extensionUtility, 'readjustFrozenColumnIndexWhenNeeded');
 
         const instance = extension.register() as SlickGridMenu;
-        instance.onColumnsChanged!.notify({ allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
+        instance.onColumnsChanged!.notify({ columnId: 'field1', showing: false, allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
 
+        expect(readjustSpy).not.toHaveBeenCalled();
         expect(handlerSpy).toHaveBeenCalledTimes(5);
         expect(handlerSpy).toHaveBeenCalledWith(
           { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
           expect.anything()
         );
-        expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub });
+        expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: false, allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub });
         expect(onAfterSpy).not.toHaveBeenCalled();
         expect(onBeforeSpy).not.toHaveBeenCalled();
         expect(onCloseSpy).not.toHaveBeenCalled();
@@ -197,21 +199,37 @@ describe('gridMenuExtension', () => {
         const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
 
         const instance = extension.register() as SlickGridMenu;
-        instance.onColumnsChanged!.notify({ allColumns: columnsMock, columns: columnsMock, grid: gridStub }, new Slick.EventData(), gridStub);
+        instance.onColumnsChanged!.notify({ columnId: 'field1', showing: true, allColumns: columnsMock, columns: columnsMock, grid: gridStub }, new Slick.EventData(), gridStub);
 
         expect(handlerSpy).toHaveBeenCalledTimes(5);
         expect(handlerSpy).toHaveBeenCalledWith(
           { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
           expect.anything()
         );
-        expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { allColumns: columnsMock, columns: columnsMock, grid: gridStub });
+        expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: true, allColumns: columnsMock, columns: columnsMock, grid: gridStub });
         expect(onAfterSpy).not.toHaveBeenCalled();
         expect(onBeforeSpy).not.toHaveBeenCalled();
         expect(onCloseSpy).not.toHaveBeenCalled();
         expect(onCommandSpy).not.toHaveBeenCalled();
         expect(visibleColsSpy).toHaveBeenCalledWith(columnsMock);
       });
 
+      it('should call internal "onColumnsChanged" event and expect "readjustFrozenColumnIndexWhenNeeded" method to be called when the grid is detected to be a frozen grid', () => {
+        gridOptionsMock.frozenColumn = 0;
+        const handlerSpy = jest.spyOn(extension.eventHandler, 'subscribe');
+        const readjustSpy = jest.spyOn(extensionUtility, 'readjustFrozenColumnIndexWhenNeeded');
+
+        const instance = extension.register() as SlickGridMenu;
+        instance.onColumnsChanged.notify({ columnId: 'field1', showing: false, allColumns: columnsMock, columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
+
+        expect(handlerSpy).toHaveBeenCalledTimes(5);
+        expect(handlerSpy).toHaveBeenCalledWith(
+          { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
+          expect.anything()
+        );
+        expect(readjustSpy).toHaveBeenCalledWith('field1', 0, false, columnsMock, columnsMock.slice(0, 1));
+      });
+
       it('should call internal event handler subscribe and expect the "onBeforeMenuShow" option to be called when addon notify is called', () => {
         const handlerSpy = jest.spyOn(extension.eventHandler, 'subscribe');
         const onColumnSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu as GridMenu, 'onColumnsChanged');
@@ -311,7 +329,7 @@ describe('gridMenuExtension', () => {
         const autoSizeSpy = jest.spyOn(gridStub, 'autosizeColumns');
 
         const instance = extension.register() as SlickGridMenu;
-        instance!.onColumnsChanged!.notify({ grid: gridStub, allColumns: columnsMock, columns: columnsMock.slice(0, 1) }, new Slick.EventData(), gridStub);
+        instance!.onColumnsChanged!.notify({ columnId: 'field1', showing: true, grid: gridStub, allColumns: columnsMock, columns: columnsMock.slice(0, 1) }, new Slick.EventData(), gridStub);
         instance.onMenuClose!.notify({ allColumns: columnsMock, visibleColumns: columnsMock, grid: gridStub, menu: divElement }, new Slick.EventData(), gridStub);
 
         expect(handlerSpy).toHaveBeenCalled();

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

@@ -316,6 +316,7 @@ describe('headerMenuExtension', () => {
         jest.spyOn(gridStub, 'getColumnIndex').mockReturnValue(1);
         jest.spyOn(gridStub, 'getColumns').mockReturnValue(columnsMock);
         const setColumnsSpy = jest.spyOn(gridStub, 'setColumns');
+        const setOptionSpy = jest.spyOn(gridStub, 'setOptions');
         const visibleSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
         const updatedColumnsMock = [{
           id: 'field1', field: 'field1', nameKey: 'TITLE', width: 100,
@@ -332,6 +333,35 @@ describe('headerMenuExtension', () => {
 
         extension.hideColumn(columnsMock[1]);
 
+        expect(setOptionSpy).not.toHaveBeenCalled();
+        expect(visibleSpy).toHaveBeenCalledWith(updatedColumnsMock);
+        expect(setColumnsSpy).toHaveBeenCalledWith(updatedColumnsMock);
+      });
+
+      it('should call hideColumn and expect "setOptions" to be called with new "frozenColumn" index when the grid is detected to be a frozen grid', () => {
+        gridOptionsMock.frozenColumn = 1;
+        jest.spyOn(SharedService.prototype, 'slickGrid', 'get').mockReturnValue(gridStub);
+        jest.spyOn(gridStub, 'getColumnIndex').mockReturnValue(1);
+        jest.spyOn(gridStub, 'getColumns').mockReturnValue(columnsMock);
+        const setColumnsSpy = jest.spyOn(gridStub, 'setColumns');
+        const setOptionSpy = jest.spyOn(gridStub, 'setOptions');
+        const visibleSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
+        const updatedColumnsMock = [{
+          id: 'field1', field: 'field1', nameKey: 'TITLE', width: 100,
+          header: {
+            menu: {
+              items: [
+                { iconCssClass: 'fa fa-thumb-tack', title: 'Geler les colonnes', command: 'freeze-columns', positionOrder: 48 },
+                { divider: true, command: '', positionOrder: 49 },
+                { command: 'hide', iconCssClass: 'fa fa-times', positionOrder: 55, title: 'Cacher la colonne' }
+              ]
+            }
+          }
+        }] as Column[];
+
+        extension.hideColumn(columnsMock[1]);
+
+        expect(setOptionSpy).toHaveBeenCalledWith({ frozenColumn: 0 });
         expect(visibleSpy).toHaveBeenCalledWith(updatedColumnsMock);
         expect(setColumnsSpy).toHaveBeenCalledWith(updatedColumnsMock);
       });

packages/common/src/extensions/columnPickerExtension.ts

@@ -64,6 +64,13 @@ export class ColumnPickerExtension implements Extension {
           if (args && Array.isArray(args.columns) && args.columns.length !== this.sharedService.visibleColumns.length) {
             this.sharedService.visibleColumns = args.columns;
           }
+          // if we're using frozen columns, we need to readjust pinning when the new hidden column becomes visible again on the left pinning container
+          // we need to readjust frozenColumn index because SlickGrid freezes by index and has no knowledge of the columns themselves
+          const frozenColumnIndex = this.sharedService.gridOptions.frozenColumn ?? -1;
+          if (frozenColumnIndex >= 0) {
+            const { showing: isColumnShown, columnId, allColumns, columns: visibleColumns } = args;
+            this.extensionUtility.readjustFrozenColumnIndexWhenNeeded(columnId, frozenColumnIndex, isColumnShown, allColumns, visibleColumns);
+          }
         });
       }
       return this._addon;