-
Notifications
You must be signed in to change notification settings - Fork 375
Charts: box plot #8900
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
Charts: box plot #8900
Conversation
|
Preview: https://patternfly-react-pr-8900.surge.sh A11y report: https://patternfly-react-pr-8900-a11y.surge.sh |
5c62cdf to
b924866
Compare
| </div> | ||
| ``` | ||
|
|
||
| ### Green with bottom aligned legend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the title be updated to match the themeColor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Thank you
mcarrano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with this. But @mceledonia @andrew-ronaldson perhaps you should take a look to see if the styling is acceptable.
| ```js | ||
| import React from 'react'; | ||
| import { Chart, ChartAxis, ChartBoxPlot, ChartCursorFlyout, ChartCursorTooltip, ChartThemeColor, createContainer } from '@patternfly/react-charts'; | ||
| // import '@patternfly/patternfly/patternfly-charts.css'; // Required for mix-blend-mode CSS property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended as a note to help consumers or did you mean to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentional, but realized mix-blend-mode only applies to area charts. I removed the comment, thanks
andrew-ronaldson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
Created a component to wrap the Victory box plot chart with PatternFly styles and tooltip functionality.
Closes #8837