Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Changes from 48 commits
1e727f4
556e4de
3416c6d
60881c6
0eca467
8485fc3
47af532
ccc2f2d
87dc4b3
72e8139
754d719
ce7a4ba
9af0693
faa31d3
135ddce
9e9d2e3
b46d24b
35de340
416c4e1
ebfbf42
a5d890b
d9eb542
e24fb1d
6b58f32
1f7eb3f
c102bff
ed25571
e052ae0
69a32d1
014c2bd
de8fb1c
b5cd7d0
78b55d5
e9faa76
d9a7c02
c8253d3
18ca9a5
ec9add9
478b16a
ec83ba7
ac3fcda
cbec91f
cec0c1c
489cd2d
85ea8c4
a44df62
1c784f4
58f724e
b5a1e39
d272e3c
dbb9a20
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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. 🫠
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:
css
prop working but the imported EUI files still were broken)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
orEuiCallOut
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.