-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat(storybook): storybook 7 #2533
Conversation
Co-authored-by: John Gedeon <john@gedeons.com>
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.
looks good from a code perspective, I'll hold off approval until one of our designers can do a once over.
@@ -46,7 +46,7 @@ export const useModal = (isInitiallyOpen?: boolean): ModalHook => { | |||
|
|||
export const getScrollbarWidth = (): string => { | |||
// Only run in browser | |||
if (typeof document !== undefined) { | |||
if (typeof document !== 'undefined') { |
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.
praise: nice catch
Appears to have been an issue with defaultValues passed to the story (I think sb 7 deprecated defaultValue in favor of the args property). I made an update that should have the custom control SiteAlert story appear correctly. |
@jpandersen87 do you mind resolving the conflicts on this PR? That should let CI run so we can move forward with these changes. Thanks for the contributions! If you'd like to be added to all-contributors, we can also do that by pinging the all-contributors bot in a comment 😄 |
@jpandersen87 thanks for the updates! Could you take a look at these new package conflicts please? 👇 |
Done. Also fixed the babel error that came up after initial merge (updated package.json versions to prevent transitive dependency mangling, always a fun issue XD). |
- fix eslint config for webpack resolve to prevent module import eslint errors - replace deprecated ComponentStory with StoryFn
@jpandersen87 Looks like all that's still failing is Prettier. Can you run |
Fixed. Apologies for the minor issue churn. |
Also to note: This will get the project to SB major version 7 but will still be out of date. Due to the delicate nature of transient dependency versions (especially since react-uswds is still using React 17) I'm sticking with the version in this branch as a stable point that can be followed by anyone with a PR to catch SB up to latest 7.X.X. |
@all-contributors please add @jpandersen87 for code, maintenance |
I've put up a pull request to add @jpandersen87! 🎉 |
I appreciate the patience with my churn as I ensured my contributions follow react-uswds norms! |
Summary
Upgrades project's Storybook to version 7.1. Storybook's auto-migrations were ran which affected configuration and stories (function to object-based stories).
In addition:
.storybook/public
folder in order to reference locally instead (in the documentation story, and as the branding logo inUSWDS.js
)Related Issues or PRs
Closes #2266
How To Test
Run storybook locally via
yarn run storybook