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.

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

Jump to bottom

CLI: Fix import of Button in react mdx template #12252

Merged
merged 1 commit into from Aug 27, 2020
Merged

CLI: Fix import of Button in react mdx template #12252

merged 1 commit into from Aug 27, 2020

Conversation

patsplat
Copy link
Contributor

@patsplat patsplat commented Aug 26, 2020 •
edited

Issue: #12253

What I did

Fixed import of Button in react mdx example

How to test

  1. Follow directions in react mdx example fails when adding second storybook config to project #12253 to reproduce error
  2. Replace the generated 1-Button.stories.mdx with the version in this PR
  3. Confirm that the error has been fixed.

@patsplat patsplat mentioned this pull request Aug 26, 2020
Closed
@shilman
Copy link
Member

shilman commented Aug 26, 2020

@tooppaaa what do you think we should do here?

shilman
shilman approved these changes Aug 26, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment •
edited

Choose a reason for hiding this comment

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

Thanks @patsplat! Looks good to me. We spent a bunch of time improving the CSF CLI templates for 6.0 and apparently broke the MDX templates. 😢

tooppaaa
tooppaaa approved these changes Aug 26, 2020
View reviewed changes
Copy link
Contributor

@tooppaaa tooppaaa left a comment

Choose a reason for hiding this comment

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

All good !

I'm on the other hand wondering if this kind of option in the CLI is still relevant ?

@patsplat
Copy link
Contributor Author

I'm on the other hand wondering if this kind of option in the CLI is still relevant ?

Yes the output of sb init -- i.e. Introduction.stories.mdx -- would have been sufficient.

Removing the option of --story-format mdx is a bit beyond my contribution comfort level, at least at this point.

@shilman
Copy link
Member

shilman commented Aug 27, 2020

@tooppaaa yeah removing the MDX format is exactly what i wanted to discuss. let's merge this for now and discuss that separately.

@shilman shilman changed the title Fix import of Button in react mdx example CLI: Fix import of Button in react mdx template Aug 27, 2020
@shilman shilman merged commit a0f0133 into storybookjs:next Aug 27, 2020
@patsplat
Copy link
Contributor Author

Thank you @shilman and @tooppaaa for the rapid review. Very pleasant experience as a contributor.

@tooppaaa tooppaaa mentioned this pull request Aug 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shilman shilman shilman approved these changes

@tooppaaa tooppaaa tooppaaa approved these changes

@ndelangen ndelangen Awaiting requested review from ndelangen

@stijnkoopal stijnkoopal Awaiting requested review from stijnkoopal

Assignees
No one assigned
Labels
bug cli
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@patsplat @shilman @tooppaaa