-
Notifications
You must be signed in to change notification settings - Fork 30
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
fix(MenuItem_Anchor_tooltipTarget): Focus state fixes #2788
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrood
changed the title
Use solid rather than auto for focus outline
fix(outline): Use solid rather than auto for focus outline
Sep 28, 2023
jrood
force-pushed
the
jrood.REACH-3073.solid-focus-outline
branch
from
September 28, 2023 20:00
8b4aaa5
to
e2148d0
Compare
jrood
changed the title
fix(outline): Use solid rather than auto for focus outline
fix(outline): Use pseudo element pattern for focus borders
Sep 28, 2023
jrood
changed the title
fix(outline): Use pseudo element pattern for focus borders
fix(outline): Focus states for anchor, menu-item, and tooltip target
Oct 3, 2023
jrood
changed the title
fix(outline): Focus states for anchor, menu-item, and tooltip target
fix(Anchor, MenuItem, Tooltip target): Update focus states for Anchor, MenuItem, and focusable Tooltip target
Oct 3, 2023
jrood
changed the title
fix(Anchor, MenuItem, Tooltip target): Update focus states for Anchor, MenuItem, and focusable Tooltip target
fix(Anchor, MenuItem, focusable Tooltip target): focus states
Oct 3, 2023
jrood
changed the title
fix(Anchor, MenuItem, focusable Tooltip target): focus states
fix(focusStates): Follow pseudo-element convention
Oct 3, 2023
jrood
changed the title
fix(focusStates): Follow pseudo-element convention
fix(MenuItem,Anchor): Focus state fixes
Oct 5, 2023
jrood
changed the title
fix(MenuItem,Anchor): Focus state fixes
fix(MenuItem_Anchor_tooltipTarget): Focus state fixes
Oct 5, 2023
jrood
force-pushed
the
jrood.REACH-3073.solid-focus-outline
branch
from
October 6, 2023 18:14
0212432
to
6f6aa45
Compare
📬Published Alpha Packages:@codecademy/[email protected] |
🚀 Styleguide deploy preview ready! |
dreamwasp
approved these changes
Oct 10, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested in the PRs envs and locally + all lgtm, thanks for taking this on!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
( This PR has some overlap with #2791 (review) )
Update implementation and color for focus states of Anchor, MenuItem, and focusable Tooltip target.
The initial desire was to fix an a11y issue found via TestIO where border color was incorrect for MenuItem focus states in Safari. In the process, we found that Safari doesn't honor
outline
styles usingauto
. The initial solution was to switchauto
tosolid
for MenuItem focus boarders and other elements with the same issue. However, we then found thatsolid
introduces new issues where native outlines don't handle border-radius as expected.For buttons and some Anchor variants, we already have a convention for using a
:before
pseudo-element rather thanoutline
for focus state borders. This approach doesn't suffer from the same color, border-radius, and browser-inconsistency issues. This PR fixes the a11y issue by updating MenuItems to use this same pseudo-element pattern for focus borders. The same change is made for the Anchor variants that did not yet use this approach and for focusable Tooltip targets. A color fix for focused Anchor text color is also included.PR Checklist
Related to designs:This PR includes unit tests for the code changeTesting Instructions
Setup
CMD + ,
-> AdvancedMenuItem
/?path=/docs/molecules-menu--menu
Anchor
/?path=/docs/typography-anchor--anchor
Focusable target for Tooltip
/?path=/docs/atoms-tooltip--tool-tip
PR Links and Envs
LE Preview