Skip to content

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

Merged
dancormier merged 3 commits intodevelopfrom
STACKS-549/focus-styles-topbar
Feb 2, 2024
Merged

fix(topbar): improve focus style to meet WCAG 2.2 AA#1628
dancormier merged 3 commits intodevelopfrom
STACKS-549/focus-styles-topbar

Conversation

@dancormier
Copy link
Copy Markdown
Contributor

@dancormier dancormier commented Feb 1, 2024

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

@dancormier dancormier marked this pull request as ready for review February 1, 2024 21:44
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 1, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 5a00cf2
🔍 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
Copy link
Copy Markdown
Collaborator

CGuindon commented Feb 1, 2024

@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
Copy link
Copy Markdown

netlify Bot commented Feb 1, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 2606cbb
🔍 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.

Copy link
Copy Markdown
Collaborator

@CGuindon CGuindon left a comment

Choose a reason for hiding this comment

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

Beauty!

Copy link
Copy Markdown
Contributor

@giamir giamir left a comment

Choose a reason for hiding this comment

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

Looks good to me. If we haven't created a ticket yet to migrate this component to PPCP we should. Apart from that I could not find anything out of the ordinary.

One question related to the search input. Shouldn't that match the focus behavior we will eventually add to all input components?
What I am asking is if the picture below represents how we will end up styling inputs going forward?
Screenshot 2024-02-02 at 14 53 32

I don't see why the topbar input should have a special style 🙂

@dancormier dancormier merged commit 94d1e8b into develop Feb 2, 2024
@dancormier dancormier deleted the STACKS-549/focus-styles-topbar branch February 2, 2024 16:04
@dancormier
Copy link
Copy Markdown
Contributor Author

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants