Skip to content

Commit

Permalink
Merge pull request #794 from ghiscoding/bugfix/preserve-extension-col…
Browse files Browse the repository at this point in the history 
…umn-index-position

fix(addon): providing columnIndexPosition should always work
  • Loading branch information
@ghiscoding
ghiscoding authored Jun 17, 2021
2 parents 37b0ae0 + 4ff9935 commit 1bb0358
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 24 deletions.
4 changes: 3 additions & 1 deletion src/app/examples/grid-rowmove.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
48 changes: 40 additions & 8 deletions src/app/modules/angular-slickgrid/services/__tests__/extension.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -71,6 +76,11 @@ const extensionHeaderMenuStub = {
...extensionStub,
translateHeaderMenu: jest.fn()
};
const extensionRowMoveStub = {
...extensionStub,
onBeforeMoveRows: jest.fn(),
onMoveRows: jest.fn()
};

describe('ExtensionService', () => {
let service: ExtensionService;
Expand All @@ -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()]
Expand Down Expand Up @@ -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);
Expand All @@ -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', () => {
Expand All @@ -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);
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
64 changes: 49 additions & 15 deletions src/app/modules/angular-slickgrid/services/extension.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
}
}
}
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1bb0358

Please sign in to comment.