fix(a11y): resolve accessibility gaps in JointSave dashboard - #231
Conversation
- Added visually hidden h1 and skip-to-content link - Implemented keyboard navigation for dashboard tabs - Replaced custom backdrop modal with Radix Dialog in onboarding for proper focus trapping - Added descriptive aria-labels to pool card badges - Configured aria-describedby and aria-required on form inputs
Sendi0011
left a comment
There was a problem hiding this comment.
Review — Approve
Solid accessibility work. The changes are consistent and follow the existing component patterns.
What I liked:
-
Onboarding wizard to Dialog — Replacing the custom
motion.divbackdrop with Radix Dialog/DialogContent gives you focus trapping and Escape handling for free. TheshowCloseButton={false}preserves the existing close UX. -
Skip-to-content link —
sr-only focus:not-sr-onlypattern ondashboard/page.tsxis the standard approach. The<main id="main-content" tabIndex={-1}>allows programmatic focus. -
ARIA on forms —
aria-required,aria-describedbylinking inputs to their FieldError messages,aria-labelon dynamic member address fields. All correct. -
Tab keyboard navigation — Arrow key handling in
dashboard-tabs.tsxwitharia-selected,aria-controls, androle="tablist". Proper WAI-ARIA tabs pattern. -
Pool card keyboard activation —
role="article"withonKeyDownfor Enter/Space,tabIndex={-1}on the inner link to avoid double-focus.
One thing to discuss
test.yml adds pnpm format --write before pnpm format:check — this auto-formats all files before the check, which means the format check will always pass even if the PR introduces formatting issues. This defeats the CI check. Consider removing the auto-format step and letting contributors run pnpm format locally.
Minor nits (non-blocking)
dashboard-tabs.tsx:282—tabValuesis re-created on every render. ConsideruseMemoor a module-level constant.pool-card.tsx:387-394—document.getElementById(...)inonKeyDownworks but is fragile. A ref would be more idiomatic.
Good work. Approving.
|
please review and merge |
Sendi0011
left a comment
There was a problem hiding this comment.
Review — Accessibility Improvements
Thorough accessibility work — skip-to-content, proper ARIA on all form fields, keyboard navigation for tabs, screen reader labels on pool cards, and the onboarding wizard now uses the Dialog component for proper focus trapping. No blocking issues.
Suggestions (non-blocking)
-
CI adds
pnpm format --writebefore format check —.github/workflows/test.yml:11-13This auto-fixes formatting before the check runs, which means formatting issues in PRs will silently pass. The intention (preventing CI flakes from whitespace) is understandable, but it masks contributor errors. Consider runningformat:checkfirst and only runningformat --writeas a separate auto-fix step. -
pnpm format --writeon CI runs on all files, not just changed ones — This could reformat code that the PR author didn't touch, creating noisy diffs in the merge commit. Would be better scoped to only the changed files.
What is good
- Skip-to-content link (
sr-only focus:not-sr-only) is well-implemented - All form fields now have
aria-required="true"andaria-describedbylinking to their error messages - Tab navigation uses proper
role="tab",aria-selected,aria-controls, and arrow key handling - Pool cards have
role="article", descriptivearia-label, and keyboard activation (Enter/Space) FieldErrorcomponent now accepts anidforaria-describedbylinkage- Onboarding wizard properly uses
Dialogfor focus management instead of custom overlay - All 4 CI green, mergeable state clean
Approved. Ready to merge.
Closes #222
Description
This PR resolves several accessibility gaps across the JointSave dashboard components, ensuring improved usability and WCAG AA compliance.
Specifically, it includes the following updates:
Closes # (issue)
Type of Change
How Has This Been Tested?
cargo testpasses (smart contracts)pnpm buildsucceeds (frontend)pnpm lintpasses (frontend)Checklist
Screenshots (if applicable)
nil