-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
docs (toolbars and globals): fixes an error in the documentation example #13601
Conversation
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.
@nicoleoliveira thank you for the time and effort you put into this pull request and for catching that small oversight. I've left a comment regarding it. Let me know once you've taken care of it and we'll be good to go.
Stay safe
<Story name="StoryWithLocale"> | ||
{(args, { globals: { locale } }) => { | ||
const caption = getCaptionForLocale(locale); | ||
return <>{caption}</>; | ||
return `<p>${caption}</p>`; | ||
}} | ||
</Story> |
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.
@nicoleoliveira we could leave the fragment in this case.
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.
When I left <> {caption} </>
it generated an error when compiling the storybook. I thought it was a specific syntax, I didn't understand that it could be modified. So I thought it best to put an html tag as an example to make it more intuitive. What do you think? :)
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.
@nicoleoliveira let me run the code once again and i'll get back to you.
Stay safe
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.
@nicoleoliveira it checks out. Here's a sample repo with the code and a deployed version.
Testing done:
- Development mode with
npm run storybook
and production mode withnpm run build-storybook && npm run serve-prod" (
serve-prod` is a script that i've included to emulate a production server running the statically built storybook). - Bumped versions back and all check out with the test above.
If you could emulate the error you've mentioned, please feel free to open an issue and if possible add a reproduction so that we could take a look at it. In terms of this pull request we're good with leaving in the fragment. But nonetheless we're truly thankful for catching that typo and fix it.
Let me know and we'll get this merged.
Stay safe
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.
Hi @jonniebigodes … I appreciate your attention. I think you tested in .js file, but I found the problem in the .mdx. Let me try to be clearly:
That’s the sample code in the page of the story book. In the line where we can read “return <>{caption}</>;” throws an error saying that an html tag was expected.
I was trying to say that maybe, should be clear if the docs use some html tag there to just run the code and work. As example:
return <p>${caption}</p>
;
I’m working with web-components and using mdx. If using the tags like: “<></>” it should work, so a real error is happening.
There is my repo link as an example.
Execute npm run storybook
to test.
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.
Sorry for the fumble fingering. I forgot to commit the mdx portion 🙈 . I've just pushed the updated config to the repo. But i'll keep looking into this, i'm going to clone the repo you've supplied (thank you very much for that) and see what's going on and let you know of my findings and we'll move from there.
Sounds good?
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.
Sound good! Appreciate it. :)
@nicoleoliveira as I've mentioned in the issue associated with this pull request, I've pushed a pull request that addresses the issue and probably other Storybook users find with the usage of the Toolbars and associated documentation. It goes without saying that I would like to thank you for putting this item in my radar. That said hope you don't mind but I'm closing this pull request. Stay safe |
Issue: #13602
What I did
When using the descriptive example in Toolbars and Globals page as the following example:
The errors below appear:
How to test
Use test instructions described in https://github.com/storybookjs/frontpage#docs-content .