feat(data-table): add trackby input #5097
Conversation
googlebot
added
the
cla: yes
|
|
||
| <span>Track By</span> | ||
| <md-radio-group [(ngModel)]="trackBy"> | ||
| <md-radio-button [value]="'id'"> ID </md-radio-button> |
There was a problem hiding this comment.
Why are there extra spaces around the text?
| @@ -7,13 +7,13 @@ import { | |||
| ContentChildren, | |||
| Directive, | |||
| ElementRef, | |||
| Input, | |||
| Input, isDevMode, | |||
|
|
||
| constructor() { | ||
| for (let i = 0; i < 100; i++) { this.addPerson(); } | ||
| } | ||
|
|
||
| randomize(changeReferences: boolean) { |
| @@ -138,6 +138,119 @@ describe('CdkTable', () => { | |||
| expect(changedRows[2].getAttribute('initialIndex')).toBe(null); | |||
| }); | |||
|
|
|||
| describe('should properly use trackBy when diffing to add/remove/move rows', () => { | |||
There was a problem hiding this comment.
The describe can just be with trackBy (only individual tests are should...)
| ` | ||
| }) | ||
| class TrackByCdkTableApp { | ||
| static TRACK_BY: 'reference' | 'propertyA' | 'index' = 'reference'; |
There was a problem hiding this comment.
Why is this static?
Also, even if it is static, CAPS_CASE is for constants (which this isn't)
There was a problem hiding this comment.
It cannot be a component property because the trackBy function does not run in the context of the component (cannot use this). Fixed the caps case, thought I saw a lint error regarding the format, but I must have confused it with a different property.
Static because the trackBy function does not have this as TrackByCdkTableApp so it cannot be
| TrackByCdkTableApp.TRACK_BY = 'reference'; | ||
| }); | ||
|
|
||
| function createComponent() { |
There was a problem hiding this comment.
createTestComponentWithTrackyByTable?
| } | ||
|
|
||
| // Swap first two elements, remove the third, add new data | ||
| function changeData() { |
|
Addressed the comments, thanks for the review. |
andrewseguin
force-pushed
the
table-track-by
branch
3 times, most recently
from
6248287 to
a11e70f
Compare
| @@ -21,7 +21,12 @@ describe('CdkTable', () => { | |||
|
|
|||
| TestBed.configureTestingModule({ | |||
| imports: [CdkDataTableModule], | |||
| declarations: [SimpleCdkTableApp, DynamicDataSourceCdkTableApp, CustomRoleCdkTableApp], | |||
| declarations: [ | |||
| SimpleCdkTableApp, | |||
andrewseguin
force-pushed
the
table-track-by
branch
from
a11e70f to
d8751ea
Compare
|
Indention fixed - added merge-ready |
andrewseguin
force-pushed
the
table-track-by
branch
from
d8751ea to
25a4773
Compare
|
PR has been rebased |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.