[EuiSelectable] Enable CodeSandbox links and misc documentation improvements #6074
Conversation
cee-chen
added
documentation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6074/ |
- The local import was preventing CodeSandbox demos from being generated, and DRYness is not as much a concern here as much as having useful demos/code that people can quickly modify - .TSX source change is required for `<>` typing additions
- rename to Sizing since it's not just a popover example - move example all the way down the page - it's a more complex example and doesn't honestly feel like it makes sense where it is, and it's a - remove data store, just use more moon names - clean up unnecessary const declarations, preferring inline JS - remove need for generated IDs - this is a one time demo - remove unnecessary array sorting - it's not relevant to what's being demo'd
- remove need for data store dependency, copy countries array - default to custom content being shown - inline more code, simplify conditional props to ternaries - make secondary text display smaller - Fix `showIcons` type error: it should be passed by the list, not to individual options
- It was confusing having it amongst the EuiSelectable docs when it's its own completely separate page/documentation - gave it its own subdir with the same name that it lives in in src/components - renamed its file names to be clearer and less generic + fix onWindowKeyDown useEffect cleanup removing the wrong event + optimize window keyup/keydown fns to not re-init on every rerender
- allow consumers to toggle between `always` and `true` to observe functional difference
- Remove props that are not specifically relevant to the documentation section - Move ending bracket on newline & other misc prettier syntax cleanup
- prefer to display all props, as there are links within EuiSelectable that no longer do anything if all child props aren't displayed
- Fix broken link - Remove random documentation copy that doesn't look like it belongs - Fix import statement
cee-chen
force-pushed
the
selectable-demos
branch
from
e7c4f8e
to
c529cdb
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6074/ |
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6074/ |
There was a problem hiding this comment.
💟 Thank you for taking this on! We really need a general guidance/tool for creating fake data. This process seemed very manual. I also personally hate this Greek(?) data.
I started out with mostly suggestions, but then found a few issues too, so there's a mix of optional and necessary changes in my review.
| }, | ||
| ], | ||
| props, | ||
| snippet: `<EuiSelectable |
There was a problem hiding this comment.
Then we could have targeted snippets for each type too.
| @import './selectable/search'; | ||
| @import './selectable/selectable_templates/sitewide'; |
There was a problem hiding this comment.
Optional:
Should we move these styles to Emotion so they render in the code sandbox?
There was a problem hiding this comment.
I gave this a shot but unfortunately there are a few limitations:
- The styles being set are using EUI theme variables, meaning I have to call
useEuiTheme(), meaning I can't just pass a staticcssprop into the list ofsearchDataarray variables - Because the options are being rendered in a popover, I can't pass the
css={}prop into<EuiSelectableTemplateSitewide>directly, I have to pass it intolistProps={{}} listProps={{ css: css`...` }}works in actual UI, but Typescript complains about thecssprop not existing on our types, which is odd. We may need to addcssin ourCommonPropsdeclaration. Which I'm fine to do, but I was really hoping to keep this PR as src-doc changes only, so I'll check in with Greg about the implications of that and then tackle it in another PR, if that's cool!
src-docs/src/views/selectable/selectable_templates/sitewide_example.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
I agree especially for complex examples like our tables (which I was hoping to look at soon 🤞), but for smaller examples like these I don't mind repeating static arrays of data personally. I think it's easier to follow and for consumers to understand when explicitly laid out strings at times.
Haha FWIW It's the name of different planets' moons that just happened to be named after different Greek/Roman mythical figures! 🌝 So really, the blame here lies with astronomers |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6074/ |
Summary
Because our
EuiSelectabledemos/examples used a shareddata.tsobject, CodeSandbox links could not be generated for the demos. This was particularly annoying when attempting to whip up a quick boilerplate for debugging/testing purposes.I removed the shared
data.tsfile and simply copied and pasted theoptionsobject across examples. I also did a pass on the docs while I was here, simplifying any non-relevant logic and other misc cleanup. I recommend following along by commit.Checklist