FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Missing key prop warning for ChartPie based components by dlabrecq · Pull Request #2943 · patternfly/patternfly-react · GitHub

Missing key prop warning for ChartPie based components - #2943

Merged
tlabaj merged 1 commit into
patternfly:masterfrom
dlabrecq:2938-missing-key-prop-warn
Sep 23, 2019
Merged

Missing key prop warning for ChartPie based components#2943
tlabaj merged 1 commit into
patternfly:masterfrom
dlabrecq:2938-missing-key-prop-warn

Conversation

dlabrecq commented Sep 16, 2019
edited
Loading

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Collaborator

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

dlabrecq force-pushed the 2938-missing-key-prop-warn branch 4 times, most recently from 7204a2f to 17eca05 Compare September 17, 2019 14:10
tlabaj requested a review from redallen September 17, 2019 14:58
dlabrecq force-pushed the 2938-missing-key-prop-warn branch 11 times, most recently from ba451c9 to 95d520b Compare September 19, 2019 21:31
dlabrecq force-pushed the 2938-missing-key-prop-warn branch from 95d520b to e0e42e0 Compare September 19, 2019 21:48
dlabrecq removed the request for review from TheRealJon September 23, 2019 13:33

AllenBW left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

<3

redallen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

tlabaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

tlabaj merged commit 219b00b into patternfly:master Sep 23, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing key prop console warning for ChartPie and related components

6 participants


Back | FazBrowse Home | New Git URL