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

Missing key prop warning for ChartPie based components #2943

Merged
merged 1 commit into from Sep 23, 2019

Conversation

@dlabrecq
Copy link
Member

dlabrecq commented Sep 16, 2019

This should help eliminate the "missing key prop" warning seem in the browser console.

Fixes #2938

  • Updated pie, donut, donut utilization, donut threshold to clone their container differently
  • Updated all other components to clone containers similarly
  • Added a key to the donut threshold's children
  • Updated the ChartLegendWrapper helper so it does not iterate over a child map; thus, eliminating the need for a key.
  • Fixed the "monthly stacked bar chart" example, which generated a missing key prop warning due to its array usage.
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 16, 2019

PatternFly-React preview: https://patternfly-react-pr-2943.surge.sh

@dlabrecq dlabrecq force-pushed the dlabrecq:2938-missing-key-prop-warn branch 4 times, most recently from 7204a2f to 17eca05 Sep 17, 2019
@tlabaj tlabaj requested a review from redallen Sep 17, 2019
@dlabrecq dlabrecq force-pushed the dlabrecq:2938-missing-key-prop-warn branch 11 times, most recently from ba451c9 to 95d520b Sep 17, 2019
@dlabrecq dlabrecq force-pushed the dlabrecq:2938-missing-key-prop-warn branch from 95d520b to e0e42e0 Sep 19, 2019
@dlabrecq dlabrecq removed the request for review from TheRealJon Sep 23, 2019
Copy link
Contributor

AllenBW left a comment

<3

Copy link
Contributor

redallen left a comment

I like this refactor and doing away with let currentId = 0;.

@tlabaj
tlabaj approved these changes Sep 23, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@tlabaj tlabaj merged commit 219b00b into patternfly:master Sep 23, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@dlabrecq dlabrecq deleted the dlabrecq:2938-missing-key-prop-warn branch Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.