fix(pinning): recalculate frozen idx properly when column shown chang…

…es (#682)

- when using ColumnPicker, GridMenu or hiding a column via HeaderMenu, it has to recalculate the frozenColumn index because SlickGrid doesn't take care of that and the previous fix that I implement sometimes become out of sync. This PR simplifies the frozenColumn index position, it will simply update it when the index is different, as simple as that.

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

@@ -132,7 +132,7 @@ describe('columnPickerExtension', () => {
         { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
         expect.anything()
       );
-      expect(readjustSpy).toHaveBeenCalledWith('field1', 0, false, columnsMock, columnsMock.slice(0, 1));
+      expect(readjustSpy).toHaveBeenCalledWith(0, columnsMock, columnsMock.slice(0, 1));
     });
 
     it('should dispose of the addon', () => {

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

@@ -156,42 +156,42 @@ describe('ExtensionUtility', () => {
         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)', () => {
+      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 of 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);
+        utility.readjustFrozenColumnIndexWhenNeeded(0, 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[];
+      it('should keep "frozenColumn" at 1 when showing a column that was previously hidden and its index is greater than provided argument of frozenColumnIndex', () => {
+        const allColumns = [{ id: 'field1' }, { id: 'field2' }] 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);
+        utility.readjustFrozenColumnIndexWhenNeeded(1, 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)', () => {
+      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 of frozenColumnIndex', () => {
         const allColumns = [{ id: 'field1' }, { id: 'field2' }, { id: 'field3' }] as Column[];
-        const visibleColumns = [{ id: 'field1' }, { id: 'field2' }] as Column[];
+        const visibleColumns = [{ id: 'field2' }] as Column[];
         const setOptionSpy = jest.spyOn(SharedService.prototype.grid, 'setOptions');
 
-        utility.readjustFrozenColumnIndexWhenNeeded('field1', 1, false, allColumns, visibleColumns);
+        utility.readjustFrozenColumnIndexWhenNeeded(1, 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)', () => {
+      it('should keep "frozenColumn" at 1 when hiding a column that was previously hidden and its index is greater than provided argument of 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);
+        utility.readjustFrozenColumnIndexWhenNeeded(1, allColumns, visibleColumns);
 
         expect(setOptionSpy).not.toHaveBeenCalled();
       });
@@ -201,7 +201,7 @@ describe('ExtensionUtility', () => {
         const visibleColumns = [{ id: 'field1' }, { field: 'field2' }] as Column[];
         const setOptionSpy = jest.spyOn(SharedService.prototype.grid, 'setOptions');
 
-        utility.readjustFrozenColumnIndexWhenNeeded('fiel3', 0, true, allColumns, visibleColumns);
+        utility.readjustFrozenColumnIndexWhenNeeded(0, allColumns, visibleColumns);
 
         expect(setOptionSpy).not.toHaveBeenCalled();
       });

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

@@ -253,7 +253,7 @@ describe('gridMenuExtension', () => {
           { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
           expect.anything()
         );
-        expect(readjustSpy).toHaveBeenCalledWith('field1', 0, false, columnsMock, columnsMock.slice(0, 1));
+        expect(readjustSpy).toHaveBeenCalledWith(0, 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', () => {
@@ -355,8 +355,8 @@ describe('gridMenuExtension', () => {
         const autoSizeSpy = jest.spyOn(gridStub, 'autosizeColumns');
 
         const instance = extension.register();
-        instance.onColumnsChanged.notify(columnsMock.slice(0, 1), new Slick.EventData(), gridStub);
-        instance.onMenuClose.notify({ grid: gridStub, menu: {} }, 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 }, new Slick.EventData(), gridStub);
 
         expect(handlerSpy).toHaveBeenCalled();
         expect(onCloseSpy).toHaveBeenCalled();

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

@@ -66,8 +66,8 @@ export class ColumnPickerExtension implements Extension {
           // 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);
+            const { allColumns, columns: visibleColumns } = args;
+            this.extensionUtility.readjustFrozenColumnIndexWhenNeeded(frozenColumnIndex, allColumns, visibleColumns);
           }
         });
       }

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

@@ -75,28 +75,15 @@ export class ExtensionUtility {
    * When using ColumnPicker/GridMenu to show/hide a column, we potentially need to readjust the grid option "frozenColumn" index.
    * That is because SlickGrid freezes by column index and it has no knowledge of the columns themselves and won't change the index, we need to do that ourselves whenever necessary.
    * Note: we call this method right after the visibleColumns array got updated, it won't work properly if we call it before the setting the visibleColumns.
-   * @param {String} pickerColumnId - what is the column id triggered by the picker
    * @param {Number} frozenColumnIndex - current frozenColumn index
-   * @param {Boolean} showingColumn - is the column being shown or hidden?
    * @param {Array<Object>} allColumns - all columns (including hidden ones)
    * @param {Array<Object>} visibleColumns - only visible columns (excluding hidden ones)
    */
-  readjustFrozenColumnIndexWhenNeeded(pickerColumnId: string | number, frozenColumnIndex: number, showingColumn: boolean, allColumns: Column[], visibleColumns: Column[]) {
-    if (frozenColumnIndex >= 0 && pickerColumnId) {
-      // calculate a possible frozenColumn index variance
-      let frozenColIndexVariance = 0;
-      if (showingColumn) {
-        const definedFrozenColumnIndex = visibleColumns.findIndex(col => col.id === this.sharedService.frozenVisibleColumnId);
-        const columnIndex = visibleColumns.findIndex(col => col.id === pickerColumnId);
-        frozenColIndexVariance = (columnIndex >= 0 && (frozenColumnIndex >= columnIndex || definedFrozenColumnIndex === columnIndex)) ? 1 : 0;
-      } else {
-        const columnIndex = allColumns.findIndex(col => col.id === pickerColumnId);
-        frozenColIndexVariance = (columnIndex >= 0 && frozenColumnIndex >= columnIndex) ? -1 : 0;
-      }
-      // if we have a variance different than 0 then apply it
-      const newFrozenColIndex = frozenColumnIndex + frozenColIndexVariance;
-      if (frozenColIndexVariance !== 0) {
-        this.sharedService.grid.setOptions({ frozenColumn: newFrozenColIndex });
+  readjustFrozenColumnIndexWhenNeeded(frozenColumnIndex: number, allColumns: Column[], visibleColumns: Column[]) {
+    if (frozenColumnIndex >= 0) {
+      const recalculatedFrozenColumnIndex = visibleColumns.findIndex(col => col.id === this.sharedService.frozenVisibleColumnId);
+      if (recalculatedFrozenColumnIndex >= 0 && recalculatedFrozenColumnIndex !== frozenColumnIndex) {
+        this.sharedService.grid.setOptions({ frozenColumn: recalculatedFrozenColumnIndex });
       }
 
       // to freeze columns, we need to take only the visible columns and we also need to use setColumns() when some of them are hidden

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

@@ -124,8 +124,8 @@ export class GridMenuExtension implements Extension {
           // 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);
+            const { allColumns, columns: visibleColumns } = args;
+            this.extensionUtility.readjustFrozenColumnIndexWhenNeeded(frozenColumnIndex, allColumns, visibleColumns);
           }
         });
         this._eventHandler.subscribe(this._addon.onCommand, (e: any, args: MenuCommandItemCallbackArgs) => {

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

@@ -219,17 +219,17 @@ export class HeaderMenuExtension implements Extension {
   hideColumn(column: Column) {
     if (this.sharedService.grid && this.sharedService.grid.getColumns && this.sharedService.grid.setColumns && this.sharedService.grid.getColumnIndex) {
       const columnIndex = this.sharedService.grid.getColumnIndex(column.id);
-      const currentColumns = this.sharedService.grid.getColumns() as Column[];
+      const currentVisibleColumns = this.sharedService.grid.getColumns() as Column[];
 
       // if we're using frozen columns, we need to readjust pinning when the new hidden column is on the left pinning container
       // we need to do this because SlickGrid freezes by index and has no knowledge of the columns themselves
-      const frozenColumnIndex = this.sharedService.gridOptions.frozenColumn || -1;
+      const frozenColumnIndex = this.sharedService.gridOptions.frozenColumn !== undefined ? this.sharedService.gridOptions.frozenColumn : -1;
       if (frozenColumnIndex >= 0 && frozenColumnIndex >= columnIndex) {
         this.sharedService.grid.setOptions({ frozenColumn: frozenColumnIndex - 1 });
       }
 
       // then proceed with hiding the column in SlickGrid & trigger an event when done
-      const visibleColumns = arrayRemoveItemByIndex(currentColumns, columnIndex);
+      const visibleColumns = arrayRemoveItemByIndex(currentVisibleColumns, columnIndex);
       this.sharedService.visibleColumns = visibleColumns;
       this.sharedService.grid.setColumns(visibleColumns);
       this.sharedService.onHeaderMenuHideColumns.next(visibleColumns);