Skip to content

Storybook: Update to latest 5.3 - #19599

Merged
ItsJonQ merged 6 commits into
masterfrom
update/storybook-5-3-x
Jan 16, 2020
Merged

Storybook: Update to latest 5.3#19599
ItsJonQ merged 6 commits into
masterfrom
update/storybook-5-3-x

Conversation

@ItsJonQ

@ItsJonQ ItsJonQ commented Jan 13, 2020

Copy link
Copy Markdown

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 requested review from epiqueras, gziolo and mkaz January 13, 2020 16:24
@ItsJonQ ItsJonQ self-assigned this Jan 13, 2020

@epiqueras epiqueras left a comment

Copy link
Copy Markdown
Contributor

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!

Comment thread storybook/main.js Outdated
Comment thread storybook/main.js
Comment thread storybook/preview.js Outdated
@ItsJonQ

ItsJonQ commented Jan 13, 2020

Copy link
Copy Markdown
Author

@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
Copy Markdown
Contributor

Thanks, we still need to update the snapshots.

@ItsJonQ

ItsJonQ commented Jan 13, 2020

Copy link
Copy Markdown
Author

@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
Copy Markdown
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

ItsJonQ commented Jan 13, 2020

Copy link
Copy Markdown
Author

Rebasing with latest master

@ItsJonQ
ItsJonQ force-pushed the update/storybook-5-3-x branch from 0e48d50 to 5752fc8 Compare January 13, 2020 19:52
@gziolo

gziolo commented Jan 14, 2020

Copy link
Copy Markdown
Member

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

It looks like types checking errors for some unknown reasons.

@ItsJonQ

ItsJonQ commented Jan 14, 2020

Copy link
Copy Markdown
Author

@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

ItsJonQ commented Jan 14, 2020

Copy link
Copy Markdown
Author

@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

ItsJonQ commented Jan 16, 2020

Copy link
Copy Markdown
Author

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