fix(list): allow setting a custom trackBy function for the underlying ngFor directive #435
Conversation
|
Deploy preview for patternfly-ng ready! Built with commit 57cdca0 |
riccardo-forina
force-pushed
the
fix-list-items-polling
branch
from
0b187e5 to
6ec1ac8
Compare
riccardo-forina
force-pushed
the
fix-list-items-polling
branch
from
6ec1ac8 to
36895b7
Compare
|
|
||
| protected getTrackBy(index: number, item: any): string { | ||
| return `track-${index}`; | ||
| } |
There was a problem hiding this comment.
This function appears to be unused.
| OnInit, | ||
| Output, | ||
| TemplateRef, | ||
| TemplateRef, TrackByFunction, |
| showCheckbox: false, | ||
| useExpandingRows: false | ||
| } as ListConfig; | ||
| } |
There was a problem hiding this comment.
Much of this config is duplicated, top of the spec. Can we reuse any of that?
| getTrackBy(index: number, item: any): any { | ||
| return index; | ||
| } | ||
|
|
There was a problem hiding this comment.
I'd like to see a new trackBy example created instead of altering the basic one. Want to ensure our basic use-case continues to work for existing applications
| getTrackBy(index: number, item: any): any { | ||
| return index; | ||
| } | ||
|
|
There was a problem hiding this comment.
Please create one, new example for "trackBy". That will help ensure existing applications continue to work as is.
|
Tested this against Syndesis UI and it works like a charm - nice enhancement |
|
Thanks for the review @dlabrecq! I have made some changes to the code, restored the examples and added a new one. |
riccardo-forina
force-pushed
the
fix-list-items-polling
branch
from
855cf99 to
00e7e3d
Compare
dlabrecq
left a comment
dlabrecq
left a comment
There was a problem hiding this comment.
Thank you for creating a new example. Just a couple nits left
| <include-content src="src/app/list/basic-list/example/list-basic-example.component.html"></include-content> | ||
| </tab> | ||
| <tab heading="typescript"> | ||
| <include-content src="src/app/list/basic-list/example/list-basic-example.component.ts"></include-content> |
There was a problem hiding this comment.
In order to show the correct example, these two links should use list-polling-example.component.ts instead of list-basic-example.component.ts.
There was a problem hiding this comment.
ah! Good catch, thanks!
| useExpandItems: true | ||
| } as ListConfig; | ||
|
|
||
| this.updateItemsInterval = setInterval(() => this.updateItems(), 2500); |
There was a problem hiding this comment.
I'm seeing an error in my IDE, which states "Type 'Timer' is not assignable to type 'number'. However, using this seems to work:
updateItemsInterval: number = <any>setInterval(() => {});
There was a problem hiding this comment.
I had it too but assumed it was something specific to my IDE (WebStorm). I'll update this to use any then.
dlabrecq
left a comment
dlabrecq
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
4 participants
It is now possible to set a custom trackBy function to be used in the underlying ngFor directive, which by default tracks changes in the items by identity.
This is useful in scenarios where the items passed to the component are periodically polled, and components with state (eg. a
pfng-actioncomponent) are used in the item's template.This should help fixing syndesisio/syndesis/issues/3121 where we have exactly this problem.