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

fix(MenuItem_Anchor_tooltipTarget): Focus state fixes #2788

Merged
merged 4 commits into from
Oct 10, 2023

Conversation

jrood
Copy link
Contributor

@jrood jrood commented Sep 28, 2023

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 using auto. The initial solution was to switch auto to solid for MenuItem focus boarders and other elements with the same issue. However, we then found that solid 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 than outline 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:
  • Related to JIRA ticket: REACH-3073
  • I have run this code to verify it works
  • This PR includes unit tests for the code change
  • This PR includes testing instructions tests for the code change
  • The alpha package of this PR is passing end-to-end tests in all relevant Codecademy repositories

Testing Instructions

Setup

  • Go to Mac System Settings -> Keyboard
  • Verify that the "Keyboard navigation" toggle is enabled
  • Open Safari, Safari Settings CMD + , -> Advanced
  • Verify that "Press Tab to highlight each item on a webpage" is checked
  • Do the three tests below using Safari:

MenuItem

  • Go to /?path=/docs/molecules-menu--menu
  • Tab through MenuItems on the Action and Navigation variants
  • Verify that a blue/hyper focus state is shown

Anchor

  • Go to /?path=/docs/typography-anchor--anchor
  • Tab through Anchor variants
  • Verify that the primary color is always for Anchor text while focused

Focusable target for Tooltip

  • Go to /?path=/docs/atoms-tooltip--tool-tip
  • Tab onto the target for the "Focusable" example
  • Verify that a blue/hyper focus state is shown

PR Links and Envs

Repository PR Link Preview Env
Monolith Monolith PR Monolith Preview
Monorepo Monorepo PR PortalApp Preview
LE Preview

@jrood 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 jrood force-pushed the jrood.REACH-3073.solid-focus-outline branch from 8b4aaa5 to e2148d0 Compare September 28, 2023 20:00
@jrood 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 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 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 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 jrood changed the title fix(Anchor, MenuItem, focusable Tooltip target): focus states fix(focusStates): Follow pseudo-element convention Oct 3, 2023
@jrood jrood changed the title fix(focusStates): Follow pseudo-element convention fix(MenuItem,Anchor): Focus state fixes Oct 5, 2023
@jrood jrood changed the title fix(MenuItem,Anchor): Focus state fixes fix(MenuItem_Anchor_tooltipTarget): Focus state fixes Oct 5, 2023
@jrood jrood marked this pull request as ready for review October 5, 2023 20:03
@jrood jrood requested a review from a team as a code owner October 5, 2023 20:03
@jrood jrood force-pushed the jrood.REACH-3073.solid-focus-outline branch from 0212432 to 6f6aa45 Compare October 6, 2023 18:14
@codecademydev
Copy link
Collaborator

📬Published Alpha Packages:

@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]

@codecademydev
Copy link
Collaborator

🚀 Styleguide deploy preview ready!

https://652050803634d8275304fb3a--gamut-preview.netlify.app

Deploy Logs

Copy link
Contributor

@dreamwasp dreamwasp left a 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!

@jrood jrood added the Ship It :shipit: Automerge this PR when possible label Oct 10, 2023
@codecademydev codecademydev merged commit 28ccb9d into main Oct 10, 2023
21 of 22 checks passed
@codecademydev codecademydev deleted the jrood.REACH-3073.solid-focus-outline branch October 10, 2023 16:52
@codecademydev codecademydev removed the Ship It :shipit: Automerge this PR when possible label Oct 10, 2023
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.

3 participants