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

[RFC] Improve Variability of Storybook Snapshots with Modes #12656

Closed
TylerAPfledderer opened this issue Apr 3, 2024 · 7 comments · Fixed by #12668
Closed

[RFC] Improve Variability of Storybook Snapshots with Modes #12656

TylerAPfledderer opened this issue Apr 3, 2024 · 7 comments · Fixed by #12668
Assignees
Labels
request for comments 🗣️ A request for comments has been made; discussion and input is encouraged

Comments

@TylerAPfledderer
Copy link
Contributor

TylerAPfledderer commented Apr 3, 2024

As part of an ongoing effort to improve the usage of Storybook with the new DS, apply the feature of Story Modes to Chromatic snapshots.

In short, with allow the generation of various snapshots to each story to show components in various viewports and color modes, among other possible combinations.

This can help cut down review time significantly by not forcing a reviewer to go into the Storybook canvas to check color mode and viewports widths for any design regressions.

An example of it's usefulness would for snapshots of the hero components, as the design for these components have defined alterations in the DS between various viewport widths.

@github-actions github-actions bot added the needs triage 📥 This issue needs triaged before being worked on label Apr 3, 2024
@pettinarip
Copy link
Member

Definitely. That will be very useful. At least the mobile visual test is critical, I would say.

Another critical one, which may not be related to these modes, is i18n testing. It would be nice to have visual tests with at least 1 locale (ru or uk, which often have long words) different from the default.

@TylerAPfledderer
Copy link
Contributor Author

@pettinarip ah yes! I'll need to dig, but I believe you can do modes with i18n.

@wackerow
Copy link
Member

wackerow commented Apr 8, 2024

Hey @TylerAPfledderer, this sounds great... how did you imagine this being implemented? Should we make a list of the existing stories and check them off with individual PRs that update them?

@wackerow wackerow added the question ❓ Further information is requested label Apr 8, 2024
@wackerow
Copy link
Member

wackerow commented Apr 8, 2024

Personally think I'd like to see:

  • Color modes
    • Light
    • Dark
  • i18n
    • en as default
    • zh as an example of shorter text (or comparable)
    • ru as an example of longer text (or comparable)
    • fa as an example of RTL text
  • Viewport
    • Desktop
    • Mobile
    • maybe tablet? This could be an opportunity to improve UX for this screen size

@wackerow wackerow added request for comments 🗣️ A request for comments has been made; discussion and input is encouraged and removed question ❓ Further information is requested needs triage 📥 This issue needs triaged before being worked on labels Apr 8, 2024
@TylerAPfledderer
Copy link
Contributor Author

TylerAPfledderer commented Apr 8, 2024

@wackerow I do have a draft PR up and didn't tie it to this issue yet. See #12668.

Yes, there are a bunch of combinations to implement. It definitely should be at the story level in terms of what to implement, but can look at exporting an object from .storybook to use for reusability.

I'm running into some trouble getting some options to generate properly. Waiting on #12674 so I can see issues through the PR checks.

I don't think small components like buttons or links warrant different Viewport snapshots like the hero components need. This is also something where we should have only enough snapshots to replicate the various forms shown in the DS figma file.

@pettinarip
Copy link
Member

I like @wackerow list but I'd start with less combinations just to not hit the snapshot limits
image

For example, I'd consider more important to test the mobile resolutions than the dark mode or tablet res. We could add those later if we have more room.

Agree also on that not all the components need to be tested with all the combinations.

Copy link
Contributor

This issue is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label May 10, 2024
@wackerow wackerow removed the Status: Stale This issue is stale because it has been open 30 days with no activity. label May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request for comments 🗣️ A request for comments has been made; discussion and input is encouraged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants