From 66d03a7abbe580cdd7b178feb2422004dd16408e Mon Sep 17 00:00:00 2001 From: Alexis Georges Date: Tue, 30 Jul 2019 11:49:32 +0200 Subject: [PATCH] feat(stark-ui): table - add support for NG CDK selection model. ISSUES CLOSED: #1366 Disabled changeDetection strategy in Stark Table from 'OnPush' to 'Default' in order to fix selection issues. --- .../table/components/table.component.html | 2 +- .../table/components/table.component.spec.ts | 124 +++++++++++++++--- .../table/components/table.component.ts | 103 ++++++++++----- .../table-with-selection.component.html | 9 +- .../table-with-selection.ts | 10 +- .../examples/table/with-selection/table.html | 9 +- .../examples/table/with-selection/table.ts | 10 +- 7 files changed, 188 insertions(+), 79 deletions(-) diff --git a/packages/stark-ui/src/modules/table/components/table.component.html b/packages/stark-ui/src/modules/table/components/table.component.html index 4d3e986abe..890e7008e3 100644 --- a/packages/stark-ui/src/modules/table/components/table.component.html +++ b/packages/stark-ui/src/modules/table/components/table.component.html @@ -78,7 +78,7 @@ @@ -56,6 +58,7 @@ class TestHostComponent { public tableComponent!: StarkTableComponent; public columnProperties?: StarkTableColumnProperties[]; + public customTableActions?: StarkAction[]; public dummyData: object[] = []; public fixedHeader?: string; public rowsSelectable?: boolean; @@ -66,7 +69,7 @@ class TestHostComponent { public tableRowActions?: StarkTableRowActions; public tableFilter?: StarkTableFilter; public orderProperties?: string[]; - public customTableActions?: StarkAction[]; + public selection?: SelectionModel; public rowClassNameFn?: (row: object, index: number) => string; public rowClickHandler?: (row: object) => void; } @@ -96,6 +99,8 @@ describe("TableComponent", () => { }; const getColumnSelector = (columnName: string): string => `.stark-table th.mat-column-${columnName} div div`; + const columnSelectSelector = "cdk-column-select"; + const rowSelector = "table tbody tr"; beforeEach(async(() => { return TestBed.configureTestingModule({ @@ -221,6 +226,52 @@ describe("TableComponent", () => { expect(component.dataSource.data).toEqual([{ name: "test-data-2" }]); }); + it("should change internal 'selection' when 'selection' is managed by host and then trigger selectChanged", () => { + const dummySelectedRows = [{ name: "selected-data-1" }, { name: "selected-data-2" }, { name: "selected-data-3" }]; + spyOn(component.selectChanged, "emit"); + + hostComponent.selection = new SelectionModel(true, []); + hostFixture.detectChanges(); + expect(hostComponent.selection).toEqual(component.selection); + + hostComponent.selection.select(...dummySelectedRows); + expect(component.selection.selected).toEqual(dummySelectedRows); + expect(component.selection.isSelected(dummySelectedRows[0])).toBe(true); + expect(component.selection.isSelected(dummySelectedRows[1])).toBe(true); + expect(component.selection.isSelected(dummySelectedRows[2])).toBe(true); + expect(component.selectChanged.emit).toHaveBeenCalledTimes(1); + expect(component.selectChanged.emit).toHaveBeenCalledWith(dummySelectedRows); + + (component.selectChanged.emit).calls.reset(); + hostComponent.selection.clear(); + expect(component.selection.selected).toEqual([]); + expect(component.selectChanged.emit).toHaveBeenCalledTimes(1); + expect(component.selectChanged.emit).toHaveBeenCalledWith([]); + }); + + it("should change host 'selection' when 'selection' is managed by host and then trigger selectChanged", () => { + const dummySelectedRows = [{ name: "selected-data-1" }, { name: "selected-data-2" }, { name: "selected-data-3" }]; + spyOn(component.selectChanged, "emit"); + + hostComponent.selection = new SelectionModel(true, []); + hostFixture.detectChanges(); + expect(hostComponent.selection).toEqual(component.selection); + + component.selection.select(...dummySelectedRows); + expect(hostComponent.selection.selected).toEqual(dummySelectedRows); + expect(hostComponent.selection.isSelected(dummySelectedRows[0])).toBe(true); + expect(hostComponent.selection.isSelected(dummySelectedRows[1])).toBe(true); + expect(hostComponent.selection.isSelected(dummySelectedRows[2])).toBe(true); + expect(component.selectChanged.emit).toHaveBeenCalledTimes(1); + expect(component.selectChanged.emit).toHaveBeenCalledWith(dummySelectedRows); + + (component.selectChanged.emit).calls.reset(); + component.selection.clear(); + expect(hostComponent.selection.selected).toEqual([]); + expect(component.selectChanged.emit).toHaveBeenCalledTimes(1); + expect(component.selectChanged.emit).toHaveBeenCalledWith([]); + }); + it("should assign right value to isFixedHeaderEnabled when fixedHeader changes", () => { hostComponent.fixedHeader = "true"; hostFixture.detectChanges(); @@ -241,15 +292,29 @@ describe("TableComponent", () => { expect(component.isMultiSortEnabled).toBe(false); }); - it("should assign right value to isMultiSelectEnabled when multiSelect changes and rowsSelectable is enabled", () => { - hostComponent.rowsSelectable = true; - hostComponent.multiSelect = "true"; + it("should assign right value to display/hide 'select' column when 'selection' changes", () => { + expect(component.displayedColumns.indexOf("select") > -1).toBe(false); + let rowThElements = >hostFixture.nativeElement.querySelectorAll(tableThSelector); + expect(rowThElements.length).toBeGreaterThanOrEqual(0); + let selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf(columnSelectSelector) > -1); + expect(selectThElement).toBeUndefined(); + + hostComponent.selection = new SelectionModel(); hostFixture.detectChanges(); - expect(component.isMultiSelectEnabled).toBe(true); - hostComponent.multiSelect = "false"; + expect(component.displayedColumns.indexOf("select") > -1).toBe(true); + rowThElements = >hostFixture.nativeElement.querySelectorAll(tableThSelector); + expect(rowThElements.length).toBeGreaterThan(0); + selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf(columnSelectSelector) > -1); + expect(selectThElement).toBeDefined(); + + hostComponent.selection = undefined; hostFixture.detectChanges(); - expect(component.isMultiSelectEnabled).toBe(false); + expect(component.displayedColumns.indexOf("select") > -1).toBe(false); + rowThElements = >hostFixture.nativeElement.querySelectorAll(tableThSelector); + expect(rowThElements.length).toBeGreaterThanOrEqual(0); + selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf(columnSelectSelector) > -1); + expect(selectThElement).toBeUndefined(); }); it("should assign right value to display/hide 'select' column when rowsSelectable changes", () => { @@ -259,7 +324,7 @@ describe("TableComponent", () => { expect(component.displayedColumns.indexOf("select") > -1).toBe(true); let rowThElements = >hostFixture.nativeElement.querySelectorAll(tableThSelector); expect(rowThElements.length).toBeGreaterThan(0); - let selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf("cdk-column-select") > -1); + let selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf(columnSelectSelector) > -1); expect(selectThElement).toBeDefined(); hostComponent.rowsSelectable = false; @@ -267,7 +332,7 @@ describe("TableComponent", () => { expect(component.displayedColumns.indexOf("select") > -1).toBe(false); rowThElements = >hostFixture.nativeElement.querySelectorAll(tableThSelector); expect(rowThElements.length).toBeGreaterThanOrEqual(0); - selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf("cdk-column-select") > -1); + selectThElement = find(rowThElements, (thElement: HTMLElement) => thElement.className.indexOf(columnSelectSelector) > -1); expect(selectThElement).toBeUndefined(); }); @@ -1312,10 +1377,7 @@ describe("TableComponent", () => { const dummyData: object[] = [ { id: 1, description: "dummy 1" }, { id: 2, description: "dummy 2" }, - { - id: 3, - description: "dummy 3" - } + { id: 3, description: "dummy 3" } ]; beforeEach(() => { @@ -1331,6 +1393,9 @@ describe("TableComponent", () => { spyOn(component.selectChanged, "emit"); const checkboxElement: HTMLElement | null = hostFixture.nativeElement.querySelector("table tbody tr input[type=checkbox]"); + const rowElement: HTMLElement = hostFixture.nativeElement.querySelector(rowSelector); + expect(rowElement.classList).not.toContain("selected"); + if (!checkboxElement) { fail("Checkbox not found."); return; @@ -1338,7 +1403,30 @@ describe("TableComponent", () => { triggerClick(checkboxElement); hostFixture.detectChanges(); + expect(rowElement.classList).toContain("selected"); expect(component.selectChanged.emit).toHaveBeenCalledWith([dummyData[0]]); + expect(component.selection.isSelected(dummyData[0])).toBe(true); + }); + + it("should select the right rows in the template when selecting them through host 'selection' object", () => { + hostComponent.selection = new SelectionModel(true); + hostComponent.rowsSelectable = undefined; + hostFixture.detectChanges(); + + expect(component.selection.selected).toEqual([]); + const rowsElements: HTMLElement[] = hostFixture.nativeElement.querySelectorAll(rowSelector); + expect(rowsElements).not.toBeNull(); + expect(rowsElements[0].classList).not.toContain("selected"); + expect(rowsElements[1].classList).not.toContain("selected"); + expect(rowsElements[2].classList).not.toContain("selected"); + + hostComponent.selection.select(dummyData[1]); + hostFixture.detectChanges(); + + expect(component.selection.selected).toEqual([dummyData[1]]); + expect(rowsElements[0].classList).not.toContain("selected"); + expect(rowsElements[1].classList).toContain("selected"); + expect(rowsElements[2].classList).not.toContain("selected"); }); }); @@ -1358,14 +1446,14 @@ describe("TableComponent", () => { }); it("should update rows after async data fetch", () => { - const rowsBeforeData: NodeListOf = hostFixture.nativeElement.querySelectorAll("table tbody tr"); + const rowsBeforeData: NodeListOf = hostFixture.nativeElement.querySelectorAll(rowSelector); expect(rowsBeforeData.length).toBe(0); // "async fetch of data resolves" hostComponent.dummyData = DUMMY_DATA; hostFixture.detectChanges(); - const rowsAfterData: NodeListOf = hostFixture.nativeElement.querySelectorAll("table tbody tr"); + const rowsAfterData: NodeListOf = hostFixture.nativeElement.querySelectorAll(rowSelector); expect(rowsAfterData.length).toBe(DUMMY_DATA.length); }); diff --git a/packages/stark-ui/src/modules/table/components/table.component.ts b/packages/stark-ui/src/modules/table/components/table.component.ts index a6f1d5855c..c7beabc006 100644 --- a/packages/stark-ui/src/modules/table/components/table.component.ts +++ b/packages/stark-ui/src/modules/table/components/table.component.ts @@ -1,5 +1,4 @@ import { - AfterContentInit, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, @@ -73,14 +72,15 @@ const DEFAULT_COLUMN_PROPERTIES: Partial = { selector: "stark-table", templateUrl: "./table.component.html", encapsulation: ViewEncapsulation.None, - changeDetection: ChangeDetectionStrategy.OnPush, + // See note on CdkTable for explanation on why this uses the default change detection strategy. + changeDetection: ChangeDetectionStrategy.Default, // We need to use host instead of @HostBinding: https://github.com/NationalBankBelgium/stark/issues/664 host: { class: componentName } }) /* tslint:enable */ -export class StarkTableComponent extends AbstractStarkUiComponent implements OnInit, AfterContentInit, AfterViewInit, OnChanges, OnDestroy { +export class StarkTableComponent extends AbstractStarkUiComponent implements OnInit, AfterViewInit, OnChanges, OnDestroy { /** * Array of {@link StarkTableColumnProperties} objects which define the columns of the data table. */ @@ -256,6 +256,44 @@ export class StarkTableComponent extends AbstractStarkUiComponent implements OnI @Input() public rowClassNameFn?: (row: object, index: number) => string; + /** + * Angular CDK selection model used for the "master" selection of the table + */ + @Input() + public get selection(): SelectionModel { + return this._selection; + } + + public set selection(selection: SelectionModel) { + if (coerceBooleanProperty(this.multiSelect) || !!this.rowsSelectable) { + this.logger.error( + `${componentName}: 'selection' cannot be used with 'multiSelect' and/or 'rowsSelectable'. Please use 'selection' only.` + ); + } + + if (selection) { + this._managedSelection = true; + + if (!this.displayedColumns.includes("select")) { + this.displayedColumns.unshift("select"); + } + + this._selection = selection; + this._resetSelection(); + } else if (this._selection) { + this._managedSelection = false; + const i: number = this.displayedColumns.indexOf("select"); + this.displayedColumns.splice(i); + + this._resetSelection(true); + } + } + + /** + * @ignore + */ + private _selection!: SelectionModel; + /** * Determine if the row index must be present or not. * Default: false @@ -387,22 +425,15 @@ export class StarkTableComponent extends AbstractStarkUiComponent implements OnI */ public isMultiSortEnabled = false; - /** - * Whether the multiple row selection is enabled. - */ - public get isMultiSelectEnabled(): boolean { - return coerceBooleanProperty(this.multiSelect); - } - /** * @ignore */ private _selectionSub!: Subscription; /** - * Angular CDK selection model used for the "master" selection of the table + * @ignore */ - public selection: SelectionModel = new SelectionModel(true, []); + private _managedSelection = false; /** * Class constructor @@ -431,13 +462,6 @@ export class StarkTableComponent extends AbstractStarkUiComponent implements OnI this._resetSelection(); } - /** - * Component lifecycle hook - */ - public ngAfterContentInit(): void { - this.logger.debug(componentName + ": ngAfterContentInit"); - } - /** * Component lifecycle hook */ @@ -513,18 +537,30 @@ export class StarkTableComponent extends AbstractStarkUiComponent implements OnI } if (changes["rowsSelectable"]) { - if (this.rowsSelectable) { - if (!this.displayedColumns.includes("select")) { - this.displayedColumns.unshift("select"); - } + if (this._managedSelection) { + this.logger.error( + `${componentName}: 'selection' cannot be used with 'multiSelect' and/or 'rowsSelectable'. Please use 'selection' only.` + ); } else { - const i: number = this.displayedColumns.indexOf("select"); - this.displayedColumns.splice(i); + if (this.rowsSelectable) { + if (!this.displayedColumns.includes("select")) { + this.displayedColumns.unshift("select"); + } + } else { + const i: number = this.displayedColumns.indexOf("select"); + this.displayedColumns.splice(i); + } } } - if (changes["multiSelect"]) { - this._resetSelection(); + if (changes["multiSelect"] && !changes["multiSelect"].isFirstChange()) { + if (this._managedSelection) { + this.logger.error( + `${componentName}: 'selection' cannot be used with 'multiSelect' and/or 'rowsSelectable'. Please use 'selection' only.` + ); + } else { + this._resetSelection(true); + } } if (changes["customTableActionsType"] || changes["customTableActions"]) { @@ -829,8 +865,10 @@ export class StarkTableComponent extends AbstractStarkUiComponent implements OnI /** * @ignore */ - private _resetSelection(): void { - this.selection = new SelectionModel(this.isMultiSelectEnabled, []); + private _resetSelection(forceReset: boolean = false): void { + if (!this.selection || forceReset) { + this._selection = new SelectionModel(coerceBooleanProperty(this.multiSelect), []); + } // Emit event when selection changes if (this._selectionSub) { @@ -1062,11 +1100,4 @@ export class StarkTableComponent extends AbstractStarkUiComponent implements OnI public trackColumnFn(_index: number, item: StarkTableColumnProperties): string { return item.name; } - - /** - * @ignore - */ - public trackActionFn(_index: number, item: StarkAction): string { - return item.id; - } } diff --git a/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.component.html b/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.component.html index f6bdb2c630..41454c5b5d 100644 --- a/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.component.html +++ b/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.component.html @@ -1,10 +1,3 @@ - +

SHOWCASE.DEMO.TABLE.WITH_SELECTION

 diff --git a/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.ts b/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.ts index fc22c811bc..b3d272ad7a 100644 --- a/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.ts +++ b/showcase/src/app/demo-ui/components/table-with-selection/table-with-selection.ts @@ -1,6 +1,7 @@ import { Component, Inject } from "@angular/core"; import { STARK_LOGGING_SERVICE, StarkLoggingService } from "@nationalbankbelgium/stark-core"; import { StarkTableColumnProperties, StarkTableFilter } from "@nationalbankbelgium/stark-ui"; +import { SelectionChange, SelectionModel } from "@angular/cdk/collections"; const DUMMY_DATA: object[] = [ { id: 1, title: { label: "first title (value: 1)", value: 1 }, description: "number one" }, @@ -23,6 +24,7 @@ const DUMMY_DATA: object[] = [ }) export class TableWithSelectionComponent { public data: object[] = DUMMY_DATA; + public selection = new SelectionModel(true); public columns: StarkTableColumnProperties[] = [ { name: "id", label: "Id", isFilterable: true, isSortable: true }, @@ -36,9 +38,9 @@ export class TableWithSelectionComponent { public filter: StarkTableFilter = { globalFilterPresent: false, columns: [] }; - public constructor(@Inject(STARK_LOGGING_SERVICE) private logger: StarkLoggingService) {} - - public handleRowSelected(data: object[]): void { - this.logger.debug("SELECTED ROW:", data); + public constructor(@Inject(STARK_LOGGING_SERVICE) private logger: StarkLoggingService) { + this.selection.changed.subscribe((change: SelectionChange) => { + this.logger.debug("SELECTED ROW:", change.source.selected); + }); } } diff --git a/showcase/src/assets/examples/table/with-selection/table.html b/showcase/src/assets/examples/table/with-selection/table.html index 1de7b57728..9ffd2c95b5 100644 --- a/showcase/src/assets/examples/table/with-selection/table.html +++ b/showcase/src/assets/examples/table/with-selection/table.html @@ -1,10 +1,3 @@ - +

Table with selection

 diff --git a/showcase/src/assets/examples/table/with-selection/table.ts b/showcase/src/assets/examples/table/with-selection/table.ts index db5bfc92ae..d010eccc65 100644 --- a/showcase/src/assets/examples/table/with-selection/table.ts +++ b/showcase/src/assets/examples/table/with-selection/table.ts @@ -1,6 +1,7 @@ import { Component, Inject } from "@angular/core"; import { STARK_LOGGING_SERVICE, StarkLoggingService } from "@nationalbankbelgium/stark-core"; import { StarkTableColumnProperties, StarkTableFilter } from "@nationalbankbelgium/stark-ui"; +import { SelectionChange, SelectionModel } from "@angular/cdk/collections"; const DUMMY_DATA: object[] = [ { id: 1, title: { label: "first title (value: 1)", value: 1 }, description: "number one" }, @@ -14,6 +15,7 @@ const DUMMY_DATA: object[] = [ }) export class TableWithSelectionComponent { public data: object[] = DUMMY_DATA; + public selection = new SelectionModel(true); public columns: StarkTableColumnProperties[] = [ { name: "id", label: "Id", isFilterable: true, isSortable: true }, @@ -27,9 +29,9 @@ export class TableWithSelectionComponent { public filter: StarkTableFilter = { globalFilterPresent: false, columns: [] }; - public constructor(@Inject(STARK_LOGGING_SERVICE) private logger: StarkLoggingService) {} - - public handleRowSelected(data: object[]): void { - this.logger.debug("SELECTED ROW:", data); + public constructor(@Inject(STARK_LOGGING_SERVICE) private logger: StarkLoggingService) { + this.selection.changed.subscribe((change: SelectionChange) => { + this.logger.debug("SELECTED ROW:", change.source.selected); + }); } }