Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(table): update implicit when using trackby #7893

Merged
merged 3 commits into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/cdk/table/table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
42 changes: 29 additions & 13 deletions src/cdk/table/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -68,6 +69,12 @@ export const CDK_TABLE_TEMPLATE = `
<ng-container headerRowPlaceholder></ng-container>
<ng-container rowPlaceholder></ng-container>`;

/**
* Class used to conveniently type the embedded view ref for rows with a context.
* @docs-private
*/
abstract class RowViewRef<T> extends EmbeddedViewRef<CdkCellOutletRowContext<T>> { }

/**
* 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.
Expand Down Expand Up @@ -292,25 +299,36 @@ export class CdkTable<T> 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<any>, adjustedPreviousIndex: number, currentIndex: number) => {
if (item.previousIndex == null) {
this._insertRow(this._data[currentIndex], currentIndex);
(record: IterableChangeRecord<T>, 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 = <RowViewRef<T>>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<T>) => {
const rowView = <RowViewRef<T>>viewContainer.get(record.currentIndex!);
rowView.context.$implicit = record.item;
});
}

/**
Expand Down Expand Up @@ -353,14 +371,13 @@ export class CdkTable<T> 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<CdkCellOutletRowContext<T>>;
const viewRef = viewContainer.get(index) as RowViewRef<T>;
viewRef.context.index = index;
viewRef.context.count = count;
viewRef.context.first = index === 0;
Expand Down Expand Up @@ -404,4 +421,3 @@ export class CdkTable<T> implements CollectionViewer {
});
}
}