StackExchange / Stacks

Stack Overflow’s Design System
https://stackoverflow.design
MIT License
610 stars 90 forks source link

fix(topbar): improve focus style to meet WCAG 2.2 AA #1628

Closed dancormier closed 8 months ago

dancormier commented 8 months ago

Addresses https://stackoverflow.atlassian.net/browse/STACKS-549

This PR updates focus styles for all elements generally included within the topbar component.

Deploy preview

Notes on visual changes

.s-topbar--item, .s-topbar--logo

From STACKS-549:

Rounded corners will match the original non-focused component.

To match the mockup, I added a border radius of var(--br-sm) of items (and of the logo for consistency). @CGuindon let me know if this is ok or if it needs modification.

focused hovered focused + hovered
item image image image
logo image image image

Search

The old focus styles looked out of place and it was easy enough to add a matching focus ring to the search input and select element. @CGuindon is this alright or should I revert/modify?

focused
select image
input image

Ring placement

For the search input and select elements, I opted to render the focus ring outside the element. This stands in contrast with other topbar elements, which all have the focus ring rendered inside the element. I did this because these elements rely on the border being visible to read as an input/select. When I tried adding the ring inside the element, the focus state was kinda tough to distinguish from the unfocused state, especially in high contrast mode.

light mode light HC mode
select image image
input image image

Notes on technical changes

Code structure

Since this component doesn't yet adhere to our PPCP structure, I decided it was best to keep all of the focus code grouped for easy parsing (and easy porting when we make the switch to PPCP).

netlify[bot] commented 8 months ago

Deploy Preview for stacks ready!

Name Link
Latest commit 5a00cf223e761573ab556d2fb8f226616400989b
Latest deploy log https://app.netlify.com/sites/stacks/deploys/65bc108b6ea5ef0008f5c684
Deploy Preview https://deploy-preview-1628--stacks.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

CGuindon commented 8 months ago

@dancormier

For the search input and select elements, I opted to render the focus ring outside the element.

Yes, I agree. I was assuming we'd have to go outside for all input fields so in this case the search bar matches that logic and even though it's a button beside the input, it probably looks weirder to have it go inside then outside with side-by-side elements so I think your approach makes the most sense. Outside for both search elements.

netlify[bot] commented 8 months ago

Deploy Preview for stacks ready!

Name Link
Latest commit 2606cbb34c7652645ff4b09e35729949df018868
Latest deploy log https://app.netlify.com/sites/stacks/deploys/65bc11b672776d0008ac8f1e
Deploy Preview https://deploy-preview-1628--stacks.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dancormier commented 8 months ago

@giamir I've created a ticket for the PPCP migration and will follow through on applying the input focus styles in an upcoming PR (STACKS-553).