From 32aec936b3a28df9796249d9304cadd232df9555 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 21 Mar 2020 06:05:47 +0100 Subject: [PATCH] fix(table): error when nesting tables (#18832) Previously we used to support nesting tables, but in v9 we had to make some changes in order to handle all cases in Ivy. As a result, nesting was broken due to parent tables picking up the cell definitions of their children. These changes add some logic to account for tables being nested. Fixes #18768. --- src/cdk/table/cell.ts | 23 +++++-- src/cdk/table/public-api.ts | 1 + src/cdk/table/row.ts | 20 ++++-- src/cdk/table/table.spec.ts | 64 +++++++++++++++++ src/cdk/table/table.ts | 27 +++++--- src/cdk/table/text-column.spec.ts | 2 +- src/cdk/table/text-column.ts | 18 +---- src/cdk/table/tokens.ts | 31 +++++++++ .../mdc-table/table.spec.ts | 68 +++++++++++++++++++ src/material/table/table.spec.ts | 64 +++++++++++++++++ src/material/table/table.ts | 7 +- tools/public_api_guard/cdk/table.d.ts | 13 +++- 12 files changed, 296 insertions(+), 42 deletions(-) create mode 100644 src/cdk/table/tokens.ts diff --git a/src/cdk/table/cell.ts b/src/cdk/table/cell.ts index 55bd798cccfc..d91acbc4f249 100644 --- a/src/cdk/table/cell.ts +++ b/src/cdk/table/cell.ts @@ -7,8 +7,17 @@ */ import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion'; -import {ContentChild, Directive, ElementRef, Input, TemplateRef} from '@angular/core'; +import { + ContentChild, + Directive, + ElementRef, + Input, + TemplateRef, + Inject, + Optional, +} from '@angular/core'; import {CanStick, CanStickCtor, mixinHasStickyInput} from './can-stick'; +import {CDK_TABLE} from './tokens'; /** Base interface for a cell definition. Captures a column's cell template definition. */ @@ -67,12 +76,10 @@ export class CdkColumnDef extends _CdkColumnDefBase implements CanStick { set name(name: string) { // If the directive is set without a name (updated programatically), then this setter will // trigger with an empty string and should not overwrite the programatically set value. - if (!name) { - return; + if (name) { + this._name = name; + this.cssClassFriendlyName = name.replace(/[^a-z0-9_-]/ig, '-'); } - - this._name = name; - this.cssClassFriendlyName = name.replace(/[^a-z0-9_-]/ig, '-'); } _name: string; @@ -108,6 +115,10 @@ export class CdkColumnDef extends _CdkColumnDefBase implements CanStick { */ cssClassFriendlyName: string; + constructor(@Inject(CDK_TABLE) @Optional() public _table?: any) { + super(); + } + static ngAcceptInputType_sticky: BooleanInput; static ngAcceptInputType_stickyEnd: BooleanInput; } diff --git a/src/cdk/table/public-api.ts b/src/cdk/table/public-api.ts index 9dce14b6d719..4095bf4082f5 100644 --- a/src/cdk/table/public-api.ts +++ b/src/cdk/table/public-api.ts @@ -13,6 +13,7 @@ export * from './table-module'; export * from './sticky-styler'; export * from './can-stick'; export * from './text-column'; +export * from './tokens'; /** Re-export DataSource for a more intuitive experience for users of just the table. */ export {DataSource} from '@angular/cdk/collections'; diff --git a/src/cdk/table/row.ts b/src/cdk/table/row.ts index aaab0b864dde..3567b229f570 100644 --- a/src/cdk/table/row.ts +++ b/src/cdk/table/row.ts @@ -19,10 +19,13 @@ import { SimpleChanges, TemplateRef, ViewContainerRef, - ViewEncapsulation + ViewEncapsulation, + Inject, + Optional } from '@angular/core'; import {CanStick, CanStickCtor, mixinHasStickyInput} from './can-stick'; import {CdkCellDef, CdkColumnDef} from './cell'; +import {CDK_TABLE} from './tokens'; /** * The row template that can be used by the mat-table. Should not be used outside of the @@ -91,7 +94,10 @@ const _CdkHeaderRowDefBase: CanStickCtor&typeof CdkHeaderRowDefBase = inputs: ['columns: cdkHeaderRowDef', 'sticky: cdkHeaderRowDefSticky'], }) export class CdkHeaderRowDef extends _CdkHeaderRowDefBase implements CanStick, OnChanges { - constructor(template: TemplateRef, _differs: IterableDiffers) { + constructor( + template: TemplateRef, + _differs: IterableDiffers, + @Inject(CDK_TABLE) @Optional() public _table?: any) { super(template, _differs); } @@ -119,7 +125,10 @@ const _CdkFooterRowDefBase: CanStickCtor&typeof CdkFooterRowDefBase = inputs: ['columns: cdkFooterRowDef', 'sticky: cdkFooterRowDefSticky'], }) export class CdkFooterRowDef extends _CdkFooterRowDefBase implements CanStick, OnChanges { - constructor(template: TemplateRef, _differs: IterableDiffers) { + constructor( + template: TemplateRef, + _differs: IterableDiffers, + @Inject(CDK_TABLE) @Optional() public _table?: any) { super(template, _differs); } @@ -152,7 +161,10 @@ export class CdkRowDef extends BaseRowDef { // TODO(andrewseguin): Add an input for providing a switch function to determine // if this template should be used. - constructor(template: TemplateRef, _differs: IterableDiffers) { + constructor( + template: TemplateRef, + _differs: IterableDiffers, + @Inject(CDK_TABLE) @Optional() public _table?: any) { super(template, _differs); } } diff --git a/src/cdk/table/table.spec.ts b/src/cdk/table/table.spec.ts index 3a00f6910f24..562f1c05c9a1 100644 --- a/src/cdk/table/table.spec.ts +++ b/src/cdk/table/table.spec.ts @@ -482,6 +482,21 @@ describe('CdkTable', () => { ]); }); + it('should be able to nest tables', () => { + const thisFixture = createComponent(NestedHtmlTableApp); + thisFixture.detectChanges(); + const outerTable = thisFixture.nativeElement.querySelector('table'); + const innerTable = outerTable.querySelector('table'); + const outerRows = Array.from(outerTable.querySelector('tbody').rows); + const innerRows = Array.from(innerTable.querySelector('tbody').rows); + + expect(outerTable).toBeTruthy(); + expect(outerRows.map(row => row.cells.length)).toEqual([3, 3, 3]); + + expect(innerTable).toBeTruthy(); + expect(innerRows.map(row => row.cells.length)).toEqual([3, 3, 3]); + }); + it('should apply correct roles for native table elements', () => { const thisFixture = createComponent(NativeHtmlTableApp); const thisTableElement: HTMLTableElement = thisFixture.nativeElement.querySelector('table'); @@ -2276,6 +2291,55 @@ class NativeHtmlTableApp { @ViewChild(CdkTable) table: CdkTable; } + +@Component({ + template: ` + + + + + + + + + + + + + + + + + + +
Column A{{row.a}} Column B + + + + + + + + + + + + + + + + + + +
Column A {{row.a}} Column B {{row.b}} Column C {{row.c}}
+ 		Column C{{row.c}}
+ ` +}) +class NestedHtmlTableApp { + dataSource: FakeDataSource | undefined = new FakeDataSource(); + columnsToRender = ['column_a', 'column_b', 'column_c']; +} + @Component({ template: ` diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts index de403673391d..2fef631e7b9b 100644 --- a/src/cdk/table/table.ts +++ b/src/cdk/table/table.ts @@ -65,6 +65,7 @@ import { getTableUnknownColumnError, getTableUnknownDataSourceError } from './table-errors'; +import {CDK_TABLE} from './tokens'; /** Interface used to provide an outlet for rows to be inserted into. */ export interface RowOutlet { @@ -171,6 +172,7 @@ export interface RenderRow { // declared elsewhere, they are checked when their declaration points are checked. // tslint:disable-next-line:validate-decorators changeDetection: ChangeDetectionStrategy.Default, + providers: [{provide: CDK_TABLE, useExisting: CdkTable}] }) export class CdkTable implements AfterContentChecked, CollectionViewer, OnDestroy, OnInit { private _document: Document; @@ -752,7 +754,8 @@ export class CdkTable implements AfterContentChecked, CollectionViewer, OnDes private _cacheColumnDefs() { this._columnDefsByName.clear(); - const columnDefs = mergeQueryListAndSet(this._contentColumnDefs, this._customColumnDefs); + const columnDefs = mergeArrayAndSet( + this._getOwnDefs(this._contentColumnDefs), this._customColumnDefs); columnDefs.forEach(columnDef => { if (this._columnDefsByName.has(columnDef.name)) { throw getTableDuplicateColumnNameError(columnDef.name); @@ -763,11 +766,12 @@ export class CdkTable implements AfterContentChecked, CollectionViewer, OnDes /** Update the list of all available row definitions that can be used. */ private _cacheRowDefs() { - this._headerRowDefs = - mergeQueryListAndSet(this._contentHeaderRowDefs, this._customHeaderRowDefs); - this._footerRowDefs = - mergeQueryListAndSet(this._contentFooterRowDefs, this._customFooterRowDefs); - this._rowDefs = mergeQueryListAndSet(this._contentRowDefs, this._customRowDefs); + this._headerRowDefs = mergeArrayAndSet( + this._getOwnDefs(this._contentHeaderRowDefs), this._customHeaderRowDefs); + this._footerRowDefs = mergeArrayAndSet( + this._getOwnDefs(this._contentFooterRowDefs), this._customFooterRowDefs); + this._rowDefs = mergeArrayAndSet( + this._getOwnDefs(this._contentRowDefs), this._customRowDefs); // After all row definitions are determined, find the row definition to be considered default. const defaultRowDefs = this._rowDefs.filter(def => !def.when); @@ -1084,10 +1088,15 @@ export class CdkTable implements AfterContentChecked, CollectionViewer, OnDes }); } + /** Filters definitions that belong to this table from a QueryList. */ + private _getOwnDefs(items: QueryList): I[] { + return items.filter(item => !item._table || item._table === this); + } + static ngAcceptInputType_multiTemplateDataRows: BooleanInput; } -/** Utility function that gets a merged list of the entries in a QueryList and values of a Set. */ -function mergeQueryListAndSet(queryList: QueryList, set: Set): T[] { - return queryList.toArray().concat(Array.from(set)); +/** Utility function that gets a merged list of the entries in an array and values of a Set. */ +function mergeArrayAndSet(array: T[], set: Set): T[] { + return array.concat(Array.from(set)); } diff --git a/src/cdk/table/text-column.spec.ts b/src/cdk/table/text-column.spec.ts index 13422e02cf1d..d9bbc04c4857 100644 --- a/src/cdk/table/text-column.spec.ts +++ b/src/cdk/table/text-column.spec.ts @@ -7,7 +7,7 @@ import { } from './table-errors'; import {CdkTableModule} from './table-module'; import {expectTableToMatchContent} from './table.spec'; -import {TEXT_COLUMN_OPTIONS, TextColumnOptions} from './text-column'; +import {TEXT_COLUMN_OPTIONS, TextColumnOptions} from './tokens'; describe('CdkTextColumn', () => { diff --git a/src/cdk/table/text-column.ts b/src/cdk/table/text-column.ts index 67045f8094af..264b8322c33b 100644 --- a/src/cdk/table/text-column.ts +++ b/src/cdk/table/text-column.ts @@ -10,7 +10,6 @@ import { ChangeDetectionStrategy, Component, Inject, - InjectionToken, Input, OnDestroy, OnInit, @@ -25,24 +24,9 @@ import { getTableTextColumnMissingParentTableError, getTableTextColumnMissingNameError, } from './table-errors'; +import {TEXT_COLUMN_OPTIONS, TextColumnOptions} from './tokens'; -/** Configurable options for `CdkTextColumn`. */ -export interface TextColumnOptions { - /** - * Default function that provides the header text based on the column name if a header - * text is not provided. - */ - defaultHeaderTextTransform?: (name: string) => string; - - /** Default data accessor to use if one is not provided. */ - defaultDataAccessor?: (data: T, name: string) => string; -} - -/** Injection token that can be used to specify the text column options. */ -export const TEXT_COLUMN_OPTIONS = - new InjectionToken>('text-column-options'); - /** * Column that simply shows text content for the header and row cells. Assumes that the table * is using the native table implementation (`
`). diff --git a/src/cdk/table/tokens.ts b/src/cdk/table/tokens.ts new file mode 100644 index 000000000000..3cbd608b56f6 --- /dev/null +++ b/src/cdk/table/tokens.ts @@ -0,0 +1,31 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {InjectionToken} from '@angular/core'; + +/** + * Used to provide a table to some of the sub-components without causing a circular dependency. + * @docs-private + */ +export const CDK_TABLE = new InjectionToken('CDK_TABLE'); + +/** Configurable options for `CdkTextColumn`. */ +export interface TextColumnOptions { + /** + * Default function that provides the header text based on the column name if a header + * text is not provided. + */ + defaultHeaderTextTransform?: (name: string) => string; + + /** Default data accessor to use if one is not provided. */ + defaultDataAccessor?: (data: T, name: string) => string; +} + +/** Injection token that can be used to specify the text column options. */ +export const TEXT_COLUMN_OPTIONS = + new InjectionToken>('text-column-options'); diff --git a/src/material-experimental/mdc-table/table.spec.ts b/src/material-experimental/mdc-table/table.spec.ts index e7c52d536c52..5e8c48509afc 100644 --- a/src/material-experimental/mdc-table/table.spec.ts +++ b/src/material-experimental/mdc-table/table.spec.ts @@ -27,6 +27,7 @@ describe('MDC-based MatTable', () => { MatTableWithPaginatorApp, StickyTableApp, TableWithNgContainerRow, + NestedTableApp, ], }).compileComponents(); })); @@ -79,6 +80,22 @@ describe('MDC-based MatTable', () => { ['Footer A'], ]); }); + + it('should be able to nest tables', () => { + const fixture = TestBed.createComponent(NestedTableApp); + fixture.detectChanges(); + const outerTable = fixture.nativeElement.querySelector('table'); + const innerTable = outerTable.querySelector('table'); + const outerRows = Array.from(outerTable.querySelector('tbody').rows); + const innerRows = Array.from(innerTable.querySelector('tbody').rows); + + expect(outerTable).toBeTruthy(); + expect(outerRows.map(row => row.cells.length)).toEqual([3, 3, 3, 3]); + + expect(innerTable).toBeTruthy(); + expect(innerRows.map(row => row.cells.length)).toEqual([3, 3, 3, 3]); + }); + }); it('should render with MatTableDataSource and sort', () => { @@ -550,6 +567,57 @@ class MatTableApp { @ViewChild(MatTable, {static: true}) table: MatTable; } +@Component({ + template: ` +
+ + + + + + + + + + + + + + + + + +
Column A{{row.a}}Column B + + + + + + + + + + + + + + + + + + + + + +
Column A {{row.a}} Footer A Column B {{row.b}} Footer B Column C {{row.c}} Footer C
+ 		Column C{{row.c}}
+ ` +}) +class NestedTableApp { + dataSource: FakeDataSource | null = new FakeDataSource(); + columnsToRender = ['column_a', 'column_b', 'column_c']; +} + @Component({ template: ` diff --git a/src/material/table/table.spec.ts b/src/material/table/table.spec.ts index 5a20e3f37026..d66968f35039 100644 --- a/src/material/table/table.spec.ts +++ b/src/material/table/table.spec.ts @@ -30,6 +30,7 @@ describe('MatTable', () => { MatTableWithPaginatorApp, StickyTableApp, TableWithNgContainerRow, + NestedHtmlTableApp, ], }).compileComponents(); })); @@ -99,6 +100,21 @@ describe('MatTable', () => { ]); }); + it('should be able to nest tables', () => { + const fixture = TestBed.createComponent(NestedHtmlTableApp); + fixture.detectChanges(); + const outerTable = fixture.nativeElement.querySelector('table'); + const innerTable = outerTable.querySelector('table'); + const outerRows = Array.from(outerTable.querySelector('tbody').rows); + const innerRows = Array.from(innerTable.querySelector('tbody').rows); + + expect(outerTable).toBeTruthy(); + expect(outerRows.map(row => row.cells.length)).toEqual([3, 3, 3, 3]); + + expect(innerTable).toBeTruthy(); + expect(innerRows.map(row => row.cells.length)).toEqual([3, 3, 3, 3]); + }); + it('should render with MatTableDataSource and sort', () => { let fixture = TestBed.createComponent(MatTableWithSortApp); fixture.detectChanges(); @@ -598,6 +614,54 @@ class NativeHtmlTableApp { @ViewChild(MatTable, {static: true}) table: MatTable; } +@Component({ + template: ` +
+ + + + + + + + + + + + + + + + + +
Column A{{row.a}} Column B + + + + + + + + + + + + + + + + + + +
Column A {{row.a}} Column B {{row.b}} Column C {{row.c}}
+ 		Column C{{row.c}}
+ ` +}) +class NestedHtmlTableApp { + dataSource: FakeDataSource | null = new FakeDataSource(); + columnsToRender = ['column_a', 'column_b', 'column_c']; +} + @Component({ template: ` diff --git a/src/material/table/table.ts b/src/material/table/table.ts index 6ec188a94e0e..921b121f1c68 100644 --- a/src/material/table/table.ts +++ b/src/material/table/table.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {CDK_TABLE_TEMPLATE, CdkTable} from '@angular/cdk/table'; +import {CDK_TABLE_TEMPLATE, CdkTable, CDK_TABLE} from '@angular/cdk/table'; import {ChangeDetectionStrategy, Component, ViewEncapsulation} from '@angular/core'; /** @@ -20,7 +20,10 @@ import {ChangeDetectionStrategy, Component, ViewEncapsulation} from '@angular/co host: { 'class': 'mat-table', }, - providers: [{provide: CdkTable, useExisting: MatTable}], + providers: [ + {provide: CdkTable, useExisting: MatTable}, + {provide: CDK_TABLE, useExisting: MatTable} + ], encapsulation: ViewEncapsulation.None, // See note on CdkTable for explanation on why this uses the default change detection strategy. // tslint:disable-next-line:validate-decorators diff --git a/tools/public_api_guard/cdk/table.d.ts b/tools/public_api_guard/cdk/table.d.ts index b43da698f544..560d65e0d989 100644 --- a/tools/public_api_guard/cdk/table.d.ts +++ b/tools/public_api_guard/cdk/table.d.ts @@ -25,6 +25,8 @@ export declare type CanStickCtor = Constructor; export declare const CDK_ROW_TEMPLATE = ""; +export declare const CDK_TABLE: InjectionToken; + export declare const CDK_TABLE_TEMPLATE = "\n \n \n \n \n"; export declare class CdkCell extends BaseCdkCell { @@ -75,6 +77,7 @@ export interface CdkCellOutletRowContext { export declare class CdkColumnDef extends _CdkColumnDefBase implements CanStick { _name: string; _stickyEnd: boolean; + _table?: any; cell: CdkCellDef; cssClassFriendlyName: string; footerCell: CdkFooterCellDef; @@ -83,6 +86,7 @@ export declare class CdkColumnDef extends _CdkColumnDefBase implements CanStick set name(name: string); get stickyEnd(): boolean; set stickyEnd(v: boolean); + constructor(_table?: any); static ngAcceptInputType_sticky: BooleanInput; static ngAcceptInputType_stickyEnd: BooleanInput; static ɵdir: i0.ɵɵDirectiveDefWithMeta; @@ -108,7 +112,8 @@ export declare class CdkFooterRow { } export declare class CdkFooterRowDef extends _CdkFooterRowDefBase implements CanStick, OnChanges { - constructor(template: TemplateRef, _differs: IterableDiffers); + _table?: any; + constructor(template: TemplateRef, _differs: IterableDiffers, _table?: any); ngOnChanges(changes: SimpleChanges): void; static ngAcceptInputType_sticky: BooleanInput; static ɵdir: i0.ɵɵDirectiveDefWithMeta; @@ -134,7 +139,8 @@ export declare class CdkHeaderRow { } export declare class CdkHeaderRowDef extends _CdkHeaderRowDefBase implements CanStick, OnChanges { - constructor(template: TemplateRef, _differs: IterableDiffers); + _table?: any; + constructor(template: TemplateRef, _differs: IterableDiffers, _table?: any); ngOnChanges(changes: SimpleChanges): void; static ngAcceptInputType_sticky: BooleanInput; static ɵdir: i0.ɵɵDirectiveDefWithMeta; @@ -147,8 +153,9 @@ export declare class CdkRow { } export declare class CdkRowDef extends BaseRowDef { + _table?: any; when: (index: number, rowData: T) => boolean; - constructor(template: TemplateRef, _differs: IterableDiffers); + constructor(template: TemplateRef, _differs: IterableDiffers, _table?: any); static ɵdir: i0.ɵɵDirectiveDefWithMeta, "[cdkRowDef]", never, { "columns": "cdkRowDefColumns"; "when": "cdkRowDefWhen"; }, {}, never>; static ɵfac: i0.ɵɵFactoryDef>; }