Fix: Each child in a list should have a unique "key" prop#5443
Fix: Each child in a list should have a unique "key" prop#5443preeesha wants to merge 11 commits intoRocketChat:developfrom
Conversation
|
Hey @GleidsonDaniel, I have made some fixes to the mentioned issue. When you have a moment, would you kindly review the changes I have made and provide any feedback? Please let me know if you have any questions or need any clarification on the updates. Thank you for your time and consideration. Regards, |
| {fields.map(field => ( | ||
| <Text style={[styles.text, styles.field, { color: themes[theme].bodyText }]}>{parser.text(field)}</Text> | ||
| {fields.map((field, index) => ( | ||
| <Text key={index} style={[styles.text, styles.field, { color: themes[theme].bodyText }]}> |
There was a problem hiding this comment.
https://react.dev/learn/rendering-lists#why-does-react-need-keys
See pitfall section
There was a problem hiding this comment.
Okay sir. Thanks for pointing that out.
There was a problem hiding this comment.
This issue has not yet been resolved.
There was a problem hiding this comment.
Sir I find this one trivial as the field is of type any which means that using this as the key it can cause major bugs in future so that's the reason I have made use of the index here.
I would love to know suggestions.
preeesha
force-pushed
the
fix--render-key-errors
branch
from
0f774bc to
2cccebe
Compare
|
@GleidsonDaniel Sir I have made the necessary changes and reviewed the review. You can have a look. Thanks for your time |
| {fields.map(field => ( | ||
| <Text style={[styles.text, styles.field, { color: themes[theme].bodyText }]}>{parser.text(field)}</Text> | ||
| {fields.map((field, index) => ( | ||
| <Text key={index} style={[styles.text, styles.field, { color: themes[theme].bodyText }]}> |
There was a problem hiding this comment.
This issue has not yet been resolved.
| {avatarSuggestions.slice(0, 7).map(item => ( | ||
| <AvatarSuggestionItem item={item} testID={`${item?.service}-avatar-suggestion`} onPress={onPress} /> | ||
| {avatarSuggestions.slice(0, 7).map((item, index) => ( | ||
| <AvatarSuggestionItem key={index} item={item} testID={`${item?.service}-avatar-suggestion`} onPress={onPress} /> |
There was a problem hiding this comment.
Sir, now I have made use of the url of the avatar instead of indices as key as it was the most feasibly unique thing that differentiates each avatar better.
Let me know if you disagree with this.
preeesha
left a comment
preeesha
left a comment
There was a problem hiding this comment.
I have reviewed the suggestions and left the comments in the respective conversation.
Kindly provide your views on them.
|
Hey @preeesha, are you still working on this PR? |
|
Closing this PR because it has multiple conflicts, hasn’t been updated for over a year and we now have a new PR #6775 |
4 participants
Proposed changes
This PR pays attention to
keyprop missing at many places where items are rendered under themapfunction. I have added theykeyprop to improve performance and fix the mentioned issue.Issue(s)
#4943
Types of changes
Checklist