fix(a11y): color contrast, skip link, focus rings, and aria attributes#421
fix(a11y): color contrast, skip link, focus rings, and aria attributes#421gabitoesmiapodo merged 6 commits intofeat/ai-integrationfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR improves accessibility across the app by addressing color-contrast issues, adding keyboard navigation support (skip link + focus rings), and improving screen-reader semantics with ARIA attributes.
Changes:
- Update semantic color tokens (warning/ok/danger) to improve WCAG contrast in light/dark modes.
- Add a skip-to-main-content link and restore visible focus styling for icon-only controls.
- Add/extend ARIA attributes for validation and icon-only buttons/links.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/__root.tsx | Adds a skip-to-main-content link and a main-content anchor target. |
| src/components/ui/provider.tsx | Updates semantic token colors for contrast-compliant warning/ok/danger states. |
| src/components/sharedComponents/ui/Modal/index.tsx | Adds aria-label to icon-only close button. |
| src/components/sharedComponents/ui/ExternalLink/index.tsx | Replaces outline="none" with _focusVisible focus ring styling. |
| src/components/sharedComponents/ui/CopyButton/index.tsx | Replaces outline="none" with _focusVisible focus ring styling. |
| src/components/sharedComponents/TokenInput/index.tsx | Adds aria-label to icon-only close button in the token dialog. |
| src/components/sharedComponents/Hash.tsx | Adds aria-label to icon-only explorer external link. |
| src/components/sharedComponents/BigNumberInput.tsx | Introduces aria-invalid wiring based on range validation failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
88ed222 to
27b5511
Compare
b84e168 to
184f8ea
Compare
27b5511 to
5d4e886
Compare
184f8ea to
13e024b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/components/sharedComponents/BigNumberInput.tsx:74
hasErroris only updated insideupdateValue, so if the parent updatesvalueprogrammatically (e.g. reset to 0 / set max) after an out-of-range entry,aria-invalidcan remain stucktrueeven though the newvalueis valid. Consider deriving/clearinghasErrorin an effect whenvalue/min/maxchange, or resetting it whenevervalueis externally synchronized.
const [hasError, setHasError] = useState(false)
// update inputValue when value changes
useEffect(() => {
const current = inputRef.current
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5d4e886 to
5c9aa29
Compare
13e024b to
3b7b376
Compare
…butes - Fix warning color contrast: #cc0 fails WCAG AA; use #996600 (light) / #e6b800 (dark) - Fix danger/ok colors: add proper light/dark variants (#ff6666, #66ee66) for dark mode contrast - Add skip-to-main-content link in root layout for keyboard navigation - Add aria-invalid to BigNumberInput default input on range validation failure - Replace outline:none with _focusVisible focus ring on CopyButton and ExternalLink - Add aria-label="Close" to icon-only close buttons in Modal and TokenInput - Add aria-label="View on explorer" to icon-only ExternalLink in Hash component
- Fix malformed transition string in CopyButton and ExternalLink (missing
closing brace on last token)
- Move aria-invalid into inputProps so it applies to both the default input
and any custom renderInput renderer
- Clear hasError state on empty-string input path (was only cleared on
valid range path, leaving aria-invalid=true after user clears the field)
- Add tabIndex={-1} to #main-content so skip link activation moves
keyboard focus into the main region
An intermediate/unparseable input string while the user is typing could cause the useEffect that syncs the DOM value on external prop changes to throw. Wrap parseUnits in a try/catch and use a sentinel value to force the DOM overwrite when the current string is not parseable.
657ada0 to
b0fe779
Compare
22c9e54 to
6a2f37e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/components/sharedComponents/BigNumberInput.tsx:153
- New
aria-invalidbehavior isn’t covered by existing tests. Since there’s alreadyBigNumberInput.test.tsx, add assertions thataria-invalidis set when range validation fails and cleared again when the value becomes valid / empty (and for other invalid input cases, if you decide to flag them).
const inputProps = {
'aria-invalid': (hasError || undefined) as true | undefined,
disabled,
onChange: updateValue,
placeholder,
type: 'text',
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Also exclude .worktrees/ from vitest to prevent cross-worktree test pollution.
Summary
#cc0fails WCAG AA;#996600(light) /#e6b800(dark)#ff6666,#66ee66)aria-invalidtoBigNumberInputon range validation failureoutline:nonewith_focusVisiblefocus ring onCopyButtonandExternalLinkaria-label="Close"to icon-only close buttons inModalandTokenInputaria-label="View on explorer"to icon-onlyExternalLinkinHashTest plan
pnpm testpasses