feat(table): add fullscreen empty state demo #7371
Conversation
jenny-s51
changed the title
jenny-s51
changed the title
|
Preview: https://patternfly-react-pr-7371.surge.sh A11y report: https://patternfly-react-pr-7371-a11y.surge.sh |
mcarrano
left a comment
mcarrano
left a comment
There was a problem hiding this comment.
@jenny-s51 Just a couple of requests...
Can we make all three demos for empty state full screen? Seems odd that only the first one is. Also, the third demo (Error) should include an icon as in the design guidelines here: https://www.patternfly.org/v4/components/empty-state/design-guidelines#back-end-failure
@mmenestr please also take a look at this one to see if there is anything else we should update.
|
Don't see anything else - agree with @mcarrano's comments, otherwise looks good! |
|
Thank you Matt, yes per your comment in the loading state PR: #7370 (review), I am adding the empty state demos separately and should have the error state up soon! Thank you @mcarrano @mmenestr |
| ```js isFullscreen | ||
| import React from 'react'; | ||
| import { | ||
| Bullseye, |
There was a problem hiding this comment.
I don't think you need the bullseye
There was a problem hiding this comment.
@mattnolting good call, I think that's from before the empty state centered its contents. We can remove it from the core loading and empty state demos, too. patternfly/patternfly#4872
@jenny-s51 I added this issue for any kinds of small changes like this that we need to push back to core.
mcarrano
left a comment
mcarrano
left a comment
There was a problem hiding this comment.
@jenny-s51 I think this is fine. I do notice subtle differences when I switch between this and the HTML demo. I think there are mostly due to navigation and masthead differences. Is this part us what we still need to sync up on for parity? It would be great if we can get to a common and consistent page shell.
@mcarrano we have an issue open to update the dashboard wrapper to match core. I think once this is done, the issues you are noticing will be resolved. |
mcarrano
left a comment
mcarrano
left a comment
There was a problem hiding this comment.
@mcarrano we have an issue open to update the dashboard wrapper to match core. I think once this is done, the issues you are noticing will be resolved.
In that case, I approve! Thanks @jenny-s51 .
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
8 participants
What: Closes #7343
Convenience link to demo: https://patternfly-react-pr-7371.surge.sh/components/table/react-demos/#empty