Skip to content
This repository was archived by the owner on Jun 1, 2025. It is now read-only.

Commit 149dd20

Browse files
Ghislain BeaulacGhislain Beaulac
authored andcommitted
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
1 parent ebad82a commit 149dd20

File tree

4 files changed

+119
-25
lines changed

4 files changed

+119
-25
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ <h2>{{title}}</h2>
66
<div class="col-sm-8">
77
<div class="alert alert-success">
88
<strong>(single select) Selected Row:</strong>
9-
<span [innerHTML]="selectedTitle"></span>
9+
<span [innerHTML]="selectedTitle" data-test="grid1-selections"></span>
1010
</div>
1111
</div>
1212
</div>
@@ -27,7 +27,7 @@ <h2>{{title}}</h2>
2727
<div class="col-sm-8">
2828
<div class="alert alert-success">
2929
<strong>(multi-select) Selected Row(s):</strong>
30-
<span [innerHTML]="selectedTitles"></span>
30+
<span [innerHTML]="selectedTitles" data-test="grid2-selections"></span>
3131
</div>
3232
</div>
3333
</div>

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -259,29 +259,40 @@ describe('ExtensionService', () => {
259259
});
260260

261261
it('should register the CheckboxSelector addon when "enableCheckboxSelector" is set in the grid options', () => {
262+
const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
262263
const gridOptionsMock = { enableCheckboxSelector: true } as GridOption;
263-
const extSpy = jest.spyOn(extensionStub, 'register').mockReturnValue(instanceMock);
264+
const extCreateSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
265+
const extRegisterSpy = jest.spyOn(extensionStub, 'register');
264266
const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
265267

268+
service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
266269
service.bindDifferentExtensions();
270+
const rowSelectionInstance = service.getExtensionByName(ExtensionName.rowSelection);
267271
const output = service.getExtensionByName(ExtensionName.checkboxSelector);
268272

269273
expect(gridSpy).toHaveBeenCalled();
270-
expect(extSpy).toHaveBeenCalled();
274+
expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
275+
expect(extRegisterSpy).toHaveBeenCalled();
276+
expect(rowSelectionInstance).not.toBeNull();
271277
expect(output).toEqual({ name: ExtensionName.checkboxSelector, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel);
272278
});
273279

274280
it('should register the RowDetailView addon when "enableRowDetailView" is set in the grid options', () => {
281+
const columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }] as Column[];
275282
const gridOptionsMock = { enableRowDetailView: true } as GridOption;
276-
const extSpy = jest.spyOn(extensionStub, 'register').mockReturnValue(instanceMock);
283+
const extCreateSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
284+
const extRegisterSpy = jest.spyOn(extensionStub, 'register');
277285
const gridSpy = jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
278286

287+
service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
279288
service.bindDifferentExtensions();
289+
const rowSelectionInstance = service.getExtensionByName(ExtensionName.rowSelection);
280290
const output = service.getExtensionByName(ExtensionName.rowDetailView);
281291

282292
expect(gridSpy).toHaveBeenCalled();
283-
expect(extSpy).toHaveBeenCalled();
284-
expect(output).toEqual({ name: ExtensionName.rowDetailView, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel);
293+
expect(extCreateSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
294+
expect(rowSelectionInstance).not.toBeNull();
295+
expect(extRegisterSpy).toHaveBeenCalled(); expect(output).toEqual({ name: ExtensionName.rowDetailView, addon: instanceMock, instance: instanceMock, class: extensionStub } as ExtensionModel);
285296
});
286297

287298
it('should register the RowMoveManager addon when "enableRowMoveManager" is set in the grid options', () => {
@@ -395,7 +406,6 @@ describe('ExtensionService', () => {
395406
const gridOptionsMock = { enableCheckboxSelector: true } as GridOption;
396407
const extSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
397408

398-
service.bindDifferentExtensions();
399409
service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
400410

401411
expect(extSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
@@ -406,7 +416,6 @@ describe('ExtensionService', () => {
406416
const gridOptionsMock = { enableRowDetailView: true } as GridOption;
407417
const extSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
408418

409-
service.bindDifferentExtensions();
410419
service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
411420

412421
expect(extSpy).toHaveBeenCalledWith(columnsMock, gridOptionsMock);
@@ -417,7 +426,6 @@ describe('ExtensionService', () => {
417426
const gridOptionsMock = { enableDraggableGrouping: true } as GridOption;
418427
const extSpy = jest.spyOn(extensionStub, 'create').mockReturnValue(instanceMock);
419428

420-
service.bindDifferentExtensions();
421429
service.createExtensionsBeforeGridCreation(columnsMock, gridOptionsMock);
422430

423431
expect(extSpy).toHaveBeenCalledWith(gridOptionsMock);

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

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { SharedService } from './shared.service';
2929

3030
@Injectable()
3131
export class ExtensionService {
32+
private _extensionCreatedList: any[] = [];
3233
private _extensionList: ExtensionModel[] = [];
3334

3435
constructor(
@@ -82,7 +83,7 @@ export class ExtensionService {
8283
* @param name
8384
*/
8485
getExtensionByName(name: ExtensionName): ExtensionModel | undefined {
85-
return this._extensionList.find((p) => p.name === name);
86+
return Array.isArray(this._extensionList) && this._extensionList.find((p) => p.name === name);
8687
}
8788

8889
/**
@@ -131,11 +132,22 @@ export class ExtensionService {
131132
}
132133
}
133134

135+
// Row Selection Plugin
136+
// this extension should be registered BEFORE the Checkbox Selector & Row Detail since it can be use by these 2 plugins
137+
if (!this.getExtensionByName(ExtensionName.rowSelection) && (this.sharedService.gridOptions.enableRowSelection || this.sharedService.gridOptions.enableCheckboxSelector || this.sharedService.gridOptions.enableRowDetailView)) {
138+
if (this.rowSelectionExtension && this.rowSelectionExtension.register) {
139+
const instance = this.rowSelectionExtension.register();
140+
this._extensionList.push({ name: ExtensionName.rowSelection, class: this.rowSelectionExtension, addon: instance, instance });
141+
}
142+
}
143+
134144
// Checkbox Selector Plugin
135145
if (this.sharedService.gridOptions.enableCheckboxSelector) {
136146
if (this.checkboxSelectorExtension && this.checkboxSelectorExtension.register) {
137147
const rowSelectionExtension = this.getExtensionByName(ExtensionName.rowSelection);
138-
const instance = this.checkboxSelectorExtension.register(rowSelectionExtension);
148+
this.checkboxSelectorExtension.register(rowSelectionExtension);
149+
const createdExtension = this.getCreatedExtensionByName(ExtensionName.checkboxSelector); // get the instance from when it was really created earlier
150+
const instance = createdExtension && createdExtension.instance;
139151
this._extensionList.push({ name: ExtensionName.checkboxSelector, class: this.checkboxSelectorExtension, addon: instance, instance });
140152
}
141153
}
@@ -193,7 +205,9 @@ export class ExtensionService {
193205
if (this.sharedService.gridOptions.enableRowDetailView) {
194206
if (this.rowDetailViewExtension && this.rowDetailViewExtension.register) {
195207
const rowSelectionExtension = this.getExtensionByName(ExtensionName.rowSelection);
196-
const instance = this.rowDetailViewExtension.register(rowSelectionExtension);
208+
this.rowDetailViewExtension.register(rowSelectionExtension);
209+
const createdExtension = this.getCreatedExtensionByName(ExtensionName.rowDetailView); // get the plugin from when it was really created earlier
210+
const instance = createdExtension && createdExtension.instance;
197211
this._extensionList.push({ name: ExtensionName.rowDetailView, class: this.rowDetailViewExtension, addon: instance, instance });
198212
}
199213
}
@@ -206,14 +220,6 @@ export class ExtensionService {
206220
}
207221
}
208222

209-
// Row Selection Plugin
210-
if (!this.sharedService.gridOptions.enableCheckboxSelector && this.sharedService.gridOptions.enableRowSelection) {
211-
if (this.rowSelectionExtension && this.rowSelectionExtension.register) {
212-
const instance = this.rowSelectionExtension.register();
213-
this._extensionList.push({ name: ExtensionName.rowSelection, class: this.rowSelectionExtension, addon: instance, instance });
214-
}
215-
}
216-
217223
// manually register other plugins
218224
if (this.sharedService.gridOptions.registerPlugins !== undefined) {
219225
if (Array.isArray(this.sharedService.gridOptions.registerPlugins)) {
@@ -239,14 +245,23 @@ export class ExtensionService {
239245
*/
240246
createExtensionsBeforeGridCreation(columnDefinitions: Column[], options: GridOption) {
241247
if (options.enableCheckboxSelector) {
242-
this.checkboxSelectorExtension.create(columnDefinitions, options);
248+
if (!this.getCreatedExtensionByName(ExtensionName.checkboxSelector)) {
249+
const checkboxInstance = this.checkboxSelectorExtension.create(columnDefinitions, options);
250+
this._extensionCreatedList.push({ name: ExtensionName.checkboxSelector, instance: checkboxInstance });
251+
}
243252
}
244253
if (options.enableRowDetailView) {
245-
this.rowDetailViewExtension.create(columnDefinitions, options);
254+
if (!this.getCreatedExtensionByName(ExtensionName.rowDetailView)) {
255+
const rowDetailInstance = this.rowDetailViewExtension.create(columnDefinitions, options);
256+
this._extensionCreatedList.push({ name: ExtensionName.rowDetailView, instance: rowDetailInstance });
257+
}
246258
}
247259
if (options.enableDraggableGrouping) {
248-
const plugin = this.draggableGroupingExtension.create(options);
249-
options.enableColumnReorder = plugin.getSetupColumnReorder;
260+
if (!this.getCreatedExtensionByName(ExtensionName.rowDetailView)) {
261+
const draggableInstance = this.draggableGroupingExtension.create(options);
262+
options.enableColumnReorder = draggableInstance.getSetupColumnReorder;
263+
this._extensionCreatedList.push({ name: ExtensionName.draggableGrouping, instance: draggableInstance });
264+
}
250265
}
251266
}
252267

@@ -350,6 +365,18 @@ export class ExtensionService {
350365
}
351366
}
352367

368+
//
369+
// private functions
370+
// -------------------
371+
372+
/**
373+
* Get an Extension that was created by calling its "create" method (there are only 3 extensions which uses this method)
374+
* @param name
375+
*/
376+
private getCreatedExtensionByName(name: ExtensionName): ExtensionModel | undefined {
377+
return Array.isArray(this._extensionCreatedList) && this._extensionCreatedList.find((p) => p.name === name);
378+
}
379+
353380
/** Translate an array of items from an input key and assign translated value to the output key */
354381
private translateItems(items: any[], inputKey: string, outputKey: string) {
355382
if (Array.isArray(items)) {
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
describe('Example 10 - Multiple Grids with Row Selection', () => {
2+
const titles = ['', 'Title', 'Duration (days)', '% Complete', 'Start', 'Finish', 'Effort Driven'];
3+
4+
it('should display Example 10 title', () => {
5+
cy.visit(`${Cypress.config('baseExampleUrl')}/selection`);
6+
cy.get('h2').should('contain', 'Example 10: Multiple Grids with Row Selection');
7+
});
8+
9+
it('should have 2 grids of size 800 by 200px', () => {
10+
cy.get('#slickGridContainer-grid1')
11+
.should('have.css', 'width', '800px');
12+
13+
cy.get('#slickGridContainer-grid1 > .slickgrid-container')
14+
.should('have.css', 'height', '200px');
15+
16+
cy.get('#slickGridContainer-grid2')
17+
.should('have.css', 'width', '800px');
18+
19+
cy.get('#slickGridContainer-grid2 > .slickgrid-container')
20+
.should('have.css', 'height', '200px');
21+
});
22+
23+
it('should have exact Titles on 1st grid', () => {
24+
cy.get('#slickGridContainer-grid1')
25+
.find('.slick-header-columns')
26+
.children()
27+
.each(($child, index) => {
28+
expect($child.text()).to.eq(titles[index]);
29+
});
30+
});
31+
32+
it('should have 2 rows pre-selected in 2nd grid', () => {
33+
cy.get('[data-test=grid2-selections]').should('contain', 'Task 0,Task 2');
34+
35+
cy.get('#grid2')
36+
.find('.slick-row')
37+
.children()
38+
.filter('.slick-cell-checkboxsel.selected.true')
39+
.should('have.length', 2);
40+
});
41+
42+
it('should have 0 rows selected in 2nd grid after typing in a search filter', () => {
43+
cy.get('#grid2')
44+
.find('.filter-title')
45+
.type('Task 1');
46+
47+
cy.get('#grid2')
48+
.find('.slick-row')
49+
.should('not.have.length', 0);
50+
51+
cy.get('[data-test=grid2-selections]').should('contain', '');
52+
53+
cy.get('#grid2')
54+
.find('.slick-row')
55+
.children()
56+
.filter('.slick-cell-checkboxsel.selected.true')
57+
.should('have.length', 0);
58+
});
59+
});

0 commit comments

Comments
 (0)