Skip to content

Commit fe56e56

Browse files
andrewseguinandrewseguin
committed
handle homogeneous data; need to write tests
1 parent 7523883 commit fe56e56

File tree

2 files changed

+147
-63
lines changed

2 files changed

+147
-63
lines changed

‎src/cdk/table/table.spec.ts

Lines changed: 117 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -133,37 +133,71 @@ describe('CdkTable', () => {
133133
});
134134
});
135135

136-
it('should use differ to add/remove/move rows', () => {
137-
// Each row receives an attribute 'initialIndex' the element's original place
138-
getRows(tableElement).forEach((row: Element, index: number) => {
139-
row.setAttribute('initialIndex', index.toString());
140-
});
136+
describe('should correctly use the differ to add/remove/move rows', () => {
137+
function addInitialIndexAttribute() {
138+
// Each row receives an attribute 'initialIndex' the element's original place
139+
getRows(tableElement).forEach((row: Element, index: number) => {
140+
row.setAttribute('initialIndex', index.toString());
141+
});
141142

142-
// Prove that the attributes match their indicies
143-
const initialRows = getRows(tableElement);
144-
expect(initialRows[0].getAttribute('initialIndex')).toBe('0');
145-
expect(initialRows[1].getAttribute('initialIndex')).toBe('1');
146-
expect(initialRows[2].getAttribute('initialIndex')).toBe('2');
143+
// Prove that the attributes match their indicies
144+
const initialRows = getRows(tableElement);
145+
expect(initialRows[0].getAttribute('initialIndex')).toBe('0');
146+
expect(initialRows[1].getAttribute('initialIndex')).toBe('1');
147+
expect(initialRows[2].getAttribute('initialIndex')).toBe('2');
148+
}
147149

148-
// Swap first and second data in data array
149-
const copiedData = component.dataSource!.data.slice();
150-
const temp = copiedData[0];
151-
copiedData[0] = copiedData[1];
152-
copiedData[1] = temp;
150+
it('when the data is heterogeneous', () => {
151+
addInitialIndexAttribute();
153152

154-
// Remove the third element
155-
copiedData.splice(2, 1);
153+
// Swap first and second data in data array
154+
const copiedData = component.dataSource!.data.slice();
155+
const temp = copiedData[0];
156+
copiedData[0] = copiedData[1];
157+
copiedData[1] = temp;
156158

157-
// Add new data
158-
component.dataSource!.data = copiedData;
159-
component.dataSource!.addData();
159+
// Remove the third element
160+
copiedData.splice(2, 1);
160161

161-
// Expect that the first and second rows were swapped and that the last row is new
162-
const changedRows = getRows(tableElement);
163-
expect(changedRows.length).toBe(3);
164-
expect(changedRows[0].getAttribute('initialIndex')).toBe('1');
165-
expect(changedRows[1].getAttribute('initialIndex')).toBe('0');
166-
expect(changedRows[2].getAttribute('initialIndex')).toBe(null);
162+
// Add new data
163+
component.dataSource!.data = copiedData;
164+
component.dataSource!.addData();
165+
166+
// Expect that the first and second rows were swapped and that the last row is new
167+
const changedRows = getRows(tableElement);
168+
expect(changedRows.length).toBe(3);
169+
expect(changedRows[0].getAttribute('initialIndex')).toBe('1');
170+
expect(changedRows[1].getAttribute('initialIndex')).toBe('0');
171+
expect(changedRows[2].getAttribute('initialIndex')).toBe(null);
172+
});
173+
174+
it('when the data is homogeneous', () => {
175+
// Change default data to be one that is a list of homogeneous data.
176+
const obj = {value: true};
177+
component.dataSource!.data = [obj, obj, obj];
178+
addInitialIndexAttribute();
179+
180+
const copiedData = component.dataSource!.data.slice();
181+
182+
// Remove the third element and add a new different obj in the beginning.
183+
copiedData.splice(2, 1);
184+
copiedData.unshift({value: false});
185+
186+
// Add new data
187+
component.dataSource!.data = copiedData;
188+
189+
// Expect that two of the three rows still have an initial index. Not as concerned about
190+
// the order they are in, but more important that there was no unnecessary removes/inserts.
191+
const changedRows = getRows(tableElement);
192+
expect(changedRows.length).toBe(3);
193+
let numInitialRows = 0;
194+
changedRows.forEach(row => {
195+
if (row.getAttribute('initialIndex') !== null) {
196+
numInitialRows++;
197+
}
198+
});
199+
expect(numInitialRows).toBe(2);
200+
});
167201
});
168202

169203
it('should clear the row view containers on destroy', () => {
@@ -529,18 +563,18 @@ describe('CdkTable', () => {
529563
});
530564

531565
it('should error if there is row data that does not have a matching row template',
532-
fakeAsync(() => {
533-
const whenRowWithoutDefaultFixture = createComponent(WhenRowWithoutDefaultCdkTableApp);
534-
const data = whenRowWithoutDefaultFixture.componentInstance.dataSource.data;
535-
expect(() => {
536-
try {
537-
whenRowWithoutDefaultFixture.detectChanges();
538-
flush();
539-
} catch {
540-
flush();
541-
}
542-
}).toThrowError(getTableMissingMatchingRowDefError(data[0]).message);
543-
}));
566+
fakeAsync(() => {
567+
const whenRowWithoutDefaultFixture = createComponent(WhenRowWithoutDefaultCdkTableApp);
568+
const data = whenRowWithoutDefaultFixture.componentInstance.dataSource.data;
569+
expect(() => {
570+
try {
571+
whenRowWithoutDefaultFixture.detectChanges();
572+
flush();
573+
} catch {
574+
flush();
575+
}
576+
}).toThrowError(getTableMissingMatchingRowDefError(data[0]).message);
577+
}));
544578

545579
it('should fail when multiple rows match data without multiTemplateRows', fakeAsync(() => {
546580
let whenFixture = createComponent(WhenRowMultipleDefaultsCdkTableApp);
@@ -552,12 +586,12 @@ describe('CdkTable', () => {
552586

553587
describe('with multiTemplateRows', () => {
554588
it('should be able to render multiple rows per data object', () => {
555-
let whenFixture = createComponent(WhenRowCdkTableApp);
556-
whenFixture.componentInstance.multiTemplateRows = true;
557-
whenFixture.detectChanges();
589+
setupTableTestApp(WhenRowCdkTableApp);
590+
component.multiTemplateRows = true;
591+
fixture.detectChanges();
558592

559-
const data = whenFixture.componentInstance.dataSource.data;
560-
expectTableToMatchContent(whenFixture.nativeElement.querySelector('cdk-table'), [
593+
const data = component.dataSource.data;
594+
expectTableToMatchContent(tableElement, [
561595
['Column A', 'Column B', 'Column C'],
562596
[data[0].a, data[0].b, data[0].c],
563597
[data[1].a, data[1].b, data[1].c],
@@ -569,12 +603,12 @@ describe('CdkTable', () => {
569603
});
570604

571605
it('should have the correct data and row indicies', () => {
572-
let whenFixture = createComponent(WhenRowCdkTableApp);
573-
whenFixture.componentInstance.multiTemplateRows = true;
574-
whenFixture.componentInstance.showIndexColumns();
575-
whenFixture.detectChanges();
606+
setupTableTestApp(WhenRowCdkTableApp);
607+
component.multiTemplateRows = true;
608+
component.showIndexColumns();
609+
fixture.detectChanges();
576610

577-
expectTableToMatchContent(whenFixture.nativeElement.querySelector('cdk-table'), [
611+
expectTableToMatchContent(tableElement, [
578612
['Index', 'Data Index', 'Render Index'],
579613
['', '0', '0'],
580614
['', '1', '1'],
@@ -584,6 +618,41 @@ describe('CdkTable', () => {
584618
['', '3', '5'],
585619
]);
586620
});
621+
622+
it('should have the correct data and row indicies when data is homogeneous', () => {
623+
setupTableTestApp(WhenRowCdkTableApp);
624+
component.multiTemplateRows = true;
625+
component.showIndexColumns();
626+
627+
const obj = {value: true};
628+
component.dataSource.data = [obj, obj, obj, obj];
629+
fixture.detectChanges();
630+
631+
expectTableToMatchContent(tableElement, [
632+
['Index', 'Data Index', 'Render Index'],
633+
['', '0', '0'],
634+
['', '1', '1'],
635+
['', '1', '2'],
636+
['', '2', '3'],
637+
['', '3', '4'],
638+
]);
639+
640+
// Push unique data on the front and add another obj to the array
641+
component.dataSource.data = [{value: false}, obj, obj, obj, obj, obj];
642+
fixture.detectChanges();
643+
644+
expectTableToMatchContent(tableElement, [
645+
['Index', 'Data Index', 'Render Index'],
646+
['', '0', '0'],
647+
['', '1', '1'],
648+
['', '1', '2'],
649+
['', '2', '3'],
650+
['', '3', '4'],
651+
['', '4', '5'],
652+
['', '5', '6'],
653+
]);
654+
655+
});
587656
});
588657
});
589658

‎src/cdk/table/table.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,11 @@ export class CdkTable<T> implements CollectionViewer, OnInit, AfterContentChecke
203203
*
204204
* Implemented as a map of maps where the first key is the `data: T` object and the second is the
205205
* `CdkRowDef<T>` object. With the two keys, the cache points to a `RenderRow<T>` object that
206-
* contains the pair.
206+
* contains an array of created pairs. The array is necessary to handle cases where the data
207+
* array contains multiple duplicate data objects and each instantiated `RenderRow` must be
208+
* stored.
207209
*/
208-
private _cachedRenderRowsMap = new Map<T, WeakMap<CdkRowDef<T>, RenderRow<T>>>();
210+
private _cachedRenderRowsMap = new Map<T, WeakMap<CdkRowDef<T>, RenderRow<T>[]>>();
209211

210212
/**
211213
* Tracking function that will be used to check the differences in data changes. Used similarly
@@ -481,10 +483,19 @@ export class CdkTable<T> implements CollectionViewer, OnInit, AfterContentChecke
481483
let data = this._data[i];
482484
const renderRowsForData = this._getRenderRowsForData(data, i, prevCachedRenderRows.get(data));
483485

484-
this._cachedRenderRowsMap.set(data, new WeakMap());
486+
if (!this._cachedRenderRowsMap.has(data)) {
487+
this._cachedRenderRowsMap.set(data, new WeakMap());
488+
}
489+
485490
for (let j = 0; j < renderRowsForData.length; j++) {
486491
let renderRow = renderRowsForData[j];
487-
this._cachedRenderRowsMap.get(renderRow.data)!.set(renderRow.rowDef, renderRow);
492+
493+
const cache = this._cachedRenderRowsMap.get(renderRow.data)!;
494+
if (cache.has(renderRow.rowDef)) {
495+
cache.get(renderRow.rowDef)!.push(renderRow);
496+
} else {
497+
cache.set(renderRow.rowDef, [renderRow]);
498+
}
488499
renderRows.push(renderRow);
489500
}
490501
}
@@ -498,12 +509,13 @@ export class CdkTable<T> implements CollectionViewer, OnInit, AfterContentChecke
498509
* `(T, CdkRowDef)` pair.
499510
*/
500511
private _getRenderRowsForData(
501-
data: T, dataIndex: number, cache?: WeakMap<CdkRowDef<T>, RenderRow<T>>): RenderRow<T>[] {
512+
data: T, dataIndex: number, cache?: WeakMap<CdkRowDef<T>, RenderRow<T>[]>): RenderRow<T>[] {
502513
const rowDefs = this._getRowDefs(data, dataIndex);
503514

504515
return rowDefs.map(rowDef => {
505-
if (cache && cache.has(rowDef)) {
506-
const dataRow = cache.get(rowDef)!;
516+
const cachedRenderRows = (cache && cache.has(rowDef)) ? cache.get(rowDef)! : [];
517+
if (cachedRenderRows.length) {
518+
const dataRow = cachedRenderRows.shift()!;
507519
dataRow.dataIndex = dataIndex;
508520
return dataRow;
509521
} else {
@@ -708,20 +720,23 @@ export class CdkTable<T> implements CollectionViewer, OnInit, AfterContentChecke
708720
*/
709721
private _updateRowIndexContext() {
710722
const viewContainer = this._rowOutlet.viewContainer;
711-
for (let index = 0, count = viewContainer.length; index < count; index++) {
712-
const viewRef = viewContainer.get(index) as RowViewRef<T>;
723+
const dataIndex = 0;
724+
725+
for (let renderIndex = 0, count = viewContainer.length; renderIndex < count; renderIndex++) {
726+
727+
const viewRef = viewContainer.get(renderIndex) as RowViewRef<T>;
713728
const context = viewRef.context as RowContext<T>;
714729
context.count = count;
715-
context.first = index === 0;
716-
context.last = index === count - 1;
717-
context.even = index % 2 === 0;
730+
context.first = renderIndex === 0;
731+
context.last = renderIndex === count - 1;
732+
context.even = renderIndex % 2 === 0;
718733
context.odd = !context.even;
719734

720735
if (this.multiTemplateRows) {
721-
context.dataIndex = this._renderRows[index].dataIndex;
722-
context.renderIndex = index;
736+
context.dataIndex = this._renderRows[renderIndex].dataIndex;
737+
context.renderIndex = renderIndex;
723738
} else {
724-
context.index = this._renderRows[index].dataIndex;
739+
context.index = this._renderRows[renderIndex].dataIndex;
725740
}
726741
}
727742
}

0 commit comments

Comments
 (0)