Skip to content

Commit

Permalink
fix(table): error when nesting tables (#18832)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto committed Mar 21, 2020
1 parent 7c75d6e commit 32aec93
Show file tree
Hide file tree
Showing 12 changed files with 296 additions and 42 deletions.
23 changes: 17 additions & 6 deletions src/cdk/table/cell.ts
Expand Up @@ -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. */
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/cdk/table/public-api.ts
Expand Up @@ -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';
20 changes: 16 additions & 4 deletions src/cdk/table/row.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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<any>, _differs: IterableDiffers) {
constructor(
template: TemplateRef<any>,
_differs: IterableDiffers,
@Inject(CDK_TABLE) @Optional() public _table?: any) {
super(template, _differs);
}

Expand Down Expand Up @@ -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<any>, _differs: IterableDiffers) {
constructor(
template: TemplateRef<any>,
_differs: IterableDiffers,
@Inject(CDK_TABLE) @Optional() public _table?: any) {
super(template, _differs);
}

Expand Down Expand Up @@ -152,7 +161,10 @@ export class CdkRowDef<T> extends BaseRowDef {

// TODO(andrewseguin): Add an input for providing a switch function to determine
// if this template should be used.
constructor(template: TemplateRef<any>, _differs: IterableDiffers) {
constructor(
template: TemplateRef<any>,
_differs: IterableDiffers,
@Inject(CDK_TABLE) @Optional() public _table?: any) {
super(template, _differs);
}
}
Expand Down
64 changes: 64 additions & 0 deletions src/cdk/table/table.spec.ts
Expand Up @@ -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<HTMLTableRowElement>(outerTable.querySelector('tbody').rows);
const innerRows = Array.from<HTMLTableRowElement>(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');
Expand Down Expand Up @@ -2276,6 +2291,55 @@ class NativeHtmlTableApp {
@ViewChild(CdkTable) table: CdkTable<TestData>;
}


@Component({
template: `
<table cdk-table [dataSource]="dataSource">
<ng-container cdkColumnDef="column_a">
<th cdk-header-cell *cdkHeaderCellDef> Column A</th>
<td cdk-cell *cdkCellDef="let row">{{row.a}}</td>
</ng-container>
<ng-container cdkColumnDef="column_b">
<th cdk-header-cell *cdkHeaderCellDef> Column B</th>
<td cdk-cell *cdkCellDef="let row">
<table cdk-table [dataSource]="dataSource">
<ng-container cdkColumnDef="column_a">
<th cdk-header-cell *cdkHeaderCellDef> Column A</th>
<td cdk-cell *cdkCellDef="let row"> {{row.a}}</td>
</ng-container>
<ng-container cdkColumnDef="column_b">
<th cdk-header-cell *cdkHeaderCellDef> Column B</th>
<td cdk-cell *cdkCellDef="let row"> {{row.b}}</td>
</ng-container>
<ng-container cdkColumnDef="column_c">
<th cdk-header-cell *cdkHeaderCellDef> Column C</th>
<td cdk-cell *cdkCellDef="let row"> {{row.c}}</td>
</ng-container>
<tr cdk-header-row *cdkHeaderRowDef="columnsToRender"></tr>
<tr cdk-row *cdkRowDef="let row; columns: columnsToRender" class="customRowClass"></tr>
</table>
</td>
</ng-container>
<ng-container cdkColumnDef="column_c">
<th cdk-header-cell *cdkHeaderCellDef> Column C</th>
<td cdk-cell *cdkCellDef="let row">{{row.c}}</td>
</ng-container>
<tr cdk-header-row *cdkHeaderRowDef="columnsToRender"></tr>
<tr cdk-row *cdkRowDef="let row; columns: columnsToRender" class="customRowClass"></tr>
</table>
`
})
class NestedHtmlTableApp {
dataSource: FakeDataSource | undefined = new FakeDataSource();
columnsToRender = ['column_a', 'column_b', 'column_c'];
}

@Component({
template: `
<table cdk-table [dataSource]="dataSource">
Expand Down
27 changes: 18 additions & 9 deletions src/cdk/table/table.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -171,6 +172,7 @@ export interface RenderRow<T> {
// 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<T> implements AfterContentChecked, CollectionViewer, OnDestroy, OnInit {
private _document: Document;
Expand Down Expand Up @@ -752,7 +754,8 @@ export class CdkTable<T> 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);
Expand All @@ -763,11 +766,12 @@ export class CdkTable<T> 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);
Expand Down Expand Up @@ -1084,10 +1088,15 @@ export class CdkTable<T> implements AfterContentChecked, CollectionViewer, OnDes
});
}

/** Filters definitions that belong to this table from a QueryList. */
private _getOwnDefs<I extends {_table?: any}>(items: QueryList<I>): 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<T>(queryList: QueryList<T>, set: Set<T>): 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<T>(array: T[], set: Set<T>): T[] {
return array.concat(Array.from(set));
}
2 changes: 1 addition & 1 deletion src/cdk/table/text-column.spec.ts
Expand Up @@ -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', () => {
Expand Down
18 changes: 1 addition & 17 deletions src/cdk/table/text-column.ts
Expand Up @@ -10,7 +10,6 @@ import {
ChangeDetectionStrategy,
Component,
Inject,
InjectionToken,
Input,
OnDestroy,
OnInit,
Expand All @@ -25,24 +24,9 @@ import {
getTableTextColumnMissingParentTableError,
getTableTextColumnMissingNameError,
} from './table-errors';
import {TEXT_COLUMN_OPTIONS, TextColumnOptions} from './tokens';


/** Configurable options for `CdkTextColumn`. */
export interface TextColumnOptions<T> {
/**
* 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<TextColumnOptions<any>>('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 (`<table>`).
Expand Down
31 changes: 31 additions & 0 deletions 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<any>('CDK_TABLE');

/** Configurable options for `CdkTextColumn`. */
export interface TextColumnOptions<T> {
/**
* 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<TextColumnOptions<any>>('text-column-options');

0 comments on commit 32aec93

Please sign in to comment.