feat(storybook): custom decorators for chromatic coverage
Description
Overall, this work will increase our Chromatic coverage without increasing our number of snapshots by showing LTR and RTL, all three background colors, both platform sizes, and every available size of component to Chromatic by default.
- Implement custom decorators for background color, platform scale, and text direction
- Turn off controls when custom decorators are in use in the toolbar (because the controls don't work unless the global value in the toolbar is set to "default," so removing them to avoid confusion of which to use)
- Turn on size decorator so Chromatic tests components at each size in one snapshot
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps
Text direction @jenndiaz
- [x] When
ltris active (the default), the component has the ltr direction - [x] When
rtlis active, the component have the rtl direction - [x] When
ltr + rtlis active, there are two components showing, one is ltr and the other is rtl
Background color @jenndiaz
- [x] When
default bg coloris active, the background is Light - [x] When
default bg coloris active, the background color can be changed in the control panel - [x] When
stacked bg colorsis active, the background color cannot be changed in the control panel (it shouldn't even show as a control) - [x] When
stacked bg colorsis active, the component shows three times with our three different background colors, stacked one on top of the other - [x] When
side-by-side bg colorsis active, the background color cannot be changed in the control panel (it shouldn't even show as a control) - [x] When
side-by-side bg colorsis active, the component shows three times with our three different background colors, side by side - [x] If you go back to the default
default bg color, the control should show back up in the control panel. If you adjust it, the component adjusts accordingly.
Platform sizes @jenndiaz
- [x] When
medium - desktopis active, the component shows at the medium platform size - [x] When
medium - desktopis active, the platform size can be changed in the control panel - [x] When
large - mobileis active, the component shows at the large platform size (usually a bit bigger visually) - [x] When
large - mobileis active, the platform size cannot be changed in the control panel (it shouldn't even show as a control) - [x] When
medium + largeis active, two components show, with a heading of "Platform Size: Size", and one is medium and the other is large (in alignment with their heading) - [x] When
medium + largeis active, the platform size cannot be changed in the control panel (it shouldn't even show as a control) - [x] If you go back to the default
medium - desktop, the control should show back up in the control panel. If you adjust it, the component adjusts accordingly.
Testing together! Try some combinations of the controls:
- [ ] Set
ltr + rtl,side-by-side bg colors, andmedium + largeto see what Chromatic will see - [ ] Try all sorts of combinations. Try to break it. Adjust control panel controls and then their toolbar counterparts, etc.
Regression testing
Validate:
- The documentation pages for at least two other components are still loading, including:
- [ ] The pages render correctly, are accessible, and are responsive.
- If components have been modified, VRTs have been run on this branch:
- [ ] VRTs have been run and looked at.
- [ ] Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.
To-do list
- [x] I have read the contribution guidelines.
- [ ] I have updated relevant storybook stories and templates.
- [ ] I have tested these changes in Windows High Contrast mode.
- [ ] If my change impacts other components, I have tested to make sure they don't break.
- [ ] If my change impacts documentation, I have updated the documentation accordingly.
- [ ] ✨ This pull request is ready to merge. ✨
🚀 Deployed on https://pr-2379--spectrum-css.netlify.app
File metrics
Summary
Total size: 3.97 MB*
🎉 No changes detected in any packages
* Size determined by adding together the size of the main file for all packages in the library.* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.
I am also seeing some issues with text colors not adjusting for color theme when displaying multiple background colors.
Well: if you toggle the individual background layers in controls the font adjusts correctly but if you display all 3 the font is displaying incorrectly on dark and darkest.
Same thing with:
- Radio read only variant
- Treeview with section, the section headers
With everything on, ltr + rtl, stacked, and medium + large, I was surprised how well everything displayed.
The popover/modal/is-open components consistently do not display well with any of the multiple or both. (Alert Dialog, Alert Banner, CoachMark, Color Loupe, ComboBox, Contextual help, Datepicker, Dialog, Menu: tray submenu, Modal, Picker, Popover, Search within, Tray, Underlay)
Good catch. I've been researching and trying different things to fix this. Here's what I'm finding:
- Unfortunately, toolbar items can't be added/removed based on the page yet (see also a duplicate, related issue). This would've been my ideal solution, to just turn off the multiple/both option for popover and modal components. Sad.
- We did find a plugin that could help with conditional toolbar items, but the last update was three years ago, and that's a bit too long for it to really be considered maintained.
- Then I thought, what if we don't use the toolbar for this and instead we can have dynamic argTypes in the control panel. But I found out that argTypes are immutable, so that won't work unless we were to set the control on every story 😬 An option I suppose, but kinda icky.
So I'm about to push up a bandaid fix with logic that says if the component is a popover/fullscreen component, instead of rendering the component, we render an error that the feature is unavailable for that component. Not my favorite...
Jenn and I also chatted a bit about how if popovers were static, the issues around them in this context would be more manageable. But making them static means losing the example of interaction and animation that are CSS based. It's almost like we need an extra story for each popover instance that shows it statically to be able to show a bunch of them on a single page for Chromatic. Which makes me wonder more about a Chromatic-only Storybook instance like react-spectrum has.
Our hope, or at least my hope, with using custom decorators was that it wouldn't be much to manage moving forward compared to a separate Storybook instance for Chromatic, but the way things are heading I wonder if the management level might be more similar than initially expected.
This is some really fantastic work! I'm sorry I didn't realize it was going on. One thing I wanted to note is that in the rework of the build systems and architecture that I've been doing, they each include pulling out decorators into distinct files to make maintaining them easier (it also gives us a chance to publish them separately at some point for the greater Storybook community to leverage). See this PR for an example: https://github.com/adobe/spectrum-css/pull/2398
One thing I've done in that PR is tried to create a utility for styling the wrapper element to facilitate groupings for Chromatic - I did this by setting flex box layout on the storybook wrappers but allowing components to opt-out or alter it using a customStorybookStyles object in the args of the story. Let me know what you think of this approach. It might help us handle grouped layouts consistently and allow for dynamically setting the background colors (it also ensures all components contain padding for focus/hover state capturing). My big question mark here is if flexbox is too wonky a layout to use as a default (it can cause weird side effects on components that are designed for use in block for example that might not be super clear until you dig into the styles above the component).
Here's some more readable documentation on the wrapper styles approach I mentioned above: https://github.com/adobe/spectrum-css/pull/2398/files?short_path=de90dbe#diff-de90dbe4186cda8c8dfb22049ae90250d47d1d38c8b61f151d9787cf49186b6b