Fix Storybook viewports #2879
Fix Storybook viewports #2879
Conversation
|
|
Size Change: 0 B Total Size: 109 kB ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-dkcifydeap.chromatic.com/ Chromatic results:
|
| }, | ||
| viewport: { | ||
| viewports: wbViewports, | ||
| options: wbViewports, |
There was a problem hiding this comment.
Updated to options based on the Storybook viewport docs: https://storybook.js.org/docs/essentials/viewport
| }, | ||
| }, | ||
| }, | ||
| globals: { |
There was a problem hiding this comment.
Updating how we configure the default viewport for a story using: https://storybook.js.org/docs/essentials/viewport#defining-the-viewport-for-a-story
| viewport: { | ||
| viewports: customViewports, | ||
| defaultViewport: "desktop", | ||
| }, |
There was a problem hiding this comment.
Removing the viewport configuration for stories that already have modes configured since both can't be used: https://www.chromatic.com/docs/modes/viewports/#can-i-use-viewports-and-modes-simultaneously
(Preferring the modes configuration since that also configures theming)
Applied this change to the other stories in the modal package.
Lmk if there are any concerns with this!
| small: allModes.small, | ||
| large: allModes.large, |
There was a problem hiding this comment.
Some of the modal stories didn't have modes set up, so I configured it to use modes too
There was a problem hiding this comment.
Do you know if this has some effect in the snapshots? or is it just for storybook rendering in the browser? mentioning this as the expectation is not to take snapshots for the stories in this file.
There was a problem hiding this comment.
Good question, this is for Chromatic snapshots! We were already using the viewport config to generate multiple snapshots for these stories, and I switched it over to use modes instead.
Before, it was generating a snapshot for each custom viewport (phone, tablet, and desktop), and now it's configured to generate a snapshot for just small and large screens (this is what the newer modal stories were doing so I did the same for this set of stories!)
Let me know if we'd rather keep the old viewport config for Chromatic snapshots! It's still supported if we don't use it with modes at the same time!
There was a problem hiding this comment.
ah got it! What I mean is that we are disabling snapshots for this file (see the line below), so can just probably get rid of both viewport and modes in this file.
If you look at the Chromatic build, there are no snapshots associated to ModalDialog.
There was a problem hiding this comment.
Ohhh yes good point! 😄 I removed setting modes for the files that disable snapshots!
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
… snapshots anyways
jandrade
left a comment
jandrade
left a comment
There was a problem hiding this comment.
Looks great! Thanks for cleaning up and get things in the right track 👏 
| defaultViewport: "desktop", | ||
| }, | ||
| chromatic: { | ||
| modes: { |
There was a problem hiding this comment.
suggestion: Could you please remove it here as well? (sorry I forgot to add a comment next to disableSnapshot).
There was a problem hiding this comment.
Yes good catch! Removed :)
…sabled for modal-launcher
beaesguerra
force-pushed
the
fix-sb-viewports
branch
from
4b45f80 to
10e2b1e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2879 +/- ##
============================
============================
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary:
While creating new stories that used Storybook viewports, I found that our existing configuration was no longer working. In this PR, we update how we configure viewports in Storybook and in specific stories
Issue: WB-XXXX
Test plan:
Verify Chromatic snapshots and the stories that were updated
For example,
?path=/story/packages-tabs-navigationtabs-testing-navigationtabs-snapshots--scenarios-small-screenhas a small screen viewport when the story is viewed