Skip to content
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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

docs (toolbars and globals): fixes an error in the documentation example #13601

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/snippets/common/my-component-story-use-globaltype.mdx.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ export const getCaptionForLocale = (locale) => {
case 'kr': return '안녕하세요!';
case 'zh': return '你好!';
default:
return 'Hello!',
return 'Hello!';
}
}

<Story name="StoryWithLocale">
{(args, { globals: { locale } }) => {
const caption = getCaptionForLocale(locale);
return <>{caption}</>;
return `<p>${caption}</p>`;
}}
</Story>
Comment on lines 15 to 20
Copy link
Contributor

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.

Copy link
Author

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? :)

Copy link
Contributor

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

Copy link
Contributor

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 with npm 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

Copy link
Author

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:

Captura de tela de 2021-01-18 09-12-29

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.

Copy link
Contributor

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sound good! Appreciate it. :)

```