-
Notifications
You must be signed in to change notification settings - Fork 357
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
feat(charts): skeletons #10311
feat(charts): skeletons #10311
Conversation
Preview: https://patternfly-react-pr-10311.surge.sh A11y report: https://patternfly-react-pr-10311-a11y.surge.sh |
750b5c8
to
7f1165c
Compare
This looks great! Thanks so much! |
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.
Looks good.
We'll need to create a follow up issue for v6 tokens in Design and dev repos.
Thanks for running with this Dan!
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.
Very cool!
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.
Just one little nit
import chart_bullet_qualitative_range_ColorScale_400 from '@patternfly/react-tokens/dist/esm/chart_bullet_qualitative_range_ColorScale_400'; | ||
import chart_bullet_qualitative_range_ColorScale_500 from '@patternfly/react-tokens/dist/esm/chart_bullet_qualitative_range_ColorScale_500'; | ||
|
||
// import global_Color_100 from '@patternfly/react-tokens/dist/esm/global_Color_100'; |
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.
Can we remove this commented code?
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.
Done
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
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
The UX design team wants to show chart skeletons in place of a generic empty state.
I created a new theme to fill-in all labels grey, but also removed chart interactions like tooltips, cursors, etc.
Developers can provide their own fake data to represent their graphs or copy the examples provided.
For UX design and mocks, please see #10310
Examples
https://patternfly-react-pr-10311.surge.sh/charts/skeletons