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

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

@@ -70,6 +70,8 @@ export class GridRowMoveComponent implements OnInit {
       enableFiltering: true,
       enableCheckboxSelector: true,
       checkboxSelector: {
+        columnIndexPosition: 1,
+
         // you can toggle these 2 properties to show the "select all" checkbox in different location
         hideInFilterHeaderRow: false,
         hideInColumnTitleRow: true
@@ -95,7 +97,7 @@ export class GridRowMoveComponent implements OnInit {
         // you can change the move icon position of any extension (RowMove, RowDetail or RowSelector icon)
         // note that you might have to play with the position when using multiple extension
         // since it really depends on which extension get created first to know what their real position are
-        // columnIndexPosition: 1,
+        columnIndexPosition: 0,
 
         // you can also override the usability of the rows, for example make every 2nd row the only moveable rows,
         // usabilityOverride: (row, dataContext, grid) => dataContext.id % 2 === 1

src/app/modules/angular-slickgrid/services/__tests__/extension.service.spec.ts

@@ -59,6 +59,11 @@ const extensionCellMenuStub = {
   ...extensionStub,
   translateCellMenu: jest.fn()
 };
+const extensionCheckboxSelectorStub = {
+  ...extensionStub,
+  selectRows: jest.fn(),
+  deSelectRows: jest.fn(),
+};
 const extensionContextMenuStub = {
   ...extensionStub,
   translateContextMenu: jest.fn()
@@ -71,6 +76,11 @@ const extensionHeaderMenuStub = {
   ...extensionStub,
   translateHeaderMenu: jest.fn()
 };
+const extensionRowMoveStub = {
+  ...extensionStub,
+  onBeforeMoveRows: jest.fn(),
+  onMoveRows: jest.fn()
+};
 
 describe('ExtensionService', () => {
   let service: ExtensionService;
@@ -91,15 +101,15 @@ describe('ExtensionService', () => {
           { provide: ColumnPickerExtension, useValue: extensionColumnPickerStub },
           { provide: CellExternalCopyManagerExtension, useValue: extensionStub },
           { provide: CellMenuExtension, useValue: extensionCellMenuStub },
-          { provide: CheckboxSelectorExtension, useValue: extensionStub },
+          { provide: CheckboxSelectorExtension, useValue: extensionCheckboxSelectorStub },
           { provide: ContextMenuExtension, useValue: extensionContextMenuStub },
           { provide: DraggableGroupingExtension, useValue: extensionStub },
           { provide: GridMenuExtension, useValue: extensionGridMenuStub },
           { provide: GroupItemMetaProviderExtension, useValue: extensionGroupItemMetaStub },
           { provide: HeaderButtonExtension, useValue: extensionHeaderButtonStub },
           { provide: HeaderMenuExtension, useValue: extensionHeaderMenuStub },
           { provide: RowDetailViewExtension, useValue: extensionStub },
-          { provide: RowMoveManagerExtension, useValue: extensionStub },
+          { provide: RowMoveManagerExtension, useValue: extensionRowMoveStub },
           { provide: RowSelectionExtension, useValue: extensionStub },
         ],
         imports: [TranslateModule.forRoot()]
@@ -279,8 +289,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);
@@ -292,7 +302,7 @@ describe('ExtensionService', () => {
         expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
         expect(extRegisterSpy).toHaveBeenCalled();
         expect(rowSelectionInstance).not.toBeNull();
-        expect(output).toEqual({ name: ExtensionName.checkboxSelector, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel);
+        expect(output).toEqual({ name: ExtensionName.checkboxSelector, addon: instanceMock, instance: instanceMock, class: extensionCheckboxSelectorStub } as ExtensionModel);
       });
 
       it('should register the RowDetailView addon when "enableRowDetailView" is set in the grid options', () => {
@@ -317,8 +327,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);
@@ -330,7 +340,7 @@ describe('ExtensionService', () => {
         expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
         expect(rowSelectionInstance).not.toBeNull();
         expect(extRegisterSpy).toHaveBeenCalled();
-        expect(output).toEqual({ name: ExtensionName.rowMoveManager, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel);
+        expect(output).toEqual({ name: ExtensionName.rowMoveManager, addon: instanceMock, instance: instanceMock, class: extensionRowMoveStub } as ExtensionModel);
       });
 
       it('should register the RowSelection addon when "enableCheckboxSelector" (false) and "enableRowSelection" (true) are set in the grid options', () => {
@@ -483,6 +493,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', () => {

src/app/modules/angular-slickgrid/services/extension.service.ts

@@ -31,6 +31,12 @@ import {
 } from '../extensions/index';
 import { SharedService } from './shared.service';
 
+interface ExtensionWithColumnIndexPosition {
+  name: ExtensionName;
+  position: number;
+  extension: CheckboxSelectorExtension | RowDetailViewExtension | RowMoveManagerExtension;
+}
+
 @Injectable()
 export class ExtensionService {
   private _extensionCreatedList: ExtensionList = {} as ExtensionList;
@@ -277,32 +283,39 @@ 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);
-        this._extensionCreatedList[ExtensionName.checkboxSelector] = { name: ExtensionName.checkboxSelector, addon: checkboxInstance, 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);
-        this._extensionCreatedList[ExtensionName.rowMoveManager] = { name: ExtensionName.rowMoveManager, addon: rowMoveInstance, 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, addon: rowDetailInstance, 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);
-        options.enableColumnReorder = draggableInstance.getSetupColumnReorder;
-        this._extensionCreatedList[ExtensionName.draggableGrouping] = { name: ExtensionName.draggableGrouping, addon: draggableInstance, instance: draggableInstance, class: draggableInstance.getSetupColumnReorder };
+        const draggableInstance = this.draggableGroupingExtension.create(gridOptions);
+        if (draggableInstance) {
+          gridOptions.enableColumnReorder = draggableInstance.getSetupColumnReorder;
+          this._extensionCreatedList[ExtensionName.draggableGrouping] = { name: ExtensionName.draggableGrouping, addon: draggableInstance, instance: draggableInstance, class: this.draggableGroupingExtension };
+        }
       }
     }
   }
@@ -439,6 +452,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 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)
+   * @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, addon: instance, 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