-
Notifications
You must be signed in to change notification settings - Fork 0
🎨 Palette: Make WritingPad interactive elements accessible #347
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
Open
RileyTheRiler
wants to merge
3
commits into
main
Choose a base branch
from
palette-writingpad-a11y-3587656293733726396
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
15a0412
Make WritingPad interactive elements accessible
google-labs-jules[bot] fc94796
Fix CI failure and add accessibility to WritingPad
google-labs-jules[bot] 68df735
🎨 Palette: Make WritingPad interactive elements accessible and fix CI
google-labs-jules[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
Oops, something went wrong.
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.
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.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
Repository: RileyTheRiler/French-Game
Length of output: 182
🌐 Web query:
Tailwind CSS v4 focus-visible:outline-hidden utility outline-none only removes outline style documentation💡 Result:
In Tailwind CSS v4, the
outline-hiddenandoutline-noneutilities behave differently regarding accessibility, specifically in forced colors mode [1][2]. The primary difference is: -outline-hidden: This utility setsoutline-style: nonebut also preserves a visible outline when the browser's forced colors mode is active (e.g., Windows High Contrast Mode) [3][2]. It is designed as the recommended replacement for the legacy v3outline-noneutility to ensure focus visibility for accessibility [1][4]. -outline-none: In v4, this utility is a simple, literal implementation that setsoutline-style: noneand does not provide special handling for forced colors mode [1][4]. The v4 upgrade process encourages replacing all legacy instances ofoutline-nonewithoutline-hiddento maintain the accessibility behavior that v3 users were implicitly receiving [1][2]. If you truly need to remove the outline entirely in all circumstances (including forced colors mode), you would useoutline-none, though it is generally recommended to provide custom focus styling for accessibility when doing so [3].Citations:
outline-nonewithoutline-hidden, add newoutline-nonetailwindlabs/tailwindcss#14926🏁 Script executed:
Repository: RileyTheRiler/French-Game
Length of output: 182
🌐 Web query:
Tailwind CSS v4 focus-visible:outline-hidden utility outline-none only removes outline style documentation💡 Result:
In Tailwind CSS v4, the
outline-hiddenandoutline-noneutilities serve distinct purposes regarding accessibility and browser behavior [1][2]. Theoutline-hiddenutility is the recommended replacement for the behavior previously associated withoutline-nonein Tailwind CSS v3 [2]. It hides the browser default outline during normal interaction but preserves the focus ring in "forced colors" mode (such as Windows High Contrast Mode) [1][3]. This is done to ensure the element remains accessible to users who rely on high-contrast assistive technologies [4][3]. Conversely, theoutline-noneutility in v4 is a simple CSS utility that setsoutline-style: none[4][2]. It does not provide the same accessibility safety net asoutline-hiddenfor forced-colors mode [4][3]. When usingfocus-visible:outline-hidden, you are applying this accessibility-aware behavior specifically to thefocus-visiblestate [1][5]. It is highly recommended to provide your own custom focus styling when using these utilities to maintain accessibility for keyboard users [1]. Summary of differences: - outline-hidden: Setsoutline-style: nonebut preserves visibility in forced-colors mode (recommended for most use cases that previously usedoutline-none) [1][2][3]. - outline-none: Setsoutline-style: noneexclusively [4][2]. It is standard practice to useoutline-hiddenif you are suppressing the default browser outline to implement your own custom focus indicators [4][3].Citations:
outline-nonewithoutline-hidden, add newoutline-nonetailwindlabs/tailwindcss#14926🏁 Script executed:
Repository: RileyTheRiler/French-Game
Length of output: 16924
Remove global
:focus-visibleoutline for these ring-styled buttonssrc/index.cssapplies a global:focus-visible { outline: 3px solid ... }(lines 72–75).WritingPad.jsxaddsfocus-visible:ring-*to the back/audio/guide toggle/color picker/stroke width buttons, but only usesfocus:outline-none(not:focus-visible), so keyboard focus can show both the global outline and the Tailwind ring.Add
focus-visible:outline-hiddento those buttons (Tailwind v4’s recommended outline-suppression that preserves forced-colors behavior) or scope the global:focus-visiblerule.🎯 Proposed fix: add `focus-visible:outline-hidden` to the buttons
<button onClick={() => navigate('/')} - className="p-2 rounded-lg bg-slate-800/50 hover:bg-slate-700/50 transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-indigo-400" + className="p-2 rounded-lg bg-slate-800/50 hover:bg-slate-700/50 transition-colors focus:outline-none focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-indigo-400" aria-label="Go back to previous page" >Apply the same
focus-visible:outline-hiddenaddition to the audio button (~line 344), guide toggle (~line 419), color picker buttons (~lines 452–458), and stroke width buttons (~line 469).Aria labels look good (including
title+aria-labelon the color picker).📝 Committable suggestion
🤖 Prompt for AI Agents