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

Storybook: Update to latest 5.3 #19599

Merged
merged 6 commits into from
Jan 16, 2020
Merged

Storybook: Update to latest 5.3 #19599

merged 6 commits into from
Jan 16, 2020

Conversation

ItsJonQ
Copy link

@ItsJonQ ItsJonQ commented Jan 13, 2020

Screen Shot 2020-01-13 at 11 09 22 AM

This update bumps Storybook to the latest 5.3.2 release. Some of the core .js
files have been renamed and refactored according to their 5.2.x -> 5.3
migration guides
.

This update also start some Storybook theming, adding the WordPress branding
logo on the top left.

#branding

The WP logo is being served from a URL. Originally, I tried to serve the image locally. However, I don't think it's working correctly with Storybook.

storybookjs/storybook#5878
storybookjs/storybook#6521

Maybe able to resolve this with Webpack work-arounds?

How has this been tested?

Run storybook locally via npm run storybook:dev. Also did a local build via npm run storybook:build. I tested the local dist build of storybook separately to ensure that it will work when built/deployed.

Types of changes

  • Bump Storybook dependencies from 5.2.4 -> 5.3.2
  • Adjusted main Storybook files according to their migration guides
  • Added WP logo 🙌

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

@ItsJonQ ItsJonQ added the Storybook Storybook and its stories for components label Jan 13, 2020
@ItsJonQ ItsJonQ self-assigned this Jan 13, 2020
Copy link
Contributor

@epiqueras epiqueras left a comment

Choose a reason for hiding this comment

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

We still need to deal with:

  • We've deprecated the ability to specify the hierarchy separators (how you control the grouping of story kinds in the sidebar). From Storybook 6.0 we will have a single separator /, which cannot be configured.
  • In 5.3, the CSF loader decouples the story's name/id, which means that displayName is no longer necessary. Unfortunately, this is a breaking change for any code that uses the story name field. Storyshots relies on story name, and the appropriate migration is to simply update your snapshots. Apologies for the inconvenience!

storybook/main.js Outdated Show resolved Hide resolved
storybook/main.js Show resolved Hide resolved
storybook/preview.js Outdated Show resolved Hide resolved
@ItsJonQ
Copy link
Author

ItsJonQ commented Jan 13, 2020

@epiqueras I just pushed the updates to the main.js config. I've also updated the separators (previously |) to the new Storybook standardized /.

@epiqueras
Copy link
Contributor

Thanks, we still need to update the snapshots.

@ItsJonQ
Copy link
Author

ItsJonQ commented Jan 13, 2020

@epiqueras Oh! Thanks for the reminder <3
I'm working on that now. Jest is throwing some DOM related errors. Not sure what's causing that.

Will keep investigating

Update: Fixed. I think it's the | -> /

@epiqueras
Copy link
Contributor

You need to mock the refs.

Jon Q added 4 commits January 13, 2020 14:51
This update bumps Storybook to the latest 5.3.2 release. Some of the core `.js`
files have been renamed and refactored according to their 5.2.x -> 5.3
migration guides.

This update also start some Storybook theming, adding the WordPress branding
logo on the top left.
@ItsJonQ
Copy link
Author

ItsJonQ commented Jan 13, 2020

Rebasing with latest master

@gziolo
Copy link
Member

gziolo commented Jan 14, 2020

https://travis-ci.com/WordPress/gutenberg/jobs/275074009#L398

It looks like types checking errors for some unknown reasons.

@ItsJonQ
Copy link
Author

ItsJonQ commented Jan 14, 2020

@gziolo Interesting 🤔. I'm not sure what's up with that. I'll poke at it to see if I can debug.

Thank you!!

This update fixes the Jest type checking issue by installing the latest
@types/jest definitions as a devDependency.
@ItsJonQ
Copy link
Author

ItsJonQ commented Jan 14, 2020

@gziolo Halloo!! I think I fixed it 🙏 . I installed the latest @types/jest as a devDependency. It looks like the project didn't have it. I think the type definitions were failing because an older version of @types/jest may have been a dependency of another dependency.

I based this fix off of this GH issue:
testing-library/jest-dom#152

@ItsJonQ
Copy link
Author

ItsJonQ commented Jan 16, 2020

Thanks for the review @epiqueras !! Will merge 💪

@ItsJonQ ItsJonQ merged commit 3bb8837 into master Jan 16, 2020
@ItsJonQ ItsJonQ deleted the update/storybook-5-3-x branch January 16, 2020 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Storybook Storybook and its stories for components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants