Merge pull request #642 from ghiscoding/bugfix/hiding-showing-frozen-column-index

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

Author: ghiscoding

package.json

@@ -178,4 +178,4 @@
     "yargs": "^15.4.1",
     "zone.js": "~0.9.1"
   }
-}
+}

src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid-constructor.spec.ts

@@ -346,6 +346,16 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
     expect(component.gridOptions.enableMouseWheelScrollHandler).toBeTrue();
   });
 
+  it('should keep frozen column index reference (via frozenVisibleColumnId) when grid is a frozen grid', () => {
+    const sharedFrozenIndexSpy = jest.spyOn(SharedService.prototype, 'frozenVisibleColumnId', 'set');
+    component.gridOptions.frozenColumn = 0;
+
+    component.ngOnInit();
+    component.ngAfterViewInit();
+
+    expect(sharedFrozenIndexSpy).toHaveBeenCalledWith('name');
+  });
+
   describe('initialization method', () => {
     describe('columns definitions changed', () => {
       it('should expect "translateColumnHeaders" being called when "enableTranslate" is set', () => {

src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts

@@ -838,6 +838,12 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
     // emit the Grid & DataView object to make them available in parent component
     this.onGridCreated.emit(this.grid);
 
+    // when it's a frozen grid, we need to keep the frozen column id for reference if we ever show/hide column from ColumnPicker/GridMenu afterward
+    const frozenColumnIndex = this.gridOptions.frozenColumn !== undefined ? this.gridOptions.frozenColumn : -1;
+    if (frozenColumnIndex >= 0 && frozenColumnIndex <= this._columnDefinitions.length) {
+      this.sharedService.frozenVisibleColumnId = this._columnDefinitions[frozenColumnIndex].id || '';
+    }
+
     // initialize the SlickGrid grid
     this.grid.init();
 

src/app/modules/angular-slickgrid/extensions/__tests__/columnPickerExtension.spec.ts

@@ -11,6 +11,8 @@ declare const Slick: any;
 const gridStub = {
   getOptions: jest.fn(),
   registerPlugin: jest.fn(),
+  setColumns: jest.fn(),
+  setOptions: jest.fn(),
 };
 
 const mockAddon = jest.fn().mockImplementation(() => ({
@@ -80,41 +82,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, 'onColumnsChanged');
       const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
+      const readjustSpy = jest.spyOn(extensionUtility, 'readjustFrozenColumnIndexWhenNeeded');
 
       const instance = extension.register();
-      instance.onColumnsChanged.notify({ columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
+      instance.onColumnsChanged.notify({ columnId: 'field1', showing: false, 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(), { columns: columnsMock.slice(0, 1), grid: gridStub });
+      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: false, 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, 'onColumnsChanged');
       const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
 
       const instance = extension.register();
-      instance.onColumnsChanged.notify({ columns: columnsMock, grid: gridStub }, new Slick.EventData(), gridStub);
+      instance.onColumnsChanged.notify({ columnId: 'field1', showing: true, 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(), { columns: columnsMock, grid: gridStub });
+      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: true, 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();
+      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();
       const destroySpy = jest.spyOn(instance, 'destroy');

src/app/modules/angular-slickgrid/extensions/__tests__/extensionUtility.spec.ts

@@ -2,11 +2,18 @@ import { TestBed } from '@angular/core/testing';
 import { TranslateService, TranslateModule } from '@ngx-translate/core';
 
 import { ExtensionUtility } from '../extensionUtility';
-import { ExtensionName, GridOption } from '../../models';
+import { Column, ExtensionName, GridOption, SlickGrid } from '../../models';
 import { SharedService } from '../../services/shared.service';
 
 declare let Slick: any;
 
+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()
@@ -227,6 +234,71 @@ describe('ExtensionUtility', () => {
         expect(output).toBe('Commandes');
       });
     });
+
+    describe('readjustFrozenColumnIndexWhenNeeded method', () => {
+      let gridOptionsMock: GridOption;
+
+      beforeEach(() => {
+        gridOptionsMock = { frozenColumn: 1 } as GridOption;
+        jest.spyOn(SharedService.prototype, 'grid', '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.grid, '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.grid, '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.grid, '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.grid, '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.grid, 'setOptions');
+
+        utility.readjustFrozenColumnIndexWhenNeeded('fiel3', 0, true, allColumns, visibleColumns);
+
+        expect(setOptionSpy).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('without ngx-translate', () => {

src/app/modules/angular-slickgrid/extensions/__tests__/gridMenuExtension.spec.ts

@@ -195,16 +195,18 @@ describe('gridMenuExtension', () => {
         const onCloseSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu, 'onMenuClose');
         const onCommandSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu, 'onCommand');
         const visibleColsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
+        const readjustSpy = jest.spyOn(extensionUtility, 'readjustFrozenColumnIndexWhenNeeded');
 
         const instance = extension.register();
-        instance.onColumnsChanged.notify({ columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
+        instance.onColumnsChanged.notify({ columnId: 'field1', showing: false, 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(), { columns: columnsMock.slice(0, 1), grid: gridStub });
+        expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columnId: 'field1', showing: false, columns: columnsMock.slice(0, 1), grid: gridStub });
         expect(onAfterSpy).not.toHaveBeenCalled();
         expect(onBeforeSpy).not.toHaveBeenCalled();
         expect(onCloseSpy).not.toHaveBeenCalled();
@@ -238,6 +240,22 @@ describe('gridMenuExtension', () => {
         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();
+        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, 'onColumnsChanged');

src/app/modules/angular-slickgrid/extensions/__tests__/headerMenuExtension.spec.ts

@@ -341,6 +341,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,
@@ -357,6 +358,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, 'grid', '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);
       });

src/app/modules/angular-slickgrid/extensions/columnPickerExtension.ts

@@ -1,5 +1,5 @@
 import { Injectable } from '@angular/core';
-import { ColumnPicker, Extension, ExtensionName, SlickEventHandler } from '../models/index';
+import { Column, ColumnPicker, Extension, ExtensionName, SlickEventHandler } from '../models/index';
 import { ExtensionUtility } from './extensionUtility';
 import { SharedService } from '../services/shared.service';
 
@@ -56,13 +56,20 @@ export class ColumnPickerExtension implements Extension {
         if (this._columnPicker.onExtensionRegistered) {
           this._columnPicker.onExtensionRegistered(this._addon);
         }
-        this._eventHandler.subscribe(this._addon.onColumnsChanged, (e: any, args: { columns: any, grid: any }) => {
+        this._eventHandler.subscribe(this._addon.onColumnsChanged, (e: any, args: { columnId: string; showing: boolean; columns: Column[]; allColumns: Column[]; grid: any; }) => {
           if (this._columnPicker && typeof this._columnPicker.onColumnsChanged === 'function') {
             this._columnPicker.onColumnsChanged(e, args);
           }
           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 !== undefined ? 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;