Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Checkbox has a visible ring on focus-visible #818

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

lyzadanger
Copy link
Contributor

There has been a little defect kicking around for...well, kind of forever. The Checkbox component lacked a visible focus ring when focused by keyboard navigation (focus-visible).

In laying out a potential new user-preferences UI, this accessibility shortcoming is feeling more glaring. The fix required me to do a little Tailwind-thinking but ended up being straightforward (i.e. can be done via CSS alone; I thought we might have to track/manage focus state for a while there...).

Testing

On this branch, navigating with the keyboard through/around checkboxes on the Checkboxes component page in the pattern library will cause focus rings to show up:

image

On main, Checkboxes do not have any focus rings.

Fixes #627

@lyzadanger lyzadanger requested a review from acelaya January 30, 2023 18:51
@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #818 (e8b18ca) into main (7a8eec0) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #818   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           52        52           
  Lines          634       633    -1     
  Branches       226       225    -1     
=========================================
- Hits           634       633    -1     
Impacted Files Coverage Δ
src/components/input/Checkbox.js 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lyzadanger lyzadanger merged commit 4862843 into main Jan 31, 2023
@lyzadanger lyzadanger deleted the fix-checkbox-ring branch January 31, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkbox component does not have a visible focus ring
2 participants