From f8062865f757cb779df123e74607d233c8a9468d Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Fri, 27 Oct 2017 09:55:31 -0700 Subject: [PATCH] fix(table): update implicit when using trackby (#7893) * fix(table): update implicit when using trackby * imports * syntax change --- src/cdk/table/table.spec.ts | 18 ++++++++++++++++ src/cdk/table/table.ts | 42 +++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/cdk/table/table.spec.ts b/src/cdk/table/table.spec.ts index 72d26ac3d67..6c4bbbd6dd4 100644 --- a/src/cdk/table/table.spec.ts +++ b/src/cdk/table/table.spec.ts @@ -391,6 +391,24 @@ describe('CdkTable', () => { expect(changedRows[1].getAttribute('initialIndex')).toBe('1'); expect(changedRows[2].getAttribute('initialIndex')).toBe(null); }); + + it('should change row implicit data even when trackBy finds no changes', () => { + createTestComponentWithTrackyByTable('index'); + const firstRow = getRows(tableElement)[0]; + expect(firstRow.textContent!.trim()).toBe('a_1 b_1'); + expect(firstRow.getAttribute('initialIndex')).toBe('0'); + mutateData(); + + // Change each item reference to show that the trackby is checking the index. + // Otherwise this would cause them all to be removed/added. + trackByComponent.dataSource.data = trackByComponent.dataSource.data + .map(item => ({a: item.a, b: item.b, c: item.c})); + + // Expect the rows were given the right implicit data even though the rows were not moved. + trackByFixture.detectChanges(); + expect(firstRow.textContent!.trim()).toBe('a_2 b_2'); + expect(firstRow.getAttribute('initialIndex')).toBe('0'); + }); }); it('should match the right table content with dynamic data', () => { diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts index eb70b25f5fd..245501575bb 100644 --- a/src/cdk/table/table.ts +++ b/src/cdk/table/table.ts @@ -37,7 +37,8 @@ import {Subscription} from 'rxjs/Subscription'; import {Subject} from 'rxjs/Subject'; import {CdkCellDef, CdkColumnDef, CdkHeaderCellDef} from './cell'; import { - getTableDuplicateColumnNameError, getTableMissingMatchingRowDefError, + getTableDuplicateColumnNameError, + getTableMissingMatchingRowDefError, getTableMultipleDefaultRowDefsError, getTableUnknownColumnError } from './table-errors'; @@ -68,6 +69,12 @@ export const CDK_TABLE_TEMPLATE = ` `; +/** + * Class used to conveniently type the embedded view ref for rows with a context. + * @docs-private + */ +abstract class RowViewRef extends EmbeddedViewRef> { } + /** * A data table that connects with a data source to retrieve data of type `T` and renders * a header row and data rows. Updates the rows when new data is provided by the data source. @@ -292,25 +299,36 @@ export class CdkTable implements CollectionViewer { this._changeDetectorRef.markForCheck(); } - /** Check for changes made in the data and render each change (row added/removed/moved). */ + /** + * Check for changes made in the data and render each change (row added/removed/moved) and update + * row contexts. + */ private _renderRowChanges() { const changes = this._dataDiffer.diff(this._data); if (!changes) { return; } const viewContainer = this._rowPlaceholder.viewContainer; changes.forEachOperation( - (item: IterableChangeRecord, adjustedPreviousIndex: number, currentIndex: number) => { - if (item.previousIndex == null) { - this._insertRow(this._data[currentIndex], currentIndex); + (record: IterableChangeRecord, adjustedPreviousIndex: number, currentIndex: number) => { + if (record.previousIndex == null) { + this._insertRow(record.item, currentIndex); } else if (currentIndex == null) { viewContainer.remove(adjustedPreviousIndex); } else { - const view = viewContainer.get(adjustedPreviousIndex); + const view = >viewContainer.get(adjustedPreviousIndex); viewContainer.move(view!, currentIndex); } }); - this._updateRowContext(); + // Update the meta context of a row's context data (index, count, first, last, ...) + this._updateRowIndexContext(); + + // Update rows that did not get added/removed/moved but may have had their identity changed, + // e.g. if trackBy matched data on some property but the actual data reference changed. + changes.forEachIdentityChange((record: IterableChangeRecord) => { + const rowView = >viewContainer.get(record.currentIndex!); + rowView.context.$implicit = record.item; + }); } /** @@ -353,14 +371,13 @@ export class CdkTable implements CollectionViewer { } /** - * Updates the context for each row to reflect any data changes that may have caused - * rows to be added, removed, or moved. The view container contains the same context - * that was provided to each of its cells. + * Updates the index-related context for each row to reflect any changes in the index of the rows, + * e.g. first/last/even/odd. */ - private _updateRowContext() { + private _updateRowIndexContext() { const viewContainer = this._rowPlaceholder.viewContainer; for (let index = 0, count = viewContainer.length; index < count; index++) { - const viewRef = viewContainer.get(index) as EmbeddedViewRef>; + const viewRef = viewContainer.get(index) as RowViewRef; viewRef.context.index = index; viewRef.context.count = count; viewRef.context.first = index === 0; @@ -404,4 +421,3 @@ export class CdkTable implements CollectionViewer { }); } } -