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

Upgrade to Storybook V7 #160

Merged
merged 7 commits into from
Jun 28, 2023
Merged

Upgrade to Storybook V7 #160

merged 7 commits into from
Jun 28, 2023

Conversation

sawyerh
Copy link
Contributor

@sawyerh sawyerh commented Jun 24, 2023

@sawyerh sawyerh marked this pull request as ready for review June 24, 2023 01:09
@sawyerh sawyerh requested a review from jcq June 24, 2023 01:10
@sawyerh sawyerh temporarily deployed to github-pages June 24, 2023 01:12 — with GitHub Actions Inactive
@sawyerh sawyerh temporarily deployed to github-pages June 24, 2023 01:16 — with GitHub Actions Inactive
@sawyerh sawyerh temporarily deployed to github-pages June 24, 2023 01:20 — with GitHub Actions Inactive
Copy link

@brandon-lent brandon-lent left a comment

Choose a reason for hiding this comment

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

I'm mostly driving by here, but this lgtm other than 1 comment. I tried finding a solution to the postcss issue for NJ also but couldn't figure it out. I'll defer to folks more involved in this project / you on if those icons that render strangely is blocking. Seems like we'll have to deal with it anyway until there is a fix.

Comment on lines -79 to -86
// Support deploying Storybook to a subdirectory (like GitHub Pages).
// This makes the Storybook JS bundles load correctly.
if (NEXT_PUBLIC_BASE_PATH) {
config.output = config.output ?? {};
config.output.publicPath = `${NEXT_PUBLIC_BASE_PATH}/`;
}

return config;

Choose a reason for hiding this comment

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

❓ Did you intend on removing this -- would this cause issues with deploying to github pages w/o it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it seems like the Next.js Storybook framework integration now does all of this for us. I tested deploying this branch to GitHub Pages and it works: https://navapbc.github.io/template-application-nextjs/


export const Home = Template.bind({});
export const Preview = {};

Choose a reason for hiding this comment

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

markup seems a lot cleaner with this new format!

@brandon-lent
Copy link

brandon-lent commented Jun 27, 2023

This may be helpful for future folks - but if you have many stories in your repo, storybook provides a codemod to automatically convert CSF2 -> CSF3

https://storybook.js.org/docs/react/migration-guide#csf2-to-csf3

You may have to update some of your stories manually, so keep that in mind.

@lorenyu
Copy link
Contributor

lorenyu commented Jun 28, 2023

@brandon-lent going forward don't be afraid to approve PRs too, you know more react than i do and are more hands on with it too. i don't need to be a bottleneck

@lorenyu
Copy link
Contributor

lorenyu commented Jun 28, 2023

oh nvm you did approve it lol. haha i misread the github notification sorry!

@brandon-lent
Copy link

brandon-lent commented Jun 28, 2023

Looks like you can install the plugin recommended from storybook to fix the icon issue, not sure if you wanted to experiment with that in this PR or a separate one

@sawyerh sawyerh merged commit b41f250 into main Jun 28, 2023
6 checks passed
@sawyerh sawyerh deleted the sawyerh/update-storybook branch June 28, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants