Skip to content

feat(data-table): use iterate differ to optimize rendering #4823

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

Merged
merged 5 commits into from
Jun 5, 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
46 changes: 39 additions & 7 deletions src/lib/core/data-table/data-table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
import {Component, ViewChild} from '@angular/core';
import {CdkTable} from './data-table';
import {CollectionViewer, DataSource} from './data-source';
import {CommonModule} from '@angular/common';
import {Observable} from 'rxjs/Observable';
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
import {customMatchers} from '../testing/jasmine-matchers';
Expand All @@ -13,7 +12,7 @@ describe('CdkTable', () => {

let component: SimpleCdkTableApp;
let dataSource: FakeDataSource;
let table: CdkTable;
let table: CdkTable<any>;
let tableElement: HTMLElement;

beforeEach(async(() => {
Expand Down Expand Up @@ -97,14 +96,47 @@ describe('CdkTable', () => {
});
});

it('should use differ to add/remove/move rows', () => {
// Each row receives an attribute 'initialIndex' the element's original place
getRows(tableElement).forEach((row: Element, index: number) => {
row.setAttribute('initialIndex', index.toString());
});

// Prove that the attributes match their indicies
const initialRows = getRows(tableElement);
expect(initialRows[0].getAttribute('initialIndex')).toBe('0');
expect(initialRows[1].getAttribute('initialIndex')).toBe('1');
expect(initialRows[2].getAttribute('initialIndex')).toBe('2');

// Swap first and second data in data array
const copiedData = component.dataSource.data.slice();
const temp = copiedData[0];
copiedData[0] = copiedData[1];
copiedData[1] = temp;

// Remove the third element
copiedData.splice(2, 1);

// Add new data
component.dataSource.data = copiedData;
component.dataSource.addData();

// Expect that the first and second rows were swapped and that the last row is new
const changedRows = getRows(tableElement);
expect(changedRows.length).toBe(3);
expect(changedRows[0].getAttribute('initialIndex')).toBe('1');
expect(changedRows[1].getAttribute('initialIndex')).toBe('0');
expect(changedRows[2].getAttribute('initialIndex')).toBe(null);
});

// TODO(andrewseguin): Add test for dynamic classes on header/rows

it('should match the right table content with dynamic data', () => {
let initialDataLength = dataSource.data.length;
const initialDataLength = dataSource.data.length;
expect(dataSource.data.length).toBe(3);
let headerContent = ['Column A', 'Column B', 'Column C'];
const headerContent = ['Column A', 'Column B', 'Column C'];

let initialTableContent = [headerContent];
const initialTableContent = [headerContent];
dataSource.data.forEach(rowData => initialTableContent.push([rowData.a, rowData.b, rowData.c]));
expect(tableElement).toMatchTableContent(initialTableContent);

Expand All @@ -114,7 +146,7 @@ describe('CdkTable', () => {
fixture.detectChanges();
fixture.detectChanges();

let changedTableContent = [headerContent];
const changedTableContent = [headerContent];
dataSource.data.forEach(rowData => changedTableContent.push([rowData.a, rowData.b, rowData.c]));
expect(tableElement).toMatchTableContent(changedTableContent);
});
Expand Down Expand Up @@ -190,7 +222,7 @@ class SimpleCdkTableApp {
dataSource: FakeDataSource = new FakeDataSource();
columnsToRender = ['column_a', 'column_b', 'column_c'];

@ViewChild(CdkTable) table: CdkTable;
@ViewChild(CdkTable) table: CdkTable<TestData>;
}

function getElements(element: Element, query: string): Element[] {
Expand Down
52 changes: 40 additions & 12 deletions src/lib/core/data-table/data-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import {
ContentChildren,
Directive,
Input,
IterableChangeRecord,
IterableDiffer,
IterableDiffers,
NgIterable,
QueryList,
ViewChild,
ViewContainerRef,
Expand Down Expand Up @@ -38,7 +42,7 @@ export class HeaderRowPlaceholder {
}

/**
* A data table that connects with a data source to retrieve data and renders
* 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.
*/
@Component({
Expand All @@ -54,12 +58,12 @@ export class HeaderRowPlaceholder {
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class CdkTable implements CollectionViewer {
export class CdkTable<T> implements CollectionViewer {
/**
* 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).
*/
@Input() dataSource: DataSource<any>;
@Input() dataSource: DataSource<T>;

// TODO(andrewseguin): Remove max value as the end index
// and instead calculate the view on init and scroll.
Expand All @@ -76,6 +80,9 @@ export class CdkTable implements CollectionViewer {
*/
private _columnDefinitionsByName = new Map<string, CdkColumnDef>();

/** Differ used to find the changes in the data provided by the data source. */
private _dataDiffer: IterableDiffer<T> = null;

// Placeholders within the table's template where the header and data rows will be inserted.
@ViewChild(RowPlaceholder) _rowPlaceholder: RowPlaceholder;
@ViewChild(HeaderRowPlaceholder) _headerRowPlaceholder: HeaderRowPlaceholder;
Expand All @@ -92,9 +99,14 @@ export class CdkTable implements CollectionViewer {
/** Set of templates that used as the data row containers. */
@ContentChildren(CdkRowDef) _rowDefinitions: QueryList<CdkRowDef>;

constructor(private _changeDetectorRef: ChangeDetectorRef) {
constructor(private readonly _differs: IterableDiffers,
private readonly _changeDetectorRef: ChangeDetectorRef) {
console.warn('The data table is still in active development ' +
'and should be considered unstable.');

// TODO(andrewseguin): Add trackby function input.
// Find and construct an iterable differ that can be used to find the diff in an array.
this._dataDiffer = this._differs.find([]).create();
}

ngOnDestroy() {
Expand Down Expand Up @@ -122,12 +134,8 @@ export class CdkTable implements CollectionViewer {
// TODO(andrewseguin): If the data source is not
// present after view init, connect it when it is defined.
// TODO(andrewseguin): Unsubscribe from this on destroy.
this.dataSource.connect(this).subscribe((rowsData: any[]) => {
// TODO(andrewseguin): Add a differ that will check if the data has changed,
// rather than re-rendering all rows
this._rowPlaceholder.viewContainer.clear();
rowsData.forEach(rowData => this.insertRow(rowData));
this._changeDetectorRef.markForCheck();
this.dataSource.connect(this).subscribe((rowsData: NgIterable<T>) => {
this.renderRowChanges(rowsData);
});
}

Expand All @@ -146,11 +154,31 @@ export class CdkTable implements CollectionViewer {
CdkCellOutlet.mostRecentCellOutlet.context = {};
}

/** Check for changes made in the data and render each change (row added/removed/moved). */
renderRowChanges(dataRows: NgIterable<T>) {
const changes = this._dataDiffer.diff(dataRows);
if (!changes) { return; }

changes.forEachOperation(
(item: IterableChangeRecord<any>, adjustedPreviousIndex: number, currentIndex: number) => {
if (item.previousIndex == null) {
this.insertRow(dataRows[currentIndex], currentIndex);
} else if (currentIndex == null) {
this._rowPlaceholder.viewContainer.remove(adjustedPreviousIndex);
} else {
const view = this._rowPlaceholder.viewContainer.get(adjustedPreviousIndex);
this._rowPlaceholder.viewContainer.move(view, currentIndex);
}
});

this._changeDetectorRef.markForCheck();
}

/**
* Create the embedded view for the data row template and place it in the correct index location
* within the data row view container.
*/
insertRow(rowData: any) {
insertRow(rowData: T, index: number) {
// TODO(andrewseguin): Add when predicates to the row definitions
// to find the right template to used based on
// the data rather than choosing the first row definition.
Expand All @@ -161,7 +189,7 @@ export class CdkTable implements CollectionViewer {

// TODO(andrewseguin): add some code to enforce that exactly one
// CdkCellOutlet was instantiated as a result of `createEmbeddedView`.
this._rowPlaceholder.viewContainer.createEmbeddedView(row.template, context);
this._rowPlaceholder.viewContainer.createEmbeddedView(row.template, context, index);

// Insert empty cells if there is no data to improve rendering time.
CdkCellOutlet.mostRecentCellOutlet.cells = rowData ? this.getCellTemplatesForRow(row) : [];
Expand Down