feat(table): support dynamic column definitions (#5545)

* checkin

* move logic from baserow to table

* update

* tests

* add tests

* lint

* fix imports, revert columnsDiffer as protected

* add tests for errors

* add license to errors file

* review

* remove unnecessary imports

* add comment about ngDoCheck()

* cleanup imports

* remove fdescribe

* rebase

* ugh, fdescribe

* rebase

Author: andrewseguin

src/cdk/table/row.ts

@@ -10,14 +10,14 @@ import {
   ChangeDetectionStrategy,
   Component,
   Directive,
+  IterableChanges,
   IterableDiffer,
   IterableDiffers,
   SimpleChanges,
   TemplateRef,
   ViewContainerRef
 } from '@angular/core';
 import {CdkCellDef} from './cell';
-import {Subject} from 'rxjs/Subject';
 
 /**
  * The row template that can be used by the md-table. Should not be used outside of the
@@ -33,21 +33,12 @@ export abstract class BaseRowDef {
   /** The columns to be displayed on this row. */
   columns: string[];
 
-  /** Event stream that emits when changes are made to the columns. */
-  columnsChange: Subject<void> = new Subject<void>();
-
   /** Differ used to check if any changes were made to the columns. */
   protected _columnsDiffer: IterableDiffer<any>;
 
-  private viewInitialized = false;
-
   constructor(public template: TemplateRef<any>,
               protected _differs: IterableDiffers) { }
 
-  ngAfterViewInit() {
-    this.viewInitialized = true;
-  }
-
   ngOnChanges(changes: SimpleChanges): void {
     // Create a new columns differ if one does not yet exist. Initialize it based on initial value
     // of the columns property.
@@ -58,12 +49,12 @@ export abstract class BaseRowDef {
     }
   }
 
-  ngDoCheck(): void {
-    if (!this.viewInitialized || !this._columnsDiffer || !this.columns) { return; }
-
-    // Notify the table if there are any changes to the columns.
-    const changes = this._columnsDiffer.diff(this.columns);
-    if (changes) { this.columnsChange.next(); }
+  /**
+   * Returns the difference between the current columns and the columns from the last diff, or null
+   * if there is no difference.
+   */
+  getColumnsDiff(): IterableChanges<any> | null {
+    return this._columnsDiffer.diff(this.columns);
   }
 }
 

src/cdk/table/table-errors.ts

@@ -0,0 +1,24 @@
+/**
+ * @license
+ * Copyright Google Inc. 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
+ */
+
+/**
+ * Returns an error to be thrown when attempting to find an unexisting column.
+ * @param id Id whose lookup failed.
+ * @docs-private
+ */
+export function getTableUnknownColumnError(id: string) {
+  return Error(`cdk-table: Could not find column with id "${id}".`);
+}
+
+/**
+ * Returns an error to be thrown when two column definitions have the same name.
+ * @docs-private
+ */
+export function getTableDuplicateColumnNameError(name: string) {
+  return Error(`cdk-table: Duplicate column definition name provided: "${name}".`);
+}

src/cdk/table/table.spec.ts

@@ -7,6 +7,7 @@ import {Observable} from 'rxjs/Observable';
 import {combineLatest} from 'rxjs/observable/combineLatest';
 import {CdkTableModule} from './index';
 import {map} from 'rxjs/operator/map';
+import {getTableDuplicateColumnNameError, getTableUnknownColumnError} from './table-errors';
 
 describe('CdkTable', () => {
   let fixture: ComponentFixture<SimpleCdkTableApp>;
@@ -24,7 +25,10 @@ describe('CdkTable', () => {
         DynamicDataSourceCdkTableApp,
         CustomRoleCdkTableApp,
         TrackByCdkTableApp,
+        DynamicColumnDefinitionsCdkTableApp,
         RowContextCdkTableApp,
+        DuplicateColumnDefNameCdkTableApp,
+        MissingColumnDefCdkTableApp,
       ],
     }).compileComponents();
   }));
@@ -107,6 +111,57 @@ describe('CdkTable', () => {
     expect(fixture.nativeElement.querySelector('cdk-table').getAttribute('role')).toBe('treegrid');
   });
 
+  it('should throw an error if two column definitions have the same name', () => {
+    expect(() => TestBed.createComponent(DuplicateColumnDefNameCdkTableApp).detectChanges())
+        .toThrowError(getTableDuplicateColumnNameError('column_a').message);
+  });
+
+  it('should throw an error if a column definition is requested but not defined', () => {
+    expect(() => TestBed.createComponent(MissingColumnDefCdkTableApp).detectChanges())
+        .toThrowError(getTableUnknownColumnError('column_a').message);
+  });
+
+  it('should be able to dynamically add/remove column definitions', () => {
+    const dynamicColumnDefFixture = TestBed.createComponent(DynamicColumnDefinitionsCdkTableApp);
+    dynamicColumnDefFixture.detectChanges();
+    dynamicColumnDefFixture.detectChanges();
+
+    const dynamicColumnDefTable = dynamicColumnDefFixture.nativeElement.querySelector('cdk-table');
+    const dynamicColumnDefComp = dynamicColumnDefFixture.componentInstance;
+
+    // Add a new column and expect it to show up in the table
+    let columnA = 'columnA';
+    dynamicColumnDefComp.dynamicColumns.push(columnA);
+    dynamicColumnDefFixture.detectChanges();
+    expectTableToMatchContent(dynamicColumnDefTable, [
+      [columnA], // Header row
+      [columnA], // Data rows
+      [columnA],
+      [columnA],
+    ]);
+
+    // Add another new column and expect it to show up in the table
+    let columnB = 'columnB';
+    dynamicColumnDefComp.dynamicColumns.push(columnB);
+    dynamicColumnDefFixture.detectChanges();
+    expectTableToMatchContent(dynamicColumnDefTable, [
+      [columnA, columnB], // Header row
+      [columnA, columnB], // Data rows
+      [columnA, columnB],
+      [columnA, columnB],
+    ]);
+
+    // Remove column A expect only column B to be rendered
+    dynamicColumnDefComp.dynamicColumns.shift();
+    dynamicColumnDefFixture.detectChanges();
+    expectTableToMatchContent(dynamicColumnDefTable, [
+      [columnB], // Header row
+      [columnB], // Data rows
+      [columnB],
+      [columnB],
+    ]);
+  });
+
   it('should re-render the rows when the data changes', () => {
     dataSource.addData();
     fixture.detectChanges();
@@ -587,6 +642,26 @@ class TrackByCdkTableApp {
   }
 }
 
+@Component({
+  template: `
+    <cdk-table [dataSource]="dataSource">
+      <ng-container [cdkColumnDef]="column" *ngFor="let column of dynamicColumns">
+        <cdk-header-cell *cdkHeaderCellDef> {{column}} </cdk-header-cell>
+        <cdk-cell *cdkCellDef="let row"> {{column}} </cdk-cell>
+      </ng-container>
+
+      <cdk-header-row *cdkHeaderRowDef="dynamicColumns"></cdk-header-row>
+      <cdk-row *cdkRowDef="let row; columns: dynamicColumns;"></cdk-row>
+    </cdk-table>
+  `
+})
+class DynamicColumnDefinitionsCdkTableApp {
+  dynamicColumns: any[] = [];
+  dataSource: FakeDataSource = new FakeDataSource();
+
+  @ViewChild(CdkTable) table: CdkTable<TestData>;
+}
+
 @Component({
   template: `
     <cdk-table [dataSource]="dataSource" role="treegrid">
@@ -607,6 +682,45 @@ class CustomRoleCdkTableApp {
   @ViewChild(CdkTable) table: CdkTable<TestData>;
 }
 
+@Component({
+  template: `
+    <cdk-table [dataSource]="dataSource">
+      <ng-container cdkColumnDef="column_a">
+        <cdk-header-cell *cdkHeaderCellDef> Column A</cdk-header-cell>
+        <cdk-cell *cdkCellDef="let row"> {{row.a}}</cdk-cell>
+      </ng-container>
+
+      <ng-container cdkColumnDef="column_a">
+        <cdk-header-cell *cdkHeaderCellDef> Column A</cdk-header-cell>
+        <cdk-cell *cdkCellDef="let row"> {{row.a}}</cdk-cell>
+      </ng-container>
+
+      <cdk-header-row *cdkHeaderRowDef="['column_a']"></cdk-header-row>
+      <cdk-row *cdkRowDef="let row; columns: ['column_a']"></cdk-row>
+    </cdk-table>
+  `
+})
+class DuplicateColumnDefNameCdkTableApp {
+  dataSource: FakeDataSource = new FakeDataSource();
+}
+
+@Component({
+  template: `
+    <cdk-table [dataSource]="dataSource">
+      <ng-container cdkColumnDef="column_b">
+        <cdk-header-cell *cdkHeaderCellDef> Column A</cdk-header-cell>
+        <cdk-cell *cdkCellDef="let row"> {{row.a}}</cdk-cell>
+      </ng-container>
+
+      <cdk-header-row *cdkHeaderRowDef="['column_a']"></cdk-header-row>
+      <cdk-row *cdkRowDef="let row; columns: ['column_a']"></cdk-row>
+    </cdk-table>
+  `
+})
+class MissingColumnDefCdkTableApp {
+  dataSource: FakeDataSource = new FakeDataSource();
+}
+
 @Component({
   template: `
     <cdk-table [dataSource]="dataSource">

src/cdk/table/table.ts

@@ -31,21 +31,12 @@ import {
 } from '@angular/core';
 import {CollectionViewer, DataSource} from './data-source';
 import {CdkCellOutlet, CdkCellOutletRowContext, CdkHeaderRowDef, CdkRowDef} from './row';
-import {merge} from 'rxjs/observable/merge';
 import {takeUntil} from 'rxjs/operator/takeUntil';
 import {BehaviorSubject} from 'rxjs/BehaviorSubject';
 import {Subscription} from 'rxjs/Subscription';
 import {Subject} from 'rxjs/Subject';
 import {CdkCellDef, CdkColumnDef, CdkHeaderCellDef} from './cell';
-
-/**
- * Returns an error to be thrown when attempting to find an unexisting column.
- * @param id Id whose lookup failed.
- * @docs-private
- */
-export function getTableUnknownColumnError(id: string) {
-  return new Error(`cdk-table: Could not find column with id "${id}".`);
-}
+import {getTableDuplicateColumnNameError, getTableUnknownColumnError} from './table-errors';
 
 /**
  * Provides a handle for the table to grab the view container's ng-container to insert data rows.
@@ -96,10 +87,7 @@ export class CdkTable<T> implements CollectionViewer {
   /** Subscription that listens for the data provided by the data source. */
   private _renderChangeSubscription: Subscription | null;
 
-  /**
-   * Map of all the user's defined columns identified by name.
-   * Contains the header and data-cell templates.
-   */
+  /** Map of all the user's defined columns (header and data cell template) identified by name. */
   private _columnDefinitionsByName = new Map<string,  CdkColumnDef>();
 
   /** Differ used to find the changes in the data provided by the data source. */
@@ -123,15 +111,6 @@ export class CdkTable<T> implements CollectionViewer {
   get trackBy(): TrackByFunction<T> { return this._trackByFn; }
   private _trackByFn: TrackByFunction<T>;
 
-  // TODO(andrewseguin): Remove max value as the end index
-  //   and instead calculate the view on init and scroll.
-  /**
-   * Stream containing the latest information on the range of rows being displayed on screen.
-   * Can be used by the data source to as a heuristic of what data should be provided.
-   */
-  viewChange =
-      new BehaviorSubject<{start: number, end: number}>({start: 0, end: Number.MAX_VALUE});
-
   /**
    * Provides a stream containing the latest data array to render. Influenced by the table's
    * stream of view window (what rows are currently on screen).
@@ -145,6 +124,15 @@ export class CdkTable<T> implements CollectionViewer {
   }
   private _dataSource: DataSource<T>;
 
+  // TODO(andrewseguin): Remove max value as the end index
+  //   and instead calculate the view on init and scroll.
+  /**
+   * Stream containing the latest information on what rows are being displayed on screen.
+   * Can be used by the data source to as a heuristic of what data should be provided.
+   */
+  viewChange =
+      new BehaviorSubject<{start: number, end: number}>({start: 0, end: Number.MAX_VALUE});
+
   // Placeholders within the table's template where the header and data rows will be inserted.
   @ViewChild(RowPlaceholder) _rowPlaceholder: RowPlaceholder;
   @ViewChild(HeaderRowPlaceholder) _headerRowPlaceholder: HeaderRowPlaceholder;
@@ -171,6 +159,24 @@ export class CdkTable<T> implements CollectionViewer {
     }
   }
 
+  ngOnInit() {
+    // TODO(andrewseguin): Setup a listener for scrolling, emit the calculated view to viewChange
+    this._dataDiffer = this._differs.find([]).create(this._trackByFn);
+  }
+
+  ngAfterContentInit() {
+    this._cacheColumnDefinitionsByName();
+    this._columnDefinitions.changes.subscribe(() => this._cacheColumnDefinitionsByName());
+    this._renderHeaderRow();
+  }
+
+  ngAfterContentChecked() {
+    this._renderUpdatedColumns();
+    if (this.dataSource && !this._renderChangeSubscription) {
+      this._observeRenderChanges();
+    }
+  }
+
   ngOnDestroy() {
     this._rowPlaceholder.viewContainer.clear();
     this._headerRowPlaceholder.viewContainer.clear();
@@ -182,41 +188,38 @@ export class CdkTable<T> implements CollectionViewer {
     }
   }
 
-  ngOnInit() {
-    // TODO(andrewseguin): Setup a listener for scroll events
-    //   and emit the calculated view to this.viewChange
-    this._dataDiffer = this._differs.find([]).create(this._trackByFn);
-  }
 
-  ngAfterContentInit() {
-    // TODO(andrewseguin): Throw an error if two columns share the same name
+  /** Update the map containing the content's column definitions. */
+  private _cacheColumnDefinitionsByName() {
+    this._columnDefinitionsByName.clear();
     this._columnDefinitions.forEach(columnDef => {
+      if (this._columnDefinitionsByName.has(columnDef.name)) {
+        throw getTableDuplicateColumnNameError(columnDef.name);
+      }
       this._columnDefinitionsByName.set(columnDef.name, columnDef);
     });
+  }
 
-    // Re-render the rows if any of their columns change.
-    // TODO(andrewseguin): Determine how to only re-render the rows that have their columns changed.
-    const columnChangeEvents = this._rowDefinitions.map(rowDef => rowDef.columnsChange);
-
-    takeUntil.call(merge(...columnChangeEvents), this._onDestroy).subscribe(() => {
-      // Reset the data to an empty array so that renderRowChanges will re-render all new rows.
-      this._rowPlaceholder.viewContainer.clear();
-      this._dataDiffer.diff([]);
-      this._renderRowChanges();
+  /**
+   * Check if the header or rows have changed what columns they want to display. If there is a diff,
+   * then re-render that section.
+   */
+  private _renderUpdatedColumns() {
+    // Re-render the rows when the row definition columns change.
+    this._rowDefinitions.forEach(rowDefinition => {
+      if (!!rowDefinition.getColumnsDiff()) {
+        // Reset the data to an empty array so that renderRowChanges will re-render all new rows.
+        this._dataDiffer.diff([]);
+
+        this._rowPlaceholder.viewContainer.clear();
+        this._renderRowChanges();
+      }
     });
 
-    // Re-render the header row if the columns change
-    takeUntil.call(this._headerDefinition.columnsChange, this._onDestroy).subscribe(() => {
+    // Re-render the header row if there is a difference in its columns.
+    if (this._headerDefinition.getColumnsDiff()) {
       this._headerRowPlaceholder.viewContainer.clear();
       this._renderHeaderRow();
-    });
-
-    this._renderHeaderRow();
-  }
-
-  ngAfterContentChecked() {
-    if (this.dataSource && !this._renderChangeSubscription) {
-      this._observeRenderChanges();
     }
   }