-
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
[Storybook] General consistency pass #7245
Conversation
- to try match how Storybook will yell if an argTypes is passed with invalid props
- just in case, + helpful for developer documentation
- these get spread to all stories in the file, and also get correctly typed automatically - no need to declare / spread an external variable - I totally forgot this was a thing we could do and would like to adopt the pattern going forward
- these changes are a bit more complex than the previous copy/paste commit - e.g. there's some args mixed in that are not defaults - or, e.g. all args are not defaults, but are repeated between stories for testing
- split component into separate stories for single vs. multi - consumers won't (shouldn't) be swapping types dynamically in any case, and it really angers typescript + remove `data-test-subj` param/control, we already handle this across all stories
- it was the first and one and therefore the worst one, and we are much better at writing stories now
@breehall Assigning this your way as you've been in the Storybook space these past few weeks. No rush at all on this (and of course lmk if you have any feedback/preferences on code style!) |
- remove unnecessary "multiple fixed headers" story - that logic now lives in EuiHeader and already has its own story - move flyout story to the end, since it doesn't have anything to control and it's a visual demo - move localstorage example up, since Kibana is actually using it, + trim controls to only relevant ones - remove unused controls in global CSS variable story, and fix bottom bar to respect left/right control
Preview staging links for this PR:
|
💚 Build Succeeded
History
|
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.
[Adding my comments in a single thread because I'm bad at going back and forth between pages]
- The new DRY helpers are very handy and significantly cut down the bloat and repetitiveness of hiding controls.
- I see that we have a TO DO task to figure out how to remove the "set up" text when we disable controls. That will be handy because that link just takes us to Storybook documentation right now.
- I like that the component defaults were added to the
Meta
object. It's one less thing to maintain. - Good call on splitting up the
EuiButtonGroup
stories into single select & multi select - Now that we've standardized certain patterns, we'll probably want to start a wiki and maybe even create a script to generate the story structure for us. It would probably be helpful. I can throw together some items in a draft wiki
Just to add a note, I don't think Storybook currently supports that kind of flexibility without it being a massive pain (literally having to go into either vanilla JS to manipulate the DOM or some other hacky workaround). There's a fair amount of open issues in the Storybook repo around hiding controls (if we really wanted to, contributing back to their repo might actually be the cooler option).
That would be amazing!! thanks ❤️ |
Summary
This PR is a general cleanup/consistency pass on our existing stories. The way we've written stories has evolved as we've learned more tips and tricks of Storybook's API, and this attempts to elevate all existing stories to the same level.
Adds new
hideStorybookControls()
anddisableStorybookControls()
utils so we're not copying the same{ table: { disable: true } }
or{ control: false }
over and over againStandardizes how and where we list component defaults - this should now always be in the initial component
meta.args
as opposed to defining them as separateconst
s and spreading them repeatedly.More opinionated cleanups of a handful of spicier components
As always, I recommend following along with changes by commit, as the final diff touches a lot of files and I'm doing a few different separate clean up things here.
QA
General checklist
N/A, dev only