fix(pinning): reordering cols position freezing cols shouldn't affect (…

…#275)

* fix(pinning): reordering cols position freezing cols shouldn't affect
- the steps to reproduce the issue was to create a grid with `frozenColumn` in the grid options, then change column position (cols reordering) and finally open header menu and freeze any of the column and the bug was that it was going back to original positions while it should keep new positions and just add new freeze column

examples/webpack-demo-vanilla-bundle/package.json

@@ -39,7 +39,7 @@
     "@types/webpack": "^4.41.26",
     "clean-webpack-plugin": "^3.0.0",
     "copy-webpack-plugin": "^7.0.0",
-    "css-loader": "^5.1.0",
+    "css-loader": "^5.1.1",
     "file-loader": "^6.2.0",
     "fork-ts-checker-webpack-plugin": "^6.1.0",
     "html-loader": "^2.1.1",
@@ -55,4 +55,4 @@
     "webpack-cli": "^4.5.0",
     "webpack-dev-server": "^3.11.2"
   }
-}
+}

package.json

@@ -52,9 +52,9 @@
   "devDependencies": {
     "@types/jest": "^26.0.20",
     "@types/node": "^14.14.31",
-    "@typescript-eslint/eslint-plugin": "^4.15.2",
-    "@typescript-eslint/parser": "^4.15.2",
-    "cypress": "^6.5.0",
+    "@typescript-eslint/eslint-plugin": "^4.16.1",
+    "@typescript-eslint/parser": "^4.16.1",
+    "cypress": "^6.6.0",
     "eslint": "^7.21.0",
     "eslint-plugin-import": "^2.22.1",
     "eslint-plugin-prefer-arrow": "^1.2.3",

packages/common/src/extensions/headerMenuExtension.ts

@@ -353,8 +353,8 @@ export class HeaderMenuExtension implements Extension {
           this.sharedService.frozenVisibleColumnId = args.column.id;
 
           // to freeze columns, we need to take only the visible columns and we also need to use setColumns() when some of them are hidden
-          // to make sure that we only use the visible columns, not doing this would show back some of the hidden columns
-          if (Array.isArray(visibleColumns) && Array.isArray(this.sharedService.allColumns) && visibleColumns.length !== this.sharedService.allColumns.length) {
+          // to make sure that we only use the visible columns, not doing this will have the undesired effect of showing back some of the hidden columns
+          if (this.sharedService.hasColumnsReordered || (Array.isArray(visibleColumns) && Array.isArray(this.sharedService.allColumns) && visibleColumns.length !== this.sharedService.allColumns.length)) {
             this.sharedService.slickGrid.setColumns(visibleColumns);
           }
           break;

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

@@ -60,6 +60,7 @@ const gridStub = {
   invalidate: jest.fn(),
   onColumnsReordered: new Slick.Event(),
   onColumnsResized: new Slick.Event(),
+  onRendered: new Slick.Event(),
   onSetOptions: new Slick.Event(),
   onSort: new Slick.Event(),
   render: jest.fn(),
@@ -168,6 +169,18 @@ describe('GroupingAndColspanService', () => {
       expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 75);
     });
 
+    it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid "onRendered"', () => {
+      const spy = jest.spyOn(service, 'renderPreHeaderRowGroupingTitles');
+
+      service.init(gridStub);
+      gridStub.onRendered.notify({ startRow: 0, endRow: 10, grid: gridStub }, new Slick.EventData(), gridStub);
+      jest.runAllTimers(); // fast-forward timer
+
+      expect(spy).toHaveBeenCalledTimes(2);
+      expect(setTimeout).toHaveBeenCalledTimes(1);
+      expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 75);
+    });
+
     it('should call the "renderPreHeaderRowGroupingTitles" after triggering a grid "onColumnsResized"', () => {
       const spy = jest.spyOn(service, 'renderPreHeaderRowGroupingTitles');
 

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

@@ -216,6 +216,16 @@ describe('Shared Service', () => {
     expect(service.frozenVisibleColumnId).toEqual('field1');
   });
 
+  it('should call "hasColumnsReordered" GETTER and expect a boolean value to be returned', () => {
+    const flag = service.hasColumnsReordered;
+    expect(flag).toEqual(false);
+  });
+
+  it('should call "hasColumnsReordered" GETTER and SETTER expect same value to be returned', () => {
+    service.hasColumnsReordered = true;
+    expect(service.hasColumnsReordered).toEqual(true);
+  });
+
   it('should call "visibleColumns" GETTER and return all columns', () => {
     const spy = jest.spyOn(service, 'visibleColumns', 'get').mockReturnValue(mockColumns);
 

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

@@ -63,6 +63,7 @@ export class GroupingAndColspanService {
         }
 
         this._eventHandler.subscribe(grid.onSort, () => this.renderPreHeaderRowGroupingTitles());
+        this._eventHandler.subscribe(grid.onRendered, () => this.renderPreHeaderRowGroupingTitles());
         this._eventHandler.subscribe(grid.onColumnsResized, () => this.renderPreHeaderRowGroupingTitles());
         this._eventHandler.subscribe(grid.onColumnsReordered, () => this.renderPreHeaderRowGroupingTitles());
         this._eventHandler.subscribe(this._dataView.onRowCountChanged, () => this.delayRenderPreHeaderRowGroupingTitles(0));

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

@@ -6,6 +6,7 @@ export class SharedService {
   private _groupItemMetadataProvider!: SlickGroupItemMetadataProvider;
   private _grid!: SlickGrid;
   private _gridOptions!: GridOption;
+  private _hasColumnsReordered = false;
   private _currentPagination!: CurrentPagination;
   private _visibleColumns!: Column[];
   private _hideHeaderRowAfterPageLoad = false;
@@ -58,6 +59,15 @@ export class SharedService {
     this._frozenVisibleColumnId = columnId;
   }
 
+  /** Setter to keep the frozen column id for reference if we ever show/hide column from ColumnPicker/GridMenu afterward */
+  get hasColumnsReordered(): boolean {
+    return this._hasColumnsReordered;
+  }
+  /** Getter to keep the frozen column id for reference if we ever show/hide column from ColumnPicker/GridMenu afterward */
+  set hasColumnsReordered(isColumnReordered: boolean) {
+    this._hasColumnsReordered = isColumnReordered;
+  }
+
   /** Getter for SlickGrid Grid object */
   get slickGrid(): SlickGrid {
     return this._grid;

packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts

@@ -257,6 +257,7 @@ const mockGrid = {
   setSelectedRows: jest.fn(),
   onClick: new MockSlickEvent(),
   onClicked: new MockSlickEvent(),
+  onColumnsReordered: new MockSlickEvent(),
   onRendered: jest.fn(),
   onScroll: jest.fn(),
   onDataviewCreated: new MockSlickEvent(),
@@ -366,6 +367,20 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
     expect(sharedFrozenIndexSpy).toHaveBeenCalledWith('name');
   });
 
+  it('should update "visibleColumns" in the Shared Service when "onColumnsReordered" event is triggered', () => {
+    const sharedHasColumnsReorderedSpy = jest.spyOn(SharedService.prototype, 'hasColumnsReordered', 'set');
+    const sharedVisibleColumnsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
+    const newVisibleColumns = [{ id: 'lastName', field: 'lastName' }, { id: 'fristName', field: 'fristName' }];
+
+    component.gridOptions = { enableFiltering: true };
+    component.initialization(divContainer, slickEventHandler);
+    mockGrid.onColumnsReordered.notify({ impactedColumns: newVisibleColumns, grid: mockGrid });
+
+    expect(component.eventHandler).toEqual(slickEventHandler);
+    expect(sharedHasColumnsReorderedSpy).toHaveBeenCalledWith(true);
+    expect(sharedVisibleColumnsSpy).toHaveBeenCalledWith(newVisibleColumns);
+  });
+
   it('should create a grid and expect multiple Event Aggregator being called', () => {
     const pubSubSpy = jest.spyOn(eventPubSubService, 'publish');
 

packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts

@@ -883,6 +883,13 @@ export class SlickVanillaGridBundle {
           }
         });
       }
+
+      // when column are reordered, we need to update the visibleColumn array
+      const onColumnsReorderedHandler = grid.onColumnsReordered;
+      (this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onColumnsReorderedHandler>>).subscribe(onColumnsReorderedHandler, (_e, args) => {
+        this.sharedService.hasColumnsReordered = true;
+        this.sharedService.visibleColumns = args.impactedColumns;
+      });
     }
 
     // does the user have a colspan callback?