fix(addon): providing columnIndexPosition should always work

- providing columnIndexPosition should work regardless of when the extension (plugin) is created, basically the previous code was assuming that you will always want the checkbox before the row move plugin but that is not always true and the other way around wasn't possible while this PR fixes it

examples/webpack-demo-vanilla-bundle/src/examples/example07.ts

@@ -196,11 +196,17 @@ export class Example7 {
         // True (Single Selection), False (Multiple Selections)
         selectActiveRow: false
       },
+      checkboxSelector: {
+        hideSelectAllCheckbox: false, // hide the "Select All" from title bar
+        columnIndexPosition: 1,
+        // row selection should only be usable & displayed on root level 0 (parent item) & grid isn't locked
+      },
       dataView: {
         syncGridSelection: true, // enable this flag so that the row selection follows the row even if we move it to another position
       },
       enableRowMoveManager: true,
       rowMoveManager: {
+        columnIndexPosition: 0,
         // when using Row Move + Row Selection, you want to move only a single row and we will enable the following flags so it doesn't cancel row selection
         singleRowMove: true,
         disableRowSelection: true,

packages/common/src/extensions/checkboxSelectorExtension.ts

@@ -42,7 +42,7 @@ export class CheckboxSelectorExtension implements Extension {
         selectionColumn.maxWidth = selectionColumn.width || 30;
 
         // column index position in the grid
-        const columnPosition = gridOptions?.checkboxSelector?.columnIndexPosition || 0;
+        const columnPosition = gridOptions?.checkboxSelector?.columnIndexPosition ?? 0;
         if (columnPosition > 0) {
           columnDefinitions.splice(columnPosition, 0, selectionColumn);
         } else {

packages/common/src/extensions/rowMoveManagerExtension.ts

@@ -64,7 +64,7 @@ export class RowMoveManagerExtension implements Extension {
       // only add the new column if it doesn't already exist
       if (!rowMoveColDef && finalRowMoveColumn) {
         // column index position in the grid
-        const columnPosition = gridOptions?.rowMoveManager?.columnIndexPosition || 0;
+        const columnPosition = gridOptions?.rowMoveManager?.columnIndexPosition ?? 0;
         if (columnPosition > 0) {
           columnDefinitions.splice(columnPosition, 0, finalRowMoveColumn);
         } else {

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

@@ -53,6 +53,11 @@ const extensionCellMenuStub = {
   ...extensionStub,
   translateCellMenu: jest.fn()
 };
+const extensionCheckboxSelectorStub = {
+  ...extensionStub,
+  selectRows: jest.fn(),
+  deSelectRows: jest.fn(),
+};
 const extensionContextMenuStub = {
   ...extensionStub,
   translateContextMenu: jest.fn()
@@ -65,6 +70,11 @@ const extensionHeaderMenuStub = {
   ...extensionStub,
   translateHeaderMenu: jest.fn()
 };
+const extensionRowMoveStub = {
+  ...extensionStub,
+  onBeforeMoveRows: jest.fn(),
+  onMoveRows: jest.fn()
+};
 
 describe('ExtensionService', () => {
   let sharedService: SharedService;
@@ -82,7 +92,7 @@ describe('ExtensionService', () => {
         extensionStub as unknown as AutoTooltipExtension,
         extensionStub as unknown as CellExternalCopyManagerExtension,
         extensionCellMenuStub as unknown as CellMenuExtension,
-        extensionStub as unknown as CheckboxSelectorExtension,
+        extensionCheckboxSelectorStub as unknown as CheckboxSelectorExtension,
         extensionColumnPickerStub as unknown as ColumnPickerExtension,
         extensionContextMenuStub as unknown as ContextMenuExtension,
         extensionStub as unknown as DraggableGroupingExtension,
@@ -91,7 +101,7 @@ describe('ExtensionService', () => {
         extensionHeaderButtonStub as unknown as HeaderButtonExtension,
         extensionHeaderMenuStub as unknown as HeaderMenuExtension,
         extensionStub as unknown as RowDetailViewExtension,
-        extensionStub as unknown as RowMoveManagerExtension,
+        extensionRowMoveStub as unknown as RowMoveManagerExtension,
         extensionStub as unknown as RowSelectionExtension,
         sharedService,
         translateService,
@@ -275,8 +285,8 @@ describe('ExtensionService', () => {
       it('should register the CheckboxSelector addon when "enableCheckboxSelector" is set in the grid options', () => {
         const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
         const gridOptionsMock = { enableCheckboxSelector: true } as GridOption;
-        const extCreateSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
-        const extRegisterSpy = jest.spyOn(extensionStub, 'register');
+        const extCreateSpy = jest.spyOn(extensionCheckboxSelectorStub, 'create').mockReturnValue(instanceMock);
+        const extRegisterSpy = jest.spyOn(extensionCheckboxSelectorStub, 'register');
         const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
 
         service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
@@ -288,7 +298,7 @@ describe('ExtensionService', () => {
         expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
         expect(extRegisterSpy).toHaveBeenCalled();
         expect(rowSelectionInstance).not.toBeNull();
-        expect(output).toEqual({ name: ExtensionName.checkboxSelector, instance: instanceMock as unknown, class: extensionStub } as ExtensionModel<any, any>);
+        expect(output).toEqual({ name: ExtensionName.checkboxSelector, instance: instanceMock as unknown, class: extensionCheckboxSelectorStub } as ExtensionModel<any, any>);
       });
 
       it('should register the RowDetailView addon when "enableRowDetailView" is set in the grid options', () => {
@@ -313,8 +323,8 @@ describe('ExtensionService', () => {
       it('should register the RowMoveManager addon when "enableRowMoveManager" is set in the grid options', () => {
         const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
         const gridOptionsMock = { enableRowMoveManager: true } as GridOption;
-        const extCreateSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
-        const extRegisterSpy = jest.spyOn(extensionStub, 'register');
+        const extCreateSpy = jest.spyOn(extensionRowMoveStub, 'create').mockReturnValue(instanceMock);
+        const extRegisterSpy = jest.spyOn(extensionRowMoveStub, 'register');
         const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
 
         service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
@@ -326,7 +336,7 @@ describe('ExtensionService', () => {
         expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
         expect(rowSelectionInstance).not.toBeNull();
         expect(extRegisterSpy).toHaveBeenCalled();
-        expect(output).toEqual({ name: ExtensionName.rowMoveManager, instance: instanceMock as unknown, class: extensionStub } as ExtensionModel<any, any>);
+        expect(output).toEqual({ name: ExtensionName.rowMoveManager, instance: instanceMock as unknown, class: extensionRowMoveStub } as ExtensionModel<any, any>);
       });
 
       it('should register the RowSelection addon when "enableCheckboxSelector" (false) and "enableRowSelection" (true) are set in the grid options', () => {
@@ -444,6 +454,28 @@ describe('ExtensionService', () => {
 
         expect(extSpy).toHaveBeenCalledWith(gridOptionsMock);
       });
+
+      it('should register the RowSelection & RowMoveManager addons with specific "columnIndexPosition" and expect these orders to be respected regardless of when the feature is enabled/created', () => {
+        const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }, { id: 'field2', field: 'field2', width: 50, }] as Column[];
+        const gridOptionsMock = {
+          enableCheckboxSelector: true, enableRowSelection: true,
+          checkboxSelector: { columnIndexPosition: 1 },
+          enableRowMoveManager: true,
+          rowMoveManager: { columnIndexPosition: 0 }
+        } as GridOption;
+        const checkboxInstanceMock = { selectRows: jest.fn(), deSelectRows: jest.fn() };
+        const rowMoveInstanceMock = { onBeforeMoveRows: jest.fn(), onMoveRows: jest.fn() };
+        const extCheckboxSpy = jest.spyOn(extensionCheckboxSelectorStub, 'create').mockReturnValue(checkboxInstanceMock);
+        const extRowMoveSpy = jest.spyOn(extensionRowMoveStub, 'create').mockReturnValue(rowMoveInstanceMock);
+
+        service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
+
+        expect(extRowMoveSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
+        expect(extCheckboxSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
+        // expect(extensionRowMoveStub.create).toHaveBeenCalledBefore(extensionCheckboxSelectorStub.create);
+        expect(extRowMoveSpy.mock.invocationCallOrder[0]).toBeLessThan(extCheckboxSpy.mock.invocationCallOrder[1]);
+        expect(columnsMock).toEqual([{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }, { id: 'field2', field: 'field2', width: 50, }]);
+      });
     });
 
     it('should call hideColumn and expect "visibleColumns" to be updated accordingly', () => {

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

@@ -24,6 +24,12 @@ import {
 import { SharedService } from './shared.service';
 import { TranslaterService } from './translater.service';
 
+interface ExtensionWithColumnIndexPosition {
+  name: ExtensionName;
+  position: number;
+  extension: CheckboxSelectorExtension | RowDetailViewExtension | RowMoveManagerExtension;
+}
+
 export class ExtensionService {
   private _extensionCreatedList: ExtensionList<any, any> = {} as ExtensionList<any, any>;
   private _extensionList: ExtensionList<any, any> = {} as ExtensionList<any, any>;
@@ -255,36 +261,37 @@ export class ExtensionService {
    * Bind/Create certain plugins before the Grid creation to avoid having odd behaviors.
    * Mostly because the column definitions might change after the grid creation, so we want to make sure to add it before then
    * @param columnDefinitions
-   * @param options
+   * @param gridOptions
    */
-  createExtensionsBeforeGridCreation(columnDefinitions: Column[], options: GridOption) {
-    if (options.enableCheckboxSelector) {
+  createExtensionsBeforeGridCreation(columnDefinitions: Column[], gridOptions: GridOption) {
+    const featureWithColumnIndexPositions: { name: ExtensionName; position: number; extension: CheckboxSelectorExtension | RowDetailViewExtension | RowMoveManagerExtension; }[] = [];
+
+    // the following 3 features might have `columnIndexPosition` that we need to respect their column order, we will execute them by their sort order further down
+    // we push them into a array and we'll process them by their position (if provided, else use same order that they were inserted)
+    if (gridOptions.enableCheckboxSelector) {
       if (!this.getCreatedExtensionByName(ExtensionName.checkboxSelector)) {
-        const checkboxInstance = this.checkboxSelectorExtension.create(columnDefinitions, options);
-        if (checkboxInstance) {
-          this._extensionCreatedList[ExtensionName.checkboxSelector] = { name: ExtensionName.checkboxSelector, instance: checkboxInstance, class: this.checkboxSelectorExtension };
-        }
+        featureWithColumnIndexPositions.push({ name: ExtensionName.checkboxSelector, extension: this.checkboxSelectorExtension, position: gridOptions?.checkboxSelector?.columnIndexPosition ?? featureWithColumnIndexPositions.length });
       }
     }
-    if (options.enableRowMoveManager) {
+    if (gridOptions.enableRowMoveManager) {
       if (!this.getCreatedExtensionByName(ExtensionName.rowMoveManager)) {
-        const rowMoveInstance = this.rowMoveManagerExtension.create(columnDefinitions, options);
-        if (rowMoveInstance) {
-          this._extensionCreatedList[ExtensionName.rowMoveManager] = { name: ExtensionName.rowMoveManager, instance: rowMoveInstance, class: this.rowMoveManagerExtension };
-        }
+        featureWithColumnIndexPositions.push({ name: ExtensionName.rowMoveManager, extension: this.rowMoveManagerExtension, position: gridOptions?.rowMoveManager?.columnIndexPosition ?? featureWithColumnIndexPositions.length });
       }
     }
-    if (options.enableRowDetailView) {
+    if (gridOptions.enableRowDetailView) {
       if (!this.getCreatedExtensionByName(ExtensionName.rowDetailView)) {
-        const rowDetailInstance = this.rowDetailViewExtension.create(columnDefinitions, options);
-        this._extensionCreatedList[ExtensionName.rowDetailView] = { name: ExtensionName.rowDetailView, instance: rowDetailInstance, class: this.rowDetailViewExtension };
+        featureWithColumnIndexPositions.push({ name: ExtensionName.rowDetailView, extension: this.rowDetailViewExtension, position: gridOptions?.rowDetailView?.columnIndexPosition ?? featureWithColumnIndexPositions.length });
       }
     }
-    if (options.enableDraggableGrouping) {
+
+    // since some features could have a `columnIndexPosition`, we need to make sure these indexes are respected in the column definitions
+    this.createExtensionByTheirColumnIndex(featureWithColumnIndexPositions, columnDefinitions, gridOptions);
+
+    if (gridOptions.enableDraggableGrouping) {
       if (!this.getCreatedExtensionByName(ExtensionName.draggableGrouping)) {
-        const draggableInstance = this.draggableGroupingExtension.create(options);
+        const draggableInstance = this.draggableGroupingExtension.create(gridOptions);
         if (draggableInstance) {
-          options.enableColumnReorder = draggableInstance.getSetupColumnReorder;
+          gridOptions.enableColumnReorder = draggableInstance.getSetupColumnReorder;
           this._extensionCreatedList[ExtensionName.draggableGrouping] = { name: ExtensionName.draggableGrouping, instance: draggableInstance, class: this.draggableGroupingExtension };
         }
       }
@@ -420,6 +427,27 @@ export class ExtensionService {
   // private functions
   // -------------------
 
+  /**
+   * Some extension (feature) have specific `columnIndexPosition` that the developer want to use, we need to make sure these indexes are respected in the column definitions in the order provided.
+   * The following 3 features could have that optional `columnIndexPosition` and we need to respect their column order, we will first sort by their optional order and only after we will create them by their specific order.
+   * We'll process them by their position (if provided, else use same order that they were inserted)
+   * @param featureWithIndexPositions
+   * @param columnDefinitions
+   * @param gridOptions
+   */
+  private createExtensionByTheirColumnIndex(featureWithIndexPositions: ExtensionWithColumnIndexPosition[], columnDefinitions: Column[], gridOptions: GridOption) {
+    // 1- first step is to sort them by their index position
+    featureWithIndexPositions.sort((feat1, feat2) => feat1.position - feat2.position);
+
+    // 2- second step, we can now proceed to create each extension/addon and that will position them accordingly in the column definitions list
+    featureWithIndexPositions.forEach(feature => {
+      const instance = feature.extension.create(columnDefinitions, gridOptions);
+      if (instance) {
+        this._extensionCreatedList[feature.name] = { name: feature.name, instance, class: feature.extension };
+      }
+    });
+  }
+
   /**
    * Get an Extension that was created by calling its "create" method (there are only 3 extensions which uses this method)
    *  @param name