fix(presets): dynamic columns should be auto-inserted with Grid Prese…

…ts (#938) - this PR fixes 2 issues 1. when Row Detail & Row Selection are used together, an extra checkbox appears in the filter section of the RowDetail column when it shouldn't (see print screen) 2. there are 3 dynamically created columns (RowMove, RowSelection & RowDetail) and all 3 should be auto-inserted when columns presets are used
ghiscoding · Mar 22, 2023 · 1f9c1c4 · 1f9c1c4
1 parent 7033fbf
commit 1f9c1c4
Show file tree

Hide file tree

Showing 8 changed files with 117 additions and 40 deletions.
diff --git a/packages/common/src/extensions/__tests__/slickCheckboxSelectColumn.spec.ts b/packages/common/src/extensions/__tests__/slickCheckboxSelectColumn.spec.ts
@@ -24,7 +24,7 @@ const addJQueryEventPropagation = function (event, commandKey = '', keyName = ''
     Object.defineProperty(event, 'target', { writable: true, configurable: true, value: target });
   }
   return event;
-}
+};
 
 const dataViewStub = {
   collapseAllGroups: jest.fn(),
@@ -36,7 +36,7 @@ const dataViewStub = {
   onPagingInfoChanged: new Slick.Event(),
   onSelectedRowIdsChanged: new Slick.Event(),
   setSelectedIds: jest.fn(),
-}
+};
 
 const getEditorLockMock = {
   commitCurrentEdit: jest.fn(),
@@ -123,7 +123,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
       applySelectOnAllPages: true,
       columnId: '_checkbox_selector',
       cssClass: null,
-      field: 'sel',
+      field: '_checkbox_selector',
       hideSelectAllCheckbox: false,
       toolTip: 'Select/Deselect All',
       width: 30,
@@ -146,7 +146,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
       applySelectOnAllPages: true,
       columnId: '_checkbox_selector',
       cssClass: 'some-class',
-      field: 'sel',
+      field: '_checkbox_selector',
       hideSelectAllCheckbox: true,
       toolTip: 'Select/Deselect All',
       width: 30,
@@ -180,7 +180,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
     const clickEvent = addJQueryEventPropagation(new Event('click'), '', '', checkboxElm);
     const preventDefaultSpy = jest.spyOn(clickEvent, 'preventDefault');
     const stopImmediatePropagationSpy = jest.spyOn(clickEvent, 'stopImmediatePropagation');
-    gridStub.onHeaderClick.notify({ column: { id: '_checkbox_selector', field: 'sel' }, grid: gridStub }, clickEvent);
+    gridStub.onHeaderClick.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, grid: gridStub }, clickEvent);
 
     expect(plugin).toBeTruthy();
     expect(updateColHeaderSpy).toHaveBeenCalledWith(
@@ -215,7 +215,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
     const clickEvent = addJQueryEventPropagation(new Event('click'), '', '', checkboxElm);
     const stopPropagationSpy = jest.spyOn(clickEvent, 'stopPropagation');
     const stopImmediatePropagationSpy = jest.spyOn(clickEvent, 'stopImmediatePropagation');
-    gridStub.onHeaderClick.notify({ column: { id: '_checkbox_selector', field: 'sel' }, grid: gridStub }, clickEvent);
+    gridStub.onHeaderClick.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, grid: gridStub }, clickEvent);
 
     expect(plugin).toBeTruthy();
     expect(stopPropagationSpy).toHaveBeenCalled();
@@ -249,7 +249,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
     const clickEvent = addJQueryEventPropagation(new Event('click'), '', '', checkboxElm);
     const stopPropagationSpy = jest.spyOn(clickEvent, 'stopPropagation');
     const stopImmediatePropagationSpy = jest.spyOn(clickEvent, 'stopImmediatePropagation');
-    gridStub.onHeaderClick.notify({ column: { id: '_checkbox_selector', field: 'sel' }, grid: gridStub }, clickEvent);
+    gridStub.onHeaderClick.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, grid: gridStub }, clickEvent);
 
     expect(plugin).toBeTruthy();
     expect(stopPropagationSpy).toHaveBeenCalled();
@@ -267,7 +267,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
 
     plugin = new SlickCheckboxSelectColumn(pubSubServiceStub, { hideInFilterHeaderRow: false, hideSelectAllCheckbox: false, });
     plugin.init(gridStub);
-    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: 'sel' }, node: nodeElm, grid: gridStub });
+    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, node: nodeElm, grid: gridStub });
     plugin.setOptions({ hideInColumnTitleRow: true, hideInFilterHeaderRow: false, hideSelectAllCheckbox: false, });
     let filterSelectAll = plugin.headerRowNode!.querySelector(`#filter-checkbox-selectall-container`) as HTMLSpanElement;
 
@@ -386,7 +386,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
       id: '_checkbox_selector',
       name: `<input id="header-selector${plugin.selectAllUid}" type="checkbox"><label for="header-selector${plugin.selectAllUid}"></label>`,
       toolTip: 'Select/Deselect All',
-      field: 'sel',
+      field: '_checkbox_selector',
       cssClass: null,
       excludeFromExport: true,
       excludeFromColumnPicker: true,
@@ -409,7 +409,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
     plugin = new SlickCheckboxSelectColumn(pubSubServiceStub, { applySelectOnAllPages: false, hideInFilterHeaderRow: false, });
     plugin.init(gridStub);
 
-    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: 'sel' }, node: nodeElm, grid: gridStub });
+    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, node: nodeElm, grid: gridStub });
     const checkboxContainerElm = nodeElm.querySelector('span#filter-checkbox-selectall-container') as HTMLDivElement;
     const inputCheckboxElm = checkboxContainerElm.querySelector('input[type=checkbox]') as HTMLDivElement;
     inputCheckboxElm.dispatchEvent(new Event('click', { bubbles: true, cancelable: true }));
@@ -427,7 +427,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
       excludeFromGridMenu: true,
       excludeFromHeaderMenu: true,
       excludeFromQuery: true,
-      field: 'sel',
+      field: 'chk-id',
       hideSelectAllCheckbox: false,
       id: 'chk-id',
       name: `<input id="header-selector${plugin.selectAllUid}" type="checkbox"><label for="header-selector${plugin.selectAllUid}"></label>`,
@@ -455,7 +455,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
       excludeFromGridMenu: true,
       excludeFromHeaderMenu: true,
       excludeFromQuery: true,
-      field: 'sel',
+      field: '_checkbox_selector',
       formatter: expect.toBeFunction(),
       hideSelectAllCheckbox: false,
       id: '_checkbox_selector',
@@ -488,7 +488,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
 
   it('should trigger "onClick" event and expect toggleRowSelection to be called', () => {
     const newCols = [
-      { id: '_checkbox_selector', toolTip: 'Select/Deselect All', field: 'sel', },
+      { id: '_checkbox_selector', toolTip: 'Select/Deselect All', field: '_checkbox_selector', },
       { id: 'firstName', field: 'firstName', name: 'First Name', },
     ];
     const toggleRowSpy = jest.spyOn(plugin, 'toggleRowSelectionWithEvent');
@@ -588,7 +588,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
 
   it('should trigger "onKeyDown" event and expect toggleRowSelection to be called when editor "commitCurrentEdit" returns True', () => {
     const newCols = [
-      { id: '_checkbox_selector', toolTip: 'Select/Deselect All', field: 'sel', },
+      { id: '_checkbox_selector', toolTip: 'Select/Deselect All', field: '_checkbox_selector', },
       { id: 'firstName', field: 'firstName', name: 'First Name', },
     ];
     const toggleRowSpy = jest.spyOn(plugin, 'toggleRowSelectionWithEvent');
@@ -673,7 +673,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
     plugin.init(gridStub);
     plugin.selectedRowsLookup = { 1: false, 2: true };
 
-    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: 'sel' }, node: nodeElm, grid: gridStub });
+    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, node: nodeElm, grid: gridStub });
 
     const checkboxElm = document.createElement('input');
     checkboxElm.type = 'checkbox';
@@ -708,7 +708,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
     plugin.init(gridStub);
     plugin.selectedRowsLookup = { 1: false, 2: true };
 
-    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: 'sel' }, node: nodeElm, grid: gridStub });
+    gridStub.onHeaderRowCellRendered.notify({ column: { id: '_checkbox_selector', field: '_checkbox_selector' }, node: nodeElm, grid: gridStub });
 
     const checkboxElm = document.createElement('input');
     checkboxElm.type = 'checkbox';

diff --git a/packages/common/src/extensions/__tests__/slickRowMoveManager.spec.ts b/packages/common/src/extensions/__tests__/slickRowMoveManager.spec.ts
@@ -15,7 +15,7 @@ const addJQueryEventPropagation = function (event, target?: HTMLElement) {
     Object.defineProperty(event, 'target', { writable: true, configurable: true, value: target });
   }
   return event;
-}
+};
 
 const mockGridOptions = {
   frozenColumn: 1,
@@ -145,7 +145,7 @@ describe('SlickRowMoveManager Plugin', () => {
       excludeFromGridMenu: true,
       excludeFromHeaderMenu: true,
       excludeFromQuery: true,
-      field: 'move',
+      field: 'move-id',
       formatter: expect.toBeFunction(),
       id: 'move-id',
       name: '',
@@ -194,7 +194,7 @@ describe('SlickRowMoveManager Plugin', () => {
       excludeFromGridMenu: true,
       excludeFromHeaderMenu: true,
       excludeFromQuery: true,
-      field: 'move',
+      field: 'move-id',
       formatter: expect.toBeFunction(),
       id: 'move-id',
       name: '',

diff --git a/packages/common/src/extensions/slickCheckboxSelectColumn.ts b/packages/common/src/extensions/slickCheckboxSelectColumn.ts
@@ -14,7 +14,7 @@ export class SlickCheckboxSelectColumn<T = any> {
   protected _defaults = {
     columnId: '_checkbox_selector',
     cssClass: null,
-    field: 'sel',
+    field: '_checkbox_selector',
     hideSelectAllCheckbox: false,
     toolTip: 'Select/Deselect All',
     width: 30,
@@ -199,11 +199,13 @@ export class SlickCheckboxSelectColumn<T = any> {
   }
 
   getColumnDefinition(): Column {
+    const columnId = String(this._addonOptions?.columnId ?? this._defaults.columnId);
+
     return {
-      id: this._addonOptions.columnId,
+      id: columnId,
       name: (this._addonOptions.hideSelectAllCheckbox || this._addonOptions.hideInColumnTitleRow) ? '' : `<input id="header-selector${this._selectAll_UID}" type="checkbox"><label for="header-selector${this._selectAll_UID}"></label>`,
       toolTip: (this._addonOptions.hideSelectAllCheckbox || this._addonOptions.hideInColumnTitleRow) ? '' : this._addonOptions.toolTip,
-      field: this._addonOptions.field || 'sel',
+      field: columnId,
       cssClass: this._addonOptions.cssClass,
       excludeFromExport: true,
       excludeFromColumnPicker: true,
@@ -281,7 +283,7 @@ export class SlickCheckboxSelectColumn<T = any> {
 
   protected addCheckboxToFilterHeaderRow(grid: SlickGrid) {
     this._eventHandler.subscribe(grid.onHeaderRowCellRendered, (_e: any, args: any) => {
-      if (args.column.field === (this._addonOptions.field || 'sel')) {
+      if (args.column.field === (this._addonOptions.field || '_checkbox_selector')) {
         emptyElement(args.node);
 
         // <span class="container"><input type="checkbox"><label for="checkbox"></label></span>

diff --git a/packages/common/src/extensions/slickRowMoveManager.ts b/packages/common/src/extensions/slickRowMoveManager.ts
@@ -127,8 +127,10 @@ export class SlickRowMoveManager {
   }
 
   getColumnDefinition(): Column {
+    const columnId = String(this._addonOptions?.columnId ?? this._defaults.columnId);
+
     return {
-      id: this._addonOptions.columnId || '_move',
+      id: columnId,
       name: '',
       behavior: 'selectAndMove',
       cssClass: this._addonOptions.cssClass,
@@ -137,7 +139,7 @@ export class SlickRowMoveManager {
       excludeFromGridMenu: true,
       excludeFromQuery: true,
       excludeFromHeaderMenu: true,
-      field: 'move',
+      field: columnId,
       resizable: false,
       selectable: false,
       width: this._addonOptions.width || 40,

diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts
@@ -205,7 +205,7 @@ describe('SlickRowDetailView plugin', () => {
     const processMock = jest.fn();
     const overrideMock = jest.fn();
     const rowDetailColumnMock = {
-      id: '_detail_', field: 'sel', name: '', alwaysRenderColumn: true, cssClass: 'some-class',
+      id: '_detail_', field: '_detail_', name: '', alwaysRenderColumn: true, cssClass: 'some-class',
       excludeFromExport: true, excludeFromColumnPicker: true, excludeFromGridMenu: true, excludeFromQuery: true, excludeFromHeaderMenu: true,
       formatter: expect.anything(),
       resizable: false, sortable: false, toolTip: 'title', width: 30,
@@ -225,7 +225,7 @@ describe('SlickRowDetailView plugin', () => {
     const output = plugin.create(mockColumns, { rowDetailView: { process: processMock, columnIndexPosition: columnIndex, panelRows: 4, columnId: '_detail_', cssClass: 'some-class', toolTip: 'title' } });
 
     expect(mockColumns[columnIndex]).toEqual({
-      id: '_detail_', field: 'sel', name: '', alwaysRenderColumn: true, cssClass: 'some-class',
+      id: '_detail_', field: '_detail_', name: '', alwaysRenderColumn: true, cssClass: 'some-class',
       excludeFromExport: true, excludeFromColumnPicker: true, excludeFromGridMenu: true, excludeFromQuery: true, excludeFromHeaderMenu: true,
       formatter: expect.anything(),
       resizable: false, sortable: false, toolTip: 'title', width: 30,

diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.ts
@@ -43,6 +43,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
   protected _defaults = {
     alwaysRenderColumn: true,
     columnId: '_detail_selector',
+    field: '_detail_selector',
     cssClass: 'detailView-toggle',
     collapseAllOnSort: true,
     collapsedClass: null,
@@ -362,9 +363,11 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
 
   /** Get the Column Definition of the first column dedicated to toggling the Row Detail View */
   getColumnDefinition(): Column {
+    const columnId = String(this._addonOptions?.columnId ?? this._defaults.columnId);
+
     return {
-      id: this._addonOptions?.columnId ?? this._defaults.columnId as string | number,
-      field: 'sel',
+      id: columnId,
+      field: columnId,
       name: '',
       alwaysRenderColumn: this._addonOptions?.alwaysRenderColumn,
       cssClass: this._addonOptions.cssClass || '',

diff --git a/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts b/packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts
@@ -397,7 +397,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
     const columnsMock = [
       { id: 'firstName', field: 'firstName', editor: undefined, internalColumnEditor: {} },
       { id: 'lastName', field: 'lastName', editor: undefined, internalColumnEditor: {} }
-    ]
+    ];
     eventPubSubService.publish('onPluginColumnsChanged', {
       columns: columnsMock,
       pluginName: 'RowMoveManager'
@@ -1419,6 +1419,56 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
         expect(setColSpy).toHaveBeenCalledWith(mockCols);
       });
 
+      it('should reflect columns with an extra row detail column in the grid when "enableRowDetailView" is set', () => {
+        const mockColsPresets = [{ columnId: 'firstName', width: 100 }];
+        const mockCol = { id: 'firstName', field: 'firstName' };
+        const mockCols = [{ id: '_detail_selector', field: '_detail_selector', editor: undefined, internalColumnEditor: {} }, mockCol];
+        const getAssocColSpy = jest.spyOn(gridStateServiceStub, 'getAssociatedGridColumns').mockReturnValue([mockCol]);
+        const setColSpy = jest.spyOn(mockGrid, 'setColumns');
+
+        component.columnDefinitions = mockCols;
+        component.gridOptions = { ...gridOptions, enableRowDetailView: true, presets: { columns: mockColsPresets } } as unknown as GridOption;
+        component.initialization(divContainer, slickEventHandler);
+
+        expect(getAssocColSpy).toHaveBeenCalledWith(mockGrid, mockColsPresets);
+        expect(setColSpy).toHaveBeenCalledWith(mockCols);
+      });
+
+      it('should reflect columns with an extra row move column in the grid when "enableRowMoveManager" is set', () => {
+        const mockColsPresets = [{ columnId: 'firstName', width: 100 }];
+        const mockCol = { id: 'firstName', field: 'firstName' };
+        const mockCols = [{ id: '_move', field: '_move', editor: undefined, internalColumnEditor: {} }, mockCol];
+        const getAssocColSpy = jest.spyOn(gridStateServiceStub, 'getAssociatedGridColumns').mockReturnValue([mockCol]);
+        const setColSpy = jest.spyOn(mockGrid, 'setColumns');
+
+        component.columnDefinitions = mockCols;
+        component.gridOptions = { ...gridOptions, enableRowMoveManager: true, presets: { columns: mockColsPresets } } as unknown as GridOption;
+        component.initialization(divContainer, slickEventHandler);
+
+        expect(getAssocColSpy).toHaveBeenCalledWith(mockGrid, mockColsPresets);
+        expect(setColSpy).toHaveBeenCalledWith(mockCols);
+      });
+
+      it('should reflect 3 dynamic columns (1-RowMove, 2-RowSelection, 3-RowDetail) when all associated extension flags are enabled', () => {
+        const mockColsPresets = [{ columnId: 'firstName', width: 100 }];
+        const mockCol = { id: 'firstName', field: 'firstName' };
+        const mockCols = [
+          { id: '_move', field: '_move', editor: undefined, internalColumnEditor: {} },
+          { id: '_checkbox_selector', field: '_checkbox_selector', editor: undefined, internalColumnEditor: {} },
+          { id: '_detail_selector', field: '_detail_selector', editor: undefined, internalColumnEditor: {} },
+          mockCol
+        ];
+        const getAssocColSpy = jest.spyOn(gridStateServiceStub, 'getAssociatedGridColumns').mockReturnValue([mockCol]);
+        const setColSpy = jest.spyOn(mockGrid, 'setColumns');
+
+        component.columnDefinitions = mockCols;
+        component.gridOptions = { ...gridOptions, enableCheckboxSelector: true, enableRowDetailView: true, enableRowMoveManager: true, presets: { columns: mockColsPresets } } as unknown as GridOption;
+        component.initialization(divContainer, slickEventHandler);
+
+        expect(getAssocColSpy).toHaveBeenCalledWith(mockGrid, mockColsPresets);
+        expect(setColSpy).toHaveBeenCalledWith(mockCols);
+      });
+
       it('should execute backend service "init" method when set', () => {
         const mockPagination = { pageNumber: 1, pageSizes: [10, 25, 50], pageSize: 10, totalItems: 100 };
         const mockGraphqlOptions = { datasetName: 'users', extraQueryArguments: [{ field: 'userId', value: 123 }] } as GraphqlServiceOption;