Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix non-unique keys error inside pf_aggregate_status_card #7295

Conversation

llivne
Copy link

@llivne llivne commented Sep 8, 2020

The following warning raised once we tried to implement new features that use the generic PfAggregateStatusCard.

Warning: Encountered two children with the same key, %s. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.%s,[object Object], in p (created by PfAggregateStatusCard) in div (created by PfAggregateStatusCard) in div (created by PfAggregateStatusCard) in PfAggregateStatusCard (created by AggregateStatusCard) in div (created by AggregateStatusCard) in div (created by AggregateStatusCard) in AggregateStatusCard in Provider

error

The error is because key={notification} returns string of notification with the value of object, and therefore for several objects we get the same key. we therefore suggest to change the key to the map index which is unique.

@skateman
Copy link
Member

skateman commented Sep 8, 2020

Can you please move it from draft to a PR? We usually don't do drafts, just prefix the PR title with [WIP] if it's not ready yet.

@llivne llivne marked this pull request as ready for review September 8, 2020 09:28
@skateman
Copy link
Member

skateman commented Sep 8, 2020

Changes look good to me, can you please squash the two commits into a single one?

@skateman skateman self-assigned this Sep 8, 2020
@llivne llivne force-pushed the liranl/SDE-1360_fix_non_unique_keys_bug_inside_pf_aggregate_status_card branch from 0536323 to 0e82aab Compare September 8, 2020 11:29
@llivne
Copy link
Author

llivne commented Sep 8, 2020

Changes look good to me, can you please squash the two commits into a single one?

squashed :)

@skateman
Copy link
Member

skateman commented Sep 9, 2020

Looks great, can you squash the commits? There's no point in a 2 line PR to have 2 commits.

Update app/javascript/components/pf_aggregate_status_card.jsx

Co-authored-by: Halász Dávid <[email protected]>

Update app/javascript/components/pf_aggregate_status_card.jsx

Co-authored-by: Halász Dávid <[email protected]>
@llivne llivne force-pushed the liranl/SDE-1360_fix_non_unique_keys_bug_inside_pf_aggregate_status_card branch from 47b3b31 to 9f1008f Compare September 9, 2020 06:01
@miq-bot
Copy link
Member

miq-bot commented Sep 9, 2020

Checked commit Autosde@9f1008f with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Seal of Approval

@skateman skateman merged commit 02d7da8 into ManageIQ:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants