Skip to content

fix(block-link): improve focus style to meet WCAG 2.2 AA#1621

Merged
dancormier merged 3 commits intodevelopfrom
STACKS-545/focus-styles-block-link
Jan 31, 2024
Merged

fix(block-link): improve focus style to meet WCAG 2.2 AA#1621
dancormier merged 3 commits intodevelopfrom
STACKS-545/focus-styles-block-link

Conversation

@dancormier
Copy link
Copy Markdown
Contributor

Resolves STACKS-545

Note
The above linked ticket shows updates as they relate to the s-menu component. The focusable elements shown within the s-menu component are s-block-links, so this PR updates the s-block-link focus style.

@netlify
Copy link
Copy Markdown

netlify Bot commented Jan 29, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit d505762
🔍 Latest deploy log https://app.netlify.com/sites/stacks/deploys/65b96b150dbf7f000899001d
😎 Deploy Preview https://deploy-preview-1621--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.

Screenshot 2024-01-30 at 7 40 31 AM

I'm assuming adding padding here won't do anything overall — devs would need to make sure the element has a container somewhere in the core code?

Is there a better way to show an example of 'no container' that would show at least 2px padding from the text?

@dancormier
Copy link
Copy Markdown
Contributor Author

@CGuindon That example is kinda misleading. Each link has markup like this:

<a href="" class="s-block-link px0"></a>

Note the px0. That class overrides any left or right padding to be 0px, so a dev would have to specifically apply an atomic class to override the padding in this case. If we ever take a good look at our docs pages, I think this example would be a good one to get rid of.

I found instances of this in two files (1, 2), so I don't think it's too big of a deal and it's probably something that can be resolved pretty easily in Core by finessing the padding a little.

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.

Code changes look good to me. Thanks @dancormier.

I see that there is a small divergence on the focus ring radius between the mockup

Screenshot 2024-01-30 at 17 30 35

and the final result

Screenshot 2024-01-30 at 17 30 28

@dancormier dancormier requested a review from CGuindon January 30, 2024 23:26
@dancormier dancormier merged commit 077b897 into develop Jan 31, 2024
@dancormier dancormier deleted the STACKS-545/focus-styles-block-link branch January 31, 2024 20:59
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