Add rowKeyGetter to Table component. #117
Conversation
src/FixedDataTableBufferedRows.js
Outdated
| @@ -142,7 +144,7 @@ var FixedDataTableBufferedRows = React.createClass({ | |||
|
|
|||
| this._staticRowArray[i] = | |||
| <FixedDataTableRow | |||
| key={i} | |||
| key={rowKeyGetter(rowIndex)} | |||
There was a problem hiding this comment.
@wcjordan Do you think there is any risk in this change? I remember there was discussion about indexing this by rowIndex instead of its array order in the past
There was a problem hiding this comment.
I think you're thinking of PR #85 regarding recycling column keys.
I think this is fine, and may be good if we think rowIndex is more likely to allow row recycling than array index. We should sanity check though and ensure this doesn't cause any funkiness with image loading or anything like that.
If we want to be really conservative, we can also make the rowKeyGetter take 2 params, rowIndex and arrayIndex and have the default just return arrayIndex.
There was a problem hiding this comment.
Do you want me to make the change to the rowKeyGetter? Happy to do it if that's blocking a merge on this. Thanks!
There was a problem hiding this comment.
@conorbranagan Sorry for the delayed response, got busy with some work and forgot about this PR.
Can we just do something like
var rowKey = props.rowKeyGetter ? props.rowKeyGetter(rowIndex) : i
passing array index seems weird.
There was a problem hiding this comment.
No worries, thanks for looking. Updated the PR
If you set a rowKeyGetter then the `key` value for each FixedDataTableRow can be overridden based on the rowIndex. This is particularly useful when using CSS animations where you only want transitions to animate if the data source remains the same. See also: facebookarchive/fixed-data-table#353
|
LGTM |
Description
If you set a rowKeyGetter prop for the Table component then the
keyvalue for each FixedDataTableRow can be overridden based on the rowIndex.Motivation and Context
This is particularly useful when using CSS animations where you only want transitions to animate if the data source for the row remains the same. So in this case your key getter might instead send the key of the data object.
See also from the original repo: facebookarchive/fixed-data-table#353
How Has This Been Tested?
Tested in a local project using a % progress-bar style viz. Once we use the object key instead of the default we can see that animations only happen when a data changes for a row that has remained in the same location.
Types of changes
Checklist: