Merge pull request #251 from ghiscoding/bugfix/row-selection-after-filtering

fix(selection): selected row should be none after filtering, closes #249

Author: ghiscoding

src/app/examples/grid-rowselection.component.html

@@ -6,7 +6,7 @@ <h2>{{title}}</h2>
         <div class="col-sm-8">
           <div class="alert alert-success">
             <strong>(single select) Selected Row:</strong>
-            <span [innerHTML]="selectedTitle"></span>
+            <span [innerHTML]="selectedTitle" data-test="grid1-selections"></span>
           </div>
         </div>
     </div>
@@ -27,7 +27,7 @@ <h2>{{title}}</h2>
         <div class="col-sm-8">
           <div class="alert alert-success">
             <strong>(multi-select) Selected Row(s):</strong>
-            <span [innerHTML]="selectedTitles"></span>
+            <span [innerHTML]="selectedTitles" data-test="grid2-selections"></span>
           </div>
         </div>
     </div>

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);

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)) {

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);
+  });
+});