fix(dom): ColumnPicker & GridMenu were creating multiple DOM elements

- add a new "getAddonInstance" method in the extension to return the correct instance of the SlickGrid addon (control)

package.json

@@ -88,7 +88,7 @@
     "lodash.isequal": "^4.5.0",
     "moment-mini": "^2.22.1",
     "rxjs": "^6.3.3",
-    "slickgrid": "2.4.9",
+    "slickgrid": "^2.4.11",
     "text-encoding-utf-8": "^1.0.2",
     "tslib": "^1.9.3",
     "vinyl-paths": "^2.1.0"

src/app/examples/grid-menu.component.html

@@ -1,22 +1,27 @@
 <div class="container-fluid">
-    <h2>{{title}}</h2>
-    <div class="subtitle" [innerHTML]="subTitle"></div>
+  <h2>{{title}}</h2>
+  <div class="subtitle"
+       [innerHTML]="subTitle"></div>
 
-    <button class="btn btn-default btn-sm" (click)="toggleGridMenu($event)">
-        <i class="fa fa-bars"></i>
-        Grid Menu
-    </button>
-    <button class="btn btn-default btn-sm" (click)="switchLanguage()">
-        <i class="fa fa-language"></i>
-        Switch Language
-    </button>
+  <button class="btn btn-default btn-sm"
+          (click)="toggleGridMenu($event)"
+          data-test="external-gridmenu">
+    <i class="fa fa-bars"></i>
+    Grid Menu
+  </button>
+  <button class="btn btn-default btn-sm"
+          (click)="switchLanguage()"
+          data-test="language">
+    <i class="fa fa-language"></i>
+    Switch Language
+  </button>
 
-    <div class="col-sm-12">
-        <angular-slickgrid gridId="grid2"
-            (onAngularGridCreated)="angularGridReady($event)"
-            [columnDefinitions]="columnDefinitions"
-            [gridOptions]="gridOptions"
-            [dataset]="dataset">
-        </angular-slickgrid>
-    </div>
+  <div class="col-sm-12">
+    <angular-slickgrid gridId="grid9"
+                       (onAngularGridCreated)="angularGridReady($event)"
+                       [columnDefinitions]="columnDefinitions"
+                       [gridOptions]="gridOptions"
+                       [dataset]="dataset">
+    </angular-slickgrid>
+  </div>
 </div>

src/app/examples/grid-menu.component.ts

@@ -35,16 +35,6 @@ export class GridMenuComponent implements OnInit {
   ngOnInit(): void {
     this.columnDefinitions = [
       { id: 'title', name: 'Title', field: 'title', headerKey: 'TITLE', filterable: true, type: FieldType.string },
-      {
-        id: 'phone', name: 'Phone Number using mask', field: 'phone',
-        filterable: true, sortable: true, minWidth: 100,
-        type: FieldType.string, // because we use a mask filter, we should always assume the value is a string for it to behave correctly
-        formatter: Formatters.mask, params: { mask: '(000) 000-0000' },
-        filter: {
-          model: Filters.inputMask,
-          operator: OperatorType.startsWith
-        }
-      },
       { id: 'duration', name: 'Duration', field: 'duration', headerKey: 'DURATION', sortable: true, filterable: true, type: FieldType.string },
       {
         id: '%', name: '% Complete', field: 'percentComplete', sortable: true, filterable: true,

src/app/examples/grid-odata.component.html

@@ -1,31 +1,35 @@
 <div class="container-fluid">
   <h2>{{title}}</h2>
-  <div class="subtitle row" [innerHTML]="subTitle"></div>
+  <div class="subtitle row"
+       [innerHTML]="subTitle"></div>
 
   <div class="row">
     <div class="col-sm-4">
-      <div [class]="status.class" role="alert">
+      <div [class]="status.class"
+           role="alert"
+           data-test="status">
         <strong>Status: </strong> {{status.text}}
         <span [hidden]="!processing">
           <i class="fa fa-refresh fa-spin fa-lg fa-fw"></i>
         </span>
       </div>
       <span *ngIf="statistics">
-            <b>Statistics:</b> {{statistics.endTime | date: 'yyyy-MM-dd HH:mm aaaaa\'m\''}} | {{statistics.executionTime}}ms | {{statistics.totalItemCount}} items
-        </span>
+        <b>Statistics:</b> {{statistics.endTime | date: 'yyyy-MM-dd HH:mm aaaaa\'m\''}} | {{statistics.executionTime}}ms
+        | {{statistics.totalItemCount}} items
+      </span>
     </div>
     <div class="col-sm-8">
-      <div class="alert alert-info">
-        <strong>OData Query:</strong> {{odataQuery}}
+      <div class="alert alert-info"
+           data-test="alert-odata-query">
+        <strong>OData Query:</strong> <span data-test="odata-query-result">{{odataQuery}}</span>
       </div>
     </div>
   </div>
 
-  <angular-slickgrid gridId="grid4"
-            [columnDefinitions]="columnDefinitions"
-            [gridOptions]="gridOptions"
-            [dataset]="dataset"
-            (onGridStateChanged)="gridStateChanged($event)">
+  <angular-slickgrid gridId="grid5"
+                     [columnDefinitions]="columnDefinitions"
+                     [gridOptions]="gridOptions"
+                     [dataset]="dataset"
+                     (onGridStateChanged)="gridStateChanged($event)">
   </angular-slickgrid>
 </div>
-

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

@@ -16,6 +16,7 @@ const gridStub = {
 const mockAddon = jest.fn().mockImplementation(() => ({
   init: jest.fn(),
   destroy: jest.fn(),
+  updateAllTitles: jest.fn(),
   onColumnsChanged: new Slick.Event(),
 }));
 
@@ -63,6 +64,7 @@ describe('columnPickerExtension', () => {
       jest.spyOn(SharedService.prototype, 'grid', 'get').mockReturnValue(gridStub);
       jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(columnsMock);
+      jest.spyOn(SharedService.prototype, 'visibleColumns', 'get').mockReturnValue(columnsMock.slice(0, 1));
       jest.spyOn(SharedService.prototype, 'columnDefinitions', 'get').mockReturnValue(columnsMock);
     });
 
@@ -81,18 +83,38 @@ describe('columnPickerExtension', () => {
     it('should call internal event handler subscribe and expect the "onColumnSpy" 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 instance = extension.register();
-      instance.onColumnsChanged.notify(columnsMock.slice(0, 1), new Slick.EventData(), gridStub);
+      instance.onColumnsChanged.notify({ 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(onColumnSpy).toHaveBeenCalledWith(expect.anything(), columnsMock.slice(0, 1));
+      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { 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
+    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);
+
+        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(visibleColsSpy).toHaveBeenCalledWith(columnsMock);
+      });
+
     it('should dispose of the addon', () => {
       const instance = extension.register();
       const destroySpy = jest.spyOn(instance, 'destroy');
@@ -110,11 +132,11 @@ describe('columnPickerExtension', () => {
 
       const instance = extension.register();
       extension.translateColumnPicker();
-      const initSpy = jest.spyOn(instance, 'init');
+      const updateColsSpy = jest.spyOn(instance, 'updateAllTitles');
 
       expect(utilitySpy).toHaveBeenCalled();
       expect(translateSpy).toHaveBeenCalled();
-      expect(initSpy).toHaveBeenCalledWith(gridStub);
+      expect(updateColsSpy).toHaveBeenCalledWith(SharedService.prototype.gridOptions.columnPicker);
       expect(SharedService.prototype.gridOptions.columnPicker.columnTitle).toBe('Colonnes');
       expect(SharedService.prototype.gridOptions.columnPicker.forceFitTitle).toBe('Ajustement forcé des colonnes');
       expect(SharedService.prototype.gridOptions.columnPicker.syncResizeTitle).toBe('Redimension synchrone');

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

@@ -45,6 +45,7 @@ const mockAddon = jest.fn().mockImplementation(() => ({
   init: jest.fn(),
   destroy: jest.fn(),
   showGridMenu: jest.fn(),
+  updateAllTitles: jest.fn(),
   onColumnsChanged: new Slick.Event(),
   onCommand: new Slick.Event(),
   onBeforeMenuShow: new Slick.Event(),
@@ -155,7 +156,7 @@ describe('gridMenuExtension', () => {
       jest.spyOn(SharedService.prototype, 'grid', 'get').mockReturnValue(gridStub);
       jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(columnsMock);
-      jest.spyOn(SharedService.prototype, 'visibleColumns', 'get').mockReturnValue(columnsMock);
+      jest.spyOn(SharedService.prototype, 'visibleColumns', 'get').mockReturnValue(columnsMock.slice(0, 1));
       jest.spyOn(SharedService.prototype, 'columnDefinitions', 'get').mockReturnValue(columnsMock);
     });
 
@@ -177,20 +178,46 @@ describe('gridMenuExtension', () => {
       const onBeforeSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu, 'onBeforeMenuShow');
       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 instance = extension.register();
-      instance.onColumnsChanged.notify(columnsMock.slice(0, 1), new Slick.EventData(), gridStub);
+      instance.onColumnsChanged.notify({ columns: columnsMock.slice(0, 1), grid: gridStub }, new Slick.EventData(), gridStub);
 
       expect(handlerSpy).toHaveBeenCalledTimes(4);
       expect(handlerSpy).toHaveBeenCalledWith(
         { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
         expect.anything()
       );
-      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), columnsMock.slice(0, 1));
+      expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columns: columnsMock.slice(0, 1), grid: gridStub });
       expect(onBeforeSpy).not.toHaveBeenCalled();
       expect(onCloseSpy).not.toHaveBeenCalled();
       expect(onCommandSpy).not.toHaveBeenCalled();
-    });
+      expect(visibleColsSpy).not.toHaveBeenCalled();
+    });
+
+    it(`should call internal event handler subscribe and expect the "onColumnsChanged" option to be 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.gridMenu, 'onColumnsChanged');
+        const onBeforeSpy = jest.spyOn(SharedService.prototype.gridOptions.gridMenu, 'onBeforeMenuShow');
+        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 instance = extension.register();
+        instance.onColumnsChanged.notify({ columns: columnsMock, grid: gridStub }, new Slick.EventData(), gridStub);
+
+        expect(handlerSpy).toHaveBeenCalledTimes(4);
+        expect(handlerSpy).toHaveBeenCalledWith(
+          { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
+          expect.anything()
+        );
+        expect(onColumnSpy).toHaveBeenCalledWith(expect.anything(), { columns: columnsMock, grid: gridStub });
+        expect(onBeforeSpy).not.toHaveBeenCalled();
+        expect(onCloseSpy).not.toHaveBeenCalled();
+        expect(onCommandSpy).not.toHaveBeenCalled();
+        expect(visibleColsSpy).toHaveBeenCalledWith(columnsMock);
+      });
 
     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');
@@ -622,11 +649,11 @@ describe('gridMenuExtension', () => {
 
       const instance = extension.register();
       extension.translateGridMenu();
-      const initSpy = jest.spyOn(instance, 'init');
+      const updateColsSpy = jest.spyOn(instance, 'updateAllTitles');
 
       expect(utilitySpy).toHaveBeenCalled();
       expect(translateSpy).toHaveBeenCalled();
-      expect(initSpy).toHaveBeenCalledWith(gridStub);
+      expect(updateColsSpy).toHaveBeenCalledWith(SharedService.prototype.gridOptions.gridMenu);
       expect(SharedService.prototype.gridOptions.gridMenu.columnTitle).toBe('Colonnes');
       expect(SharedService.prototype.gridOptions.gridMenu.forceFitTitle).toBe('Ajustement forcé des colonnes');
       expect(SharedService.prototype.gridOptions.gridMenu.syncResizeTitle).toBe('Redimension synchrone');

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

@@ -52,10 +52,13 @@ export class ColumnPickerExtension implements Extension {
         if (this.sharedService.gridOptions.columnPicker.onExtensionRegistered) {
           this.sharedService.gridOptions.columnPicker.onExtensionRegistered(this._addon);
         }
-        this._eventHandler.subscribe(this._addon.onColumnsChanged, (e: any, args: CellArgs) => {
+        this._eventHandler.subscribe(this._addon.onColumnsChanged, (e: any, args: { columns: any, grid: any }) => {
           if (this.sharedService.gridOptions.columnPicker && typeof this.sharedService.gridOptions.columnPicker.onColumnsChanged === 'function') {
             this.sharedService.gridOptions.columnPicker.onColumnsChanged(e, args);
           }
+          if (args && Array.isArray(args.columns) && args.columns.length > this.sharedService.visibleColumns.length) {
+            this.sharedService.visibleColumns = args.columns;
+          }
         });
       }
       return this._addon;
@@ -77,11 +80,8 @@ export class ColumnPickerExtension implements Extension {
       // translate all columns (including hidden columns)
       this.extensionUtility.translateItems(this.sharedService.allColumns, 'headerKey', 'name');
 
-      // re-initialize the Column Picker, that will recreate all the list
-      // doing an "init()" won't drop any existing command attached
-      if (this._addon.init) {
-        this._addon.init(this.sharedService.grid);
-      }
+      // update the Titles of each sections (command, customTitle, ...)
+      this._addon.updateAllTitles(this.sharedService.gridOptions.columnPicker);
     }
   }
 

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

@@ -91,11 +91,14 @@ export class GridMenuExtension implements Extension {
             this.sharedService.gridOptions.gridMenu.onBeforeMenuShow(e, args);
           }
         });
-        this._eventHandler.subscribe(this._addon.onColumnsChanged, (e: any, args: CellArgs) => {
+        this._eventHandler.subscribe(this._addon.onColumnsChanged, (e: any, args: { columns: any, grid: any }) => {
           this._areVisibleColumnDifferent = true;
           if (this.sharedService.gridOptions.gridMenu && typeof this.sharedService.gridOptions.gridMenu.onColumnsChanged === 'function') {
             this.sharedService.gridOptions.gridMenu.onColumnsChanged(e, args);
           }
+          if (args && Array.isArray(args.columns) && args.columns.length > this.sharedService.visibleColumns.length) {
+            this.sharedService.visibleColumns = args.columns;
+          }
         });
         this._eventHandler.subscribe(this._addon.onCommand, (e: any, args: any) => {
           this.executeGridMenuInternalCustomCommands(e, args);
@@ -206,11 +209,8 @@ export class GridMenuExtension implements Extension {
       // translate all columns (including non-visible)
       this.extensionUtility.translateItems(this.sharedService.allColumns, 'headerKey', 'name');
 
-      // re-initialize the Grid Menu, that will recreate all the menus & list
-      // doing an "init()" won't drop any existing command attached
-      if (this._addon.init) {
-        this._addon.init(this.sharedService.grid);
-      }
+      // update the Titles of each sections (command, customTitle, ...)
+      this._addon.updateAllTitles(this.sharedService.gridOptions.gridMenu);
     }
   }