-
Notifications
You must be signed in to change notification settings - Fork 841
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] Add story code snippets #7716
[Storybook] Add story code snippets #7716
Conversation
6c1c416
to
01bfb67
Compare
e144ec5
to
6d3413d
Compare
packages/eui/src/components/header/header_alert/header_alert.stories.tsx
Outdated
Show resolved
Hide resolved
41f4c6a
to
e76dd6f
Compare
<SyntaxHighlighter | ||
language="tsx" | ||
copyable | ||
padded | ||
showLineNumbers={false} | ||
wrapLongLines | ||
> | ||
{code} | ||
</SyntaxHighlighter> |
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.
Any thoughts about dogfooding EuiCodeBlock
for this instead of using Storybook's component? 👀
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.
Yeah that's a good idea! I think I briefly had that thought while initially building this but went with the Storybook component just because it works out of the box (as they use it for their docs source code) and saved me some time 😅.
Let me have another look how it would work with our component 👍
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.
Yeah I realize now why I took the Storybook component. There are issues using the EUI components because emotion is not properly resolved somehow for the addon panel. 🫠
You have tried to stringify object returned from `css` function. It isn't supposed to be used directly (e.g. as value of the `className` prop), but rather handed to emotion so it can handle it (e.g. as value of `css` prop).
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.
Haha whoops 🙈 Yeah, Storybook probably needs the Emotion babel preset which is a whole fun setup thing (https://emotion.sh/docs/css-prop). You could the JSX pragma to see if that works?
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.
But no worries if it doesn't, I'm totally fine with the Storybook component as-is!
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.
Yeah I had a first brief look at it to set this up but it didn't work as expected and it will need some more digging. I think I'd opt for leaving it as is for now and doing the update as a standalone task because I do think it would be generally good to be able to use our component as the Storybook SyntaxHighlighter
has some limitation that it does not always properly highlighting the code.
🗒️ What I tried/noticed:
- Emotion works fine in the preview but not the manager parts
- I tried adding the emotion preset and it did absolutely nothing (it also seemed like the root babel config is not used - adding a manual config for Storybook with the preset resulted in custom components with
css
prop working but the imported EUI files still were broken) - adding the JSX pragma worked when it was set in the EUI component files that are imported, not when added in the Storybook files (which is not an option)
{code} | ||
</SyntaxHighlighter> | ||
) : ( | ||
<Container>{isLoaded ? emptyState : loadingState}</Container> |
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.
Thoughts on using EuiEmptyPrompt
or EuiCallOut
instead of this container? Am I being too extra here right 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.
No, that's a good idea! Why not reuse what we already have!
Let's see first if we can hide the panel for not available code snippets because we would not need this, if we just hide it 😄
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.
Ah wait, for the loading state it would still be used, so yes! I'll check how that'll look like 🙂
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.
ℹ️ This is blocked due to the same issue around Emotion mentioned here.
packages/eui/src/components/header/header_alert/header_alert.stories.tsx
Show resolved
Hide resolved
packages/eui/.storybook/addons/code-snippet/decorators/jsx_decorator.tsx
Show resolved
Hide resolved
packages/eui/.storybook/addons/code-snippet/decorators/utils.ts
Outdated
Show resolved
Hide resolved
packages/eui/.storybook/addons/code-snippet/components/panel.tsx
Outdated
Show resolved
Hide resolved
packages/eui/.storybook/addons/code-snippet/decorators/utils.ts
Outdated
Show resolved
Hide resolved
-adds example for EuiDatePicker story
- uses Story wrapper for ComboBox story as example
- it's now only outputting `<input type="text" />` which is even less helpful than before. We can just move ahead with extra cruft in the snippet for now, that's fine
- still useful even without full render functions
- adds check if single child is story element to prevent overrides and additional checks to ensure proper element - adds simplification for return undefined
- this will ensure the passed content is output as is instead of being executed or coerced
- ensures forwarded components are properly resolved when checking for the story component instead of being skipped
54380d3
to
58f724e
Compare
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.
Pulled this down to test and the EuiDatePicker story is working perfectly!! Seriously huge kudos for this fantastic work Lene, this is going to be an amazing tool for devs to use! 👏 🚀
- updates readme to reflect that functionality
Preview staging links for this PR:
|
💚 Build Succeeded
History
cc @mgadewoll |
Summary
https://eui.elastic.co/pr_7716/storybook/
Important
Disclaimer: This PR is the result of a spacetime/hackathon-y week and therefore mainly focused on building functionality instead of optimizing it as much as possible.
The goal of this is to improve devX by providing dynamically updated code snippets for the stories. The code snippet is generated on story load as well as when any
args
change (e.g. when controls in the controls table are being updated)This new functionality for code snippets is provided in an additional story panel next to the available panels for "Controls", "Actions" and "Interactions".
The basis for the code snippet generation is heavily inspired (copied and enhanced) on Storybooks Source block. The internal
jsxDecorator
file was copied and then adjusted to our specific needs. The main functionality to generate a string from react elements comes from thereact-element-to-jsx-string
package that is used.The overall setup
We run our custom
jsxDecorator
on every story as a decorator viapreview.tsx
. This decorator generates the code snippet asstring
and sends it via Storybooks Channel events to our newly added custom addon panel which outputs the code string using StorybooksSyntaxHighlighter
copmponent.The main updates to the
jsxDecorator
on our side are to ensure the code outputs clean and relevant code snippets.What was considered:
css
on a component in a story it will be an Emotion component)Stateful
(requires us to follow an agreed convention)_
underscore,<_Component>
is changed to<Component>
)css
attribute to ensure it is not transformed to Emotionfalse
values wherefalse
has meaning)The updates happen in different stages in
jsxDecorator
:react-element-to-jsx-string
and with which optionsreact-element-to-jsx-string
prettier
Options
Currently there are a few addon specific parameter options added with this PR that can be used under the key
codeSnippet
in theparameters
config key.Limitations
children
or any prop value)Components that make use of render functions (specifically for
children
) are currently (manually) skipped until support is added.ℹ️ Currently this implementation is using Storybooks
SyntaxHighlighter
component to output the code snippets. This works mainly well but seems to have trouble properly detecting the code parts for large snippets which results in some partially uncolored snippets. We could in the future check to replace this component with another to improve.Screenshots
skipped code snippet generation
Disclaimer
jsxDecorator
functionality yet.QA