From 149dd201e6627290c8ee61b0485fe8e6e6cb7fa2 Mon Sep 17 00:00:00 2001 From: Ghislain Beaulac Date: Mon, 29 Jul 2019 13:18:39 -0400 Subject: [PATCH] fix(selection): selected row should be none after filtering, closes #249 - this was a regression bug, cannot pinpoint exactly since when this regression bug started - the isssue, row selection and filtering are completely independent, they don't talk to each other and the filtering as no clue which row it was selected prior to the filter, so the best we can do is to remove any row selection after typing a search filter (filtering) - update Jest unit tests & add Cypress E2E tests to cover this in the UI as well --- .../examples/grid-rowselection.component.html | 4 +- .../__tests__/extension.service.spec.ts | 24 +++++--- .../services/extension.service.ts | 57 +++++++++++++----- test/cypress/integration/example10.spec.js | 59 +++++++++++++++++++ 4 files changed, 119 insertions(+), 25 deletions(-) create mode 100644 test/cypress/integration/example10.spec.js diff --git a/src/app/examples/grid-rowselection.component.html b/src/app/examples/grid-rowselection.component.html index bf42ee7c1..f3eb5160f 100644 --- a/src/app/examples/grid-rowselection.component.html +++ b/src/app/examples/grid-rowselection.component.html @@ -6,7 +6,7 @@

{{title}}

(single select) Selected Row: - +
@@ -27,7 +27,7 @@

{{title}}

(multi-select) Selected Row(s): - +
diff --git a/src/app/modules/angular-slickgrid/services/__tests__/extension.service.spec.ts b/src/app/modules/angular-slickgrid/services/__tests__/extension.service.spec.ts index 39abfad74..08cda0e4b 100644 --- a/src/app/modules/angular-slickgrid/services/__tests__/extension.service.spec.ts +++ b/src/app/modules/angular-slickgrid/services/__tests__/extension.service.spec.ts @@ -259,29 +259,40 @@ 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 extSpy = jest.spyOn(extensionStub, 'register').mockReturnValue(instanceMock); + const extCreateSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock); + const extRegisterSpy = jest.spyOn(extensionStub, 'register'); const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock); + service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock); service.bindDifferentExtensions(); + const rowSelectionInstance = service.getExtensionByName(ExtensionName.rowSelection); const output = service.getExtensionByName(ExtensionName.checkboxSelector); expect(gridSpy).toHaveBeenCalled(); - expect(extSpy).toHaveBeenCalled(); + 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); }); it('should register the RowDetailView addon when "enableRowDetailView" is set in the grid options', () => { + const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[]; const gridOptionsMock = { enableRowDetailView: true } as GridOption; - const extSpy = jest.spyOn(extensionStub, 'register').mockReturnValue(instanceMock); + const extCreateSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock); + const extRegisterSpy = jest.spyOn(extensionStub, 'register'); const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock); + service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock); service.bindDifferentExtensions(); + const rowSelectionInstance = service.getExtensionByName(ExtensionName.rowSelection); const output = service.getExtensionByName(ExtensionName.rowDetailView); expect(gridSpy).toHaveBeenCalled(); - expect(extSpy).toHaveBeenCalled(); - expect(output).toEqual({ name: ExtensionName.rowDetailView, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel); + expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock); + expect(rowSelectionInstance).not.toBeNull(); + expect(extRegisterSpy).toHaveBeenCalled(); expect(output).toEqual({ name: ExtensionName.rowDetailView, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel); }); it('should register the RowMoveManager addon when "enableRowMoveManager" is set in the grid options', () => { @@ -395,7 +406,6 @@ describe('ExtensionService', () => { const gridOptionsMock = { enableCheckboxSelector: true } as GridOption; const extSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock); - service.bindDifferentExtensions(); service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock); expect(extSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock); @@ -406,7 +416,6 @@ describe('ExtensionService', () => { const gridOptionsMock = { enableRowDetailView: true } as GridOption; const extSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock); - service.bindDifferentExtensions(); service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock); expect(extSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock); @@ -417,7 +426,6 @@ describe('ExtensionService', () => { const gridOptionsMock = { enableDraggableGrouping: true } as GridOption; const extSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock); - service.bindDifferentExtensions(); service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock); expect(extSpy).toHaveBeenCalledWith(gridOptionsMock); diff --git a/src/app/modules/angular-slickgrid/services/extension.service.ts b/src/app/modules/angular-slickgrid/services/extension.service.ts index c662a2107..f58f8fdc0 100644 --- a/src/app/modules/angular-slickgrid/services/extension.service.ts +++ b/src/app/modules/angular-slickgrid/services/extension.service.ts @@ -29,6 +29,7 @@ import { SharedService } from './shared.service'; @Injectable() export class ExtensionService { + private _extensionCreatedList: any[] = []; private _extensionList: ExtensionModel[] = []; constructor( @@ -82,7 +83,7 @@ export class ExtensionService { * @param name */ getExtensionByName(name: ExtensionName): ExtensionModel | undefined { - return this._extensionList.find((p) => p.name === name); + return Array.isArray(this._extensionList) && this._extensionList.find((p) => p.name === name); } /** @@ -131,11 +132,22 @@ export class ExtensionService { } } + // Row Selection Plugin + // this extension should be registered BEFORE the Checkbox Selector & Row Detail since it can be use by these 2 plugins + if (!this.getExtensionByName(ExtensionName.rowSelection) && (this.sharedService.gridOptions.enableRowSelection || this.sharedService.gridOptions.enableCheckboxSelector || this.sharedService.gridOptions.enableRowDetailView)) { + if (this.rowSelectionExtension && this.rowSelectionExtension.register) { + const instance = this.rowSelectionExtension.register(); + this._extensionList.push({ name: ExtensionName.rowSelection, class: this.rowSelectionExtension, addon: instance, instance }); + } + } + // Checkbox Selector Plugin if (this.sharedService.gridOptions.enableCheckboxSelector) { if (this.checkboxSelectorExtension && this.checkboxSelectorExtension.register) { const rowSelectionExtension = this.getExtensionByName(ExtensionName.rowSelection); - const instance = this.checkboxSelectorExtension.register(rowSelectionExtension); + this.checkboxSelectorExtension.register(rowSelectionExtension); + const createdExtension = this.getCreatedExtensionByName(ExtensionName.checkboxSelector); // get the instance from when it was really created earlier + const instance = createdExtension && createdExtension.instance; this._extensionList.push({ name: ExtensionName.checkboxSelector, class: this.checkboxSelectorExtension, addon: instance, instance }); } } @@ -193,7 +205,9 @@ export class ExtensionService { if (this.sharedService.gridOptions.enableRowDetailView) { if (this.rowDetailViewExtension && this.rowDetailViewExtension.register) { const rowSelectionExtension = this.getExtensionByName(ExtensionName.rowSelection); - const instance = this.rowDetailViewExtension.register(rowSelectionExtension); + this.rowDetailViewExtension.register(rowSelectionExtension); + const createdExtension = this.getCreatedExtensionByName(ExtensionName.rowDetailView); // get the plugin from when it was really created earlier + const instance = createdExtension && createdExtension.instance; this._extensionList.push({ name: ExtensionName.rowDetailView, class: this.rowDetailViewExtension, addon: instance, instance }); } } @@ -206,14 +220,6 @@ export class ExtensionService { } } - // Row Selection Plugin - if (!this.sharedService.gridOptions.enableCheckboxSelector && this.sharedService.gridOptions.enableRowSelection) { - if (this.rowSelectionExtension && this.rowSelectionExtension.register) { - const instance = this.rowSelectionExtension.register(); - this._extensionList.push({ name: ExtensionName.rowSelection, class: this.rowSelectionExtension, addon: instance, instance }); - } - } - // manually register other plugins if (this.sharedService.gridOptions.registerPlugins !== undefined) { if (Array.isArray(this.sharedService.gridOptions.registerPlugins)) { @@ -239,14 +245,23 @@ export class ExtensionService { */ createExtensionsBeforeGridCreation(columnDefinitions: Column[], options: GridOption) { if (options.enableCheckboxSelector) { - this.checkboxSelectorExtension.create(columnDefinitions, options); + if (!this.getCreatedExtensionByName(ExtensionName.checkboxSelector)) { + const checkboxInstance = this.checkboxSelectorExtension.create(columnDefinitions, options); + this._extensionCreatedList.push({ name: ExtensionName.checkboxSelector, instance: checkboxInstance }); + } } if (options.enableRowDetailView) { - this.rowDetailViewExtension.create(columnDefinitions, options); + if (!this.getCreatedExtensionByName(ExtensionName.rowDetailView)) { + const rowDetailInstance = this.rowDetailViewExtension.create(columnDefinitions, options); + this._extensionCreatedList.push({ name: ExtensionName.rowDetailView, instance: rowDetailInstance }); + } } if (options.enableDraggableGrouping) { - const plugin = this.draggableGroupingExtension.create(options); - options.enableColumnReorder = plugin.getSetupColumnReorder; + if (!this.getCreatedExtensionByName(ExtensionName.rowDetailView)) { + const draggableInstance = this.draggableGroupingExtension.create(options); + options.enableColumnReorder = draggableInstance.getSetupColumnReorder; + this._extensionCreatedList.push({ name: ExtensionName.draggableGrouping, instance: draggableInstance }); + } } } @@ -350,6 +365,18 @@ export class ExtensionService { } } + // + // private functions + // ------------------- + + /** + * Get an Extension that was created by calling its "create" method (there are only 3 extensions which uses this method) + * @param name + */ + private getCreatedExtensionByName(name: ExtensionName): ExtensionModel | undefined { + return Array.isArray(this._extensionCreatedList) && this._extensionCreatedList.find((p) => p.name === name); + } + /** Translate an array of items from an input key and assign translated value to the output key */ private translateItems(items: any[], inputKey: string, outputKey: string) { if (Array.isArray(items)) { diff --git a/test/cypress/integration/example10.spec.js b/test/cypress/integration/example10.spec.js new file mode 100644 index 000000000..97368c49e --- /dev/null +++ b/test/cypress/integration/example10.spec.js @@ -0,0 +1,59 @@ +describe('Example 10 - Multiple Grids with Row Selection', () => { + const titles = ['', 'Title', 'Duration (days)', '% Complete', 'Start', 'Finish', 'Effort Driven']; + + it('should display Example 10 title', () => { + cy.visit(`${Cypress.config('baseExampleUrl')}/selection`); + cy.get('h2').should('contain', 'Example 10: Multiple Grids with Row Selection'); + }); + + it('should have 2 grids of size 800 by 200px', () => { + cy.get('#slickGridContainer-grid1') + .should('have.css', 'width', '800px'); + + cy.get('#slickGridContainer-grid1 > .slickgrid-container') + .should('have.css', 'height', '200px'); + + cy.get('#slickGridContainer-grid2') + .should('have.css', 'width', '800px'); + + cy.get('#slickGridContainer-grid2 > .slickgrid-container') + .should('have.css', 'height', '200px'); + }); + + it('should have exact Titles on 1st grid', () => { + cy.get('#slickGridContainer-grid1') + .find('.slick-header-columns') + .children() + .each(($child, index) => { + expect($child.text()).to.eq(titles[index]); + }); + }); + + it('should have 2 rows pre-selected in 2nd grid', () => { + cy.get('[data-test=grid2-selections]').should('contain', 'Task 0,Task 2'); + + cy.get('#grid2') + .find('.slick-row') + .children() + .filter('.slick-cell-checkboxsel.selected.true') + .should('have.length', 2); + }); + + it('should have 0 rows selected in 2nd grid after typing in a search filter', () => { + cy.get('#grid2') + .find('.filter-title') + .type('Task 1'); + + cy.get('#grid2') + .find('.slick-row') + .should('not.have.length', 0); + + cy.get('[data-test=grid2-selections]').should('contain', ''); + + cy.get('#grid2') + .find('.slick-row') + .children() + .filter('.slick-cell-checkboxsel.selected.true') + .should('have.length', 0); + }); +});