-
Notifications
You must be signed in to change notification settings - Fork 843
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
[EUI+] Fix/update various docs examples to match production #8118
Conversation
…nd Emotion conversion
- fix erroring toast list example - add a DRY preview wrapper that sets a single maxWidth
- it doesn't do anything now and honestly it doesn't need a whole example, props table is sufficient documentation
- keep this one instead of deleting it, since these APIs are from the react-datepicker library
+ replace div w/ fragment
+ remove Fragments
+ make the layout more readable for narrower width
- "fix" `locale` demo by commenting out imports (matches EuiDatePicker locale demo) - docusaurus supports modular imports - add missing custom rendering quick select panels demo with dynamic snippet + remove Fragments
- inline tabbed content instead of importing it
with a custom selector for better source
…ed examples like in prod
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.
🚢 🐈⬛ Thanks for the effort to check the docs and not only fix issues but also update/improve them!
While the layout will adjust properly for wrapping titles, they do not | ||
look particularly good. Similarily, do not use a whole lot of text in your | ||
body either. At a certain point people will not have enough time to read | ||
these things. Like, you probably are not even reading this now. |
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.
Lol, I see this for the first time 😆
<EuiButton | ||
color="danger" | ||
iconType="trash" | ||
onClick={deleteSelectedUsers} |
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.
Not a change request: I noticed a general quirkiness here (also reproducible in current docs):
When removing selected users multiple times it eventually ends in a state where there is only disabled users left (because disabled users won't be selected and deleted) and nothing can be selected anymore in the example and it needs a page reload 😅
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.
Right yeah, we just needed an example of what a user could do with selected rows, the deleting isn't supposed to be the main focus of the demo. Maybe "email users" or something would be a better mock action instead? But someone else can change that instead of me 😛
Preview staging links for this PR:
|
💚 Build Succeeded
History
|
Summary
Updates various docs examples to match prod + fixes others that were erroring/broken.
There's a lot going on here, so I'd recommend following along by commit.
QA
Updated from prod
Fixed broken demos
General checklist
N/A, docs-only