[EuiEmptyPrompt] Create Storybook Stories for EuiEmptyPrompt & Import Sass Styles for Components Not Using Emotion
#7057
Conversation
…mponents still using Sass.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_7057_buildkite/ |
breehall
added
skip-changelog
documentation
|
@breehall We discussed a timeboxed exploration of dynamically swapping the imported CSS file based on the light/dark mode global. Can you write up a short summary of the approaches and invesigations you took to try and get this working? |
…nt stories - Remove Multiple Actions story in favor of displaying this in the Playground
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_7057_buildkite/ |
| element: { control: 'text' }, | ||
| body: { | ||
| control: 'text', | ||
| }, |
There was a problem hiding this comment.
Actually let's go ahead and remove the body control completely. I don't think this needs configuration personally, it flips just fine between text vs JSX.
| element: { control: 'text' }, | |
| body: { | |
| control: 'text', | |
| }, | |
| element: { control: 'text' }, |
There was a problem hiding this comment.
Should body in the playground remain a string? Or should we move it back to a <p> since the Page Template story is using multiple paragraphs?
There was a problem hiding this comment.
The playground should have as editable of controls as possible, so the body there should remain a string.
Other stories that are not the playground do not (necessarily) need to have this concern. They could only care about a certain subset of props being editable, or they could be simply demonstrating a state that is difficult to obtain with Storybook's controls, and not need to have any editable controls.
So body should be a string for the playground, and JSX for the page template demo.
| }, | ||
| }; | ||
|
|
||
| export const PageTemplate: Story = { |
There was a problem hiding this comment.
This story ended up changing too much with the changes implemented. The PageTemplate story was fine as-is and in fact shouldn't be too similar to the Playground - if it is, what's the point of it?
Let's move the ...componentDefaults to the start of the args and then override args as necessary (e.g. layout="horizontal") to get this demo back to parity with where it was / the example on EUI docs.
There was a problem hiding this comment.
The PageTemplate story was fine as-is and in fact shouldn't be too similar
I think the direction to DRY out the defaults caused a little confusion on my part here.
… used and does not affect the component. This was realized in the creation of the Storybook documentation for EuiEmptyPropmt
…ove custom argTypes for the body and element props
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_7057_buildkite/ |
There was a problem hiding this comment.
Changes look great! I have one minor/optional order nit left, but overall feel like these stories are in solid shape. I know there was some back and forth here, so I just want to TL;DR some of the items we discussed:
- Playground stories should have as editable of controls as possible
- For some props that take React nodes, this means preferring strings instead, as Storybook has no JSX editing.
- Storybook unfortunately does not inherit default props information. For playground stories, we should manually set those default prop values via
args, to indicate to viewing devs what the component defaults are at a glance. - If a playground control is not working or doesn't belong in the component, investigate why - the prop may need a different control type, or should be removed
- Additional stories should have a distinct reason for being separate from the playground. They typically demonstrate a visual state that is difficult to obtain from Storybook's controls alone. Their controls do not need to be as editable as the playground.
If I've missed anything in the above, please feel free to add on!
Co-authored-by: Cee Chen <[email protected]>
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_7057_buildkite/ |
💚 Build Succeeded
History
|
Summary
EuiEmptyPrompt. This creates three stories:EmptyPromptwith multiple actionsEmptyPromptinside of aPageTemplateI chose not to create stories for the panel options, title sizes, icon colors, and loading/error states as these can be replicated with toggles within the Playground.
This is my first set of stories, so I would love feedback / recommendations on how the stories are created.
QA
gh pr checkout 7057yarn && yarn storybookEuiEmptyPromptstoriesSass Import Exploration Summary
Problem Statement: EUI components are currently styled in two different formats (CSS-in-JS with Emotion and Sass). For components that have not been converted to Emotion, we need a way to dynamically import light and dark theme Sass styles to Storybook.
I time-boxed myself to two hours of research and testing. Here are a few resources I used to attempt to solve this problem.
@storybook/addon-stylingaddon-stylingis a package created by the Storybook team that simplifies styling components. This felt like the most promising option because the implementation only requires a few line of configuration within ourstorybook/main.js.After installing and completing configuration steps, I found that the Sass components still weren't being styled properly. The components would show in Storybook, but wouldn't update when props were changed. Upon additional research, I found that using this configuration would look for Sass styling for each of our components.
My next search led me to creating a custom decorator by using
withThemeFromJSXProvider. This decorator allows you to specify your themes and global styles. I this is where I paused my research as I was continuing to dig into rabbit holes. If I were to pick up research again, this is likely the place I would start.Additional Links & Research:
addon-stylingwith SasswithThemeFromJSXProviderdecorator