Chromatic testing of hover/focus states #495
Comments
m-akinc
added
tech debt
triage
|
Wonder if we can use play functions in storybook to trigger these states: https://storybook.js.org/docs/react/api/csf#play-function |
|
Looked into this some during a previous Innovation Days and ran into problems.
|
|
I tried looking into this recently (for Innovation Days), but still ran into problems. I tried the Pseudo States addon, which kind-of worked, but not the way it is supposed to. Whether I enabled a pseudo-state from code (through configuration of the Storybook |
|
After a couple rounds of investigation it looks like the Chromatic approach isn't going anywhere soon. Closing this in favor of #705; we can always revive it if needed. |
|
@jattasNI @m-akinc based on discussion in #1155 (comment) I took another quick look at this issue. It does look like the pseduo states plugin intends to support adoptedStylesheets as leveraged by fast: https://github.com/chromaui/storybook-addon-pseudo-states/blob/e1deec870979e16259466bfe73e93a70ed662f12/src/withPseudoState.js#L96 If we were running into issues maybe we should root cause it and create an issue and / or submit a patch? |
|
Chromatic announced a beta for a feature which allows you to take snapshots from within a Playwright test and have Chromatic manage them and track diffs. I think we could use this to write a test that hovers and focuses each of our input components in succession and takes snapshots to ensure that the hover and focus visuals are consistent. We'd have to figure out how this relates to our matrix tests. In an ideal world, we'd test the hover and focus states combined with all the other state combinations. However, that'd be a pretty big explosion in the number of snapshots we need to take, since you can only hover or focus one thing at a time. Perhaps as a stopgap we'd just have tests for each input component in its default configuration in the 3 themes x the 3 states (hover, focus, hover+focus). We could expand that for specific components that are known to look different in other states, like button appearances. But omit testing for uninteresting states that look identical to each other. A possible next step might be to prototype this on an innovation day. |
|
I need the addon to accept a bunch of fixes, then we can update to that version and write our tests. |
|
@fredvisser think we can leverage our chromatic support to get them to look at the PRs mert put up? https://github.com/chromaui/storybook-addon-pseudo-states/pulls/m-akinc |
## 🤨 Rationale Part of #495 ## 👩💻 Implementation I had to make some changes in `storybook-addon-pseudo-states` to support our styling, but now we can pull in that fixed release and add some initial visual tests. The way the addon typically works is that you set story parameters to select which component(s) should have the pseudo-state styling, e.g. ```js ButtonStory.parameters = { pseudo: { hover: ["#one", "#two", "#three"], focus: true, // means apply to all components active: ".some-class", }, } ``` However, this does work well with our matrix story approach. The effect of these parameters is just to set certain classes on the indicated elements. So, instead, we can bypass the story parameters and directly apply the classes to our elements. For example, any element with the class `hover` or `pseudo-hover-all` will be styled by the addon as if hovered. NOTE: Because we have to put the classes directly on the elements in our Story html, we can't put them on any shadow DOM elements, e.g. the inc/dec buttons of the number field. However, puting `pseudo-<state>-all` on a host element will cause all elements in its shadow DOM to be styled as if they have that state. This can create some visuals that are practically impossible (e.g. a number field and both of its inc/dec buttons all having keyboard focus at the same time), but they are still useful from a visual testing standpoint. The pseudo-states supported by the addon are: - `:hover` - `:active` - `:focus-visible` - `:focus-within` - `:focus` - `:visited` - `:link` - `:target` It is only interesting to test states that we specifically style for, so generally, that is hover, active, and some form of focused. Because our components use both `:focus-visible` (`focusVisible` from FAST) and `:focus-within`, I'm not distinguishing the two, and I'm setting classes for both whenever we want to see "focused" styling. We could potentially test all combinations of these pseudo-states, but that seems unnecessary, especially considering that we want to test these states in _conjunction_ with other appearance/config variations. So I'm limiting combination testing to just hovered+active, since that is a common use case for mouse-based interaction. To summarize, that means I'm proposing we test: - hover (only) - active (only) - focused (only) - hovered and active Initially, I'm adding new test matrix stories for: - anchor - anchor-button - button - menu-button - number-field - toggle-button These seemed to be some of the more interesting components in terms of having distinct keyboard-focused and/or active states, (in addition to hover, which applies to most/all? components). Some combinations of interaction states with control attribute values aren't interesting, e.g. a disabled control with focus or active interaction state. In order to exclude certain combinations, we had to refactor the `createMatrix` utility function and introduce a new `createMatrixInteractionsFromStates` function. ## 🧪 Testing Storybook visual tests ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. --------- Co-authored-by: rajsite <rajsite@users.noreply.github.com>
🧹 Tech Debt
We would like to capture regressions in the visual styling of interactive states, like hover or keyboard focus.
We should update contributing docs to describe our expectations for interactive testing (cover hover/focus/active).
We need to add tests for these components. Devs a pick up one or more at a time; be sure to indicate that you're working on them.
Not needed for:
The text was updated successfully, but these errors were encountered: