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

…es (#241)

- 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.

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

@@ -126,7 +126,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', () => {

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

@@ -126,42 +126,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.slickGrid, '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.slickGrid, '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.slickGrid, '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.slickGrid, 'setOptions');
 
-        utility.readjustFrozenColumnIndexWhenNeeded('field3', 1, false, allColumns, visibleColumns);
+        utility.readjustFrozenColumnIndexWhenNeeded(1, allColumns, visibleColumns);
 
         expect(setOptionSpy).not.toHaveBeenCalled();
       });
@@ -171,7 +171,7 @@ describe('extensionUtility', () => {
         const visibleColumns = [{ id: 'field1' }, { field: 'field2' }] as Column[];
         const setOptionSpy = jest.spyOn(SharedService.prototype.slickGrid, 'setOptions');
 
-        utility.readjustFrozenColumnIndexWhenNeeded('fiel3', 0, true, allColumns, visibleColumns);
+        utility.readjustFrozenColumnIndexWhenNeeded(0, allColumns, visibleColumns);
 
         expect(setOptionSpy).not.toHaveBeenCalled();
       });

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

@@ -225,7 +225,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', () => {

packages/common/src/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 ?? -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);
           }
         });
       }

packages/common/src/extensions/extensionUtility.ts

@@ -73,28 +73,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.slickGrid.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.slickGrid.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

packages/common/src/extensions/gridMenuExtension.ts

@@ -129,8 +129,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 ?? -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);
             }
           });
         }

packages/common/src/extensions/headerMenuExtension.ts

@@ -228,17 +228,17 @@ export class HeaderMenuExtension implements Extension {
   hideColumn(column: Column) {
     if (this.sharedService.slickGrid && this.sharedService.slickGrid.getColumns && this.sharedService.slickGrid.setColumns && this.sharedService.slickGrid.getColumnIndex) {
       const columnIndex = this.sharedService.slickGrid.getColumnIndex(column.id);
-      const currentColumns = this.sharedService.slickGrid.getColumns();
+      const currentVisibleColumns = this.sharedService.slickGrid.getColumns();
 
       // 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 ?? -1;
       if (frozenColumnIndex >= 0 && frozenColumnIndex >= columnIndex) {
         this.sharedService.slickGrid.setOptions({ frozenColumn: frozenColumnIndex - 1 });
       }
 
       // then proceed with hiding the column in SlickGrid & trigger an event when done
-      const visibleColumns = arrayRemoveItemByIndex<Column>(currentColumns, columnIndex);
+      const visibleColumns = arrayRemoveItemByIndex<Column>(currentVisibleColumns, columnIndex);
       this.sharedService.visibleColumns = visibleColumns;
       this.sharedService.slickGrid.setColumns(visibleColumns);
       this.pubSubService.publish('onHeaderMenuHideColumns', { columns: visibleColumns, hiddenColumn: column });