fix(stepper): focus bugs #2358

mdt2 · 2023-12-11T16:13:23Z

⚠️ Rebased on #2288

Description

This PR fixes two focus bugs in the Stepper component:

Active state for the Infield Buttons sticks around after the mouse click is released
Invalid variant keyboard focus outline doesn't fit the component
Infield Button isn't focusable within the Stepper component (aligning with implementations from SWC and React teams per Patrick's guidance)
Invalid hover state border colors aren't mismatched (infield buttons were getting a different color than the text field)

And also adds kitchen sink style Chromatic coverage for all the variants of Stepper.

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

In the docs site (check Spectrum and Express theme):

Infield buttons don't show their focus state when Stepper has focus/keyboard focus
Infield buttons show their focus state when they get focus/keyboard focus state within Stepper
Invalid Stepper keyboard focus outline looks the same as the keyboard focus outline on valid Stepper
Invalid Stepper hover border is just one color (be extra sure to check Express on this one too)
Infield Buttons aren't keyboard accessible in the Stepper (aligning with implementations from SWC and React teams per Patrick's guidance)

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

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. ✨

github-actions · 2023-12-11T16:25:44Z

File metrics

Summary

Total size: 3.97 MB^*
Total change (Δ): ⬇ 0.27 KB (0.01%)

Package	Size	Δ
infieldbutton	18.37 KB	🚨 deleted, moved, or renamed
stepper	20.62 KB	🚨 deleted, moved, or renamed

Details

infieldbutton

File	Size	Base	Δ
Total	18.72 KB	18.37 KB	⬇ 0.36 KB (1.86%)
index-base.css	16.57 KB	16.22 KB	⬇ 0.36 KB (2.10%)
index-theme.css	2.72 KB	2.72 KB	-
index-vars.css	18.72 KB	18.37 KB	⬇ 0.36 KB (1.86%)
index.css	18.72 KB	18.37 KB	⬇ 0.36 KB (1.86%)
mods.json	2.16 KB	2.16 KB	-
themes/express.css	1.64 KB	1.64 KB	-
themes/spectrum.css	1.08 KB	1.08 KB	-

stepper

File	Size	Base	Δ
Total	20.39 KB	20.62 KB	⬆ 0.23 KB (1.11%)
index-base.css	17.00 KB	17.22 KB	⬆ 0.23 KB (1.33%)
index-theme.css	3.98 KB	3.98 KB	-
index-vars.css	20.39 KB	20.62 KB	⬆ 0.23 KB (1.11%)
index.css	20.39 KB	20.62 KB	⬆ 0.23 KB (1.11%)
mods.json	1.27 KB	1.38 KB	⬆ 0.11 KB (8.51%)
themes/express.css	2.30 KB	2.30 KB	-
themes/spectrum.css	1.67 KB	1.67 KB	-

* 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.

github-actions · 2023-12-11T16:45:13Z

🚀 Deployed on https://pr-2358--spectrum-css.netlify.app

pfulton

Thanks for taking care of this one!

It looks like this is slightly different in Firefox vs. Chrome, which is maybe ok. I'm a bit more concerned about the missing (overlapping?) focus indicator border when the stepper buttons are focused.

Firefox:

Chrome:

Also, and I think we could do this as a separate/follow-up ticket: can we get representation in Chromatic for the focus states?

pfulton

Looking much better! Thank you.

Just one question regarding Storybook: if I flip the focused control to true, it doesn't appear to affect the component in any visual way. Should it be turning off and on the focus indicator?

pfulton · 2023-12-14T15:23:30Z

components/stepper/stories/stepper.stories.js

+				...args
+			})}
+
+			${isChromatic() ? chromaticKitchenSink(args) : null}


mdt2 · 2023-12-14T15:57:48Z

Looking much better! Thank you.

Just one question regarding Storybook: if I flip the focused control to true, it doesn't appear to affect the component in any visual way. Should it be turning off and on the focus indicator?

Good catch! I broke it in the Chromatic Coverage commit by passing args manually when I didn't need to. Should be fixed now.

mdt2 changed the title ~~Mdt2/css 648 stepper focus bugs~~ fix(stepper): focus bugs Dec 11, 2023

mdt2 requested review from pfulton, jawinn and jenndiaz December 11, 2023 16:52

pfulton reviewed Dec 13, 2023

View reviewed changes

mdt2 force-pushed the mdt2/css-648-stepper-focus-bugs branch from e99062d to 9b7665e Compare December 13, 2023 20:53

pfulton reviewed Dec 14, 2023

View reviewed changes

components/stepper/stories/stepper.stories.js

...args

})}

${isChromatic() ? chromaticKitchenSink(args) : null}

Copy link

Collaborator

pfulton Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

mdt2 force-pushed the mdt2/css-648-stepper-focus-bugs branch from 2a5e198 to 28804db Compare December 14, 2023 15:57

mdt2 force-pushed the mdt2/css-648-stepper-focus-bugs branch from 28804db to e31e7b0 Compare December 14, 2023 16:23

pfulton approved these changes Dec 15, 2023

View reviewed changes

Melissa Thompson added 9 commits December 15, 2023 11:34

fix(infieldbutton): no outline on focus-visible

a6ed495

fix(infieldbutton): high contrast mode has a focus outline

d3a131a

fix(stepper): no longer touches InfieldButton classes

e408272

fix(stepper): button size + space match infieldbutton spec

6ab77f8

fix(stepper): infieldbutton background unchanged on stepper focus

c01d2b6

fix(stepper): invalid keyboard focus outline alignment

5b74dfe

fix(stepper): infieldbutton isnt focusable in stepper

4529874

fix(stepper): invalid hover state border color

4ea4e6c

docs(stepper): kitchen sink storybook examples for Chromatic

8172ec9

mdt2 force-pushed the mdt2/css-648-stepper-focus-bugs branch from e31e7b0 to 8172ec9 Compare December 15, 2023 16:34

pfulton added the run_vrt For use on PRs looking to kick off VRT label Dec 15, 2023

pfulton merged commit 8172ec9 into main Dec 15, 2023

pfulton deleted the mdt2/css-648-stepper-focus-bugs branch December 15, 2023 21:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(stepper): focus bugs #2358

fix(stepper): focus bugs #2358

Uh oh!

mdt2 commented Dec 11, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Dec 11, 2023 •

edited

Loading

infieldbutton

stepper

Uh oh!

github-actions bot commented Dec 11, 2023 •

edited

Loading

Uh oh!

pfulton left a comment

Uh oh!

pfulton left a comment

Uh oh!

pfulton Dec 14, 2023

Uh oh!

mdt2 commented Dec 14, 2023

Uh oh!

Uh oh!

fix(stepper): focus bugs #2358

fix(stepper): focus bugs #2358

Uh oh!

Conversation

mdt2 commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How and where has this been tested?

Validation steps

Regression testing

To-do list

Uh oh!

github-actions bot commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

infieldbutton

stepper

Uh oh!

github-actions bot commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfulton left a comment

Choose a reason for hiding this comment

Uh oh!

pfulton left a comment

Choose a reason for hiding this comment

Uh oh!

pfulton Dec 14, 2023

Choose a reason for hiding this comment

Uh oh!

mdt2 commented Dec 14, 2023

Uh oh!

Uh oh!

mdt2 commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023 •

edited

Loading