Improve devtools accessibility by adding aria attributes, using semantic components and field labels #9806
Conversation
🦋 Changeset detectedLatest commit: caa1baf The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughRefactors Query Devtools UI: replaces imperative DropdownMenu item selections with declarative RadioGroup/RadioItem controls for position, theme, and disabled-queries; adds ARIA attributes (aria-label, aria-live/role/atomic, aria-expanded) and links form labels/inputs with generated IDs for improved accessibility. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit caa1baf
☁️ Nx Cloud last updated this comment at |
| aria-label="Open settings menu" | ||
| title="Open settings menu" |
There was a problem hiding this comment.
This menu item has no visible text, so adding label and title to account for this for screen readers
| <span>Right</span> | ||
| <ArrowRight /> | ||
| </DropdownMenu.Item> | ||
| <DropdownMenu.RadioGroup aria-label="Position settings" value={props.localStore.position} onChange={value => setDevtoolsPosition(value as DevtoolsPosition)}> |
There was a problem hiding this comment.
Replaced DropdownMenu.Item with DropdownMenu.Radio(Group|Item)
These internally handle checked states - so moved the selected styles into data-checked instead of a separate class
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9806 +/- ##
=======================================
Coverage ? 21.35%
=======================================
Files ? 42
Lines ? 2388
Branches ? 600
=======================================
Hits ? 510
Misses ? 1641
Partials ? 237
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-devtools/src/Devtools.tsx (1)
1217-1257: Consider letting data-checked handle selection indication.The
RadioGroupcorrectly implements controlled selection, but the manualCheckCircleicons (lines 1232-1238, 1249-1255) are redundant with the[data-checked]styling defined at lines 3519-3528. Thedata-checkedattribute already provides visual indication of the selected state.You could simplify by removing the manual icon logic and relying solely on the CSS styling:
<DropdownMenu.RadioItem value="false" class={cx( styles().settingsSubButton, 'tsqd-settings-menu-position-btn', 'tsqd-settings-menu-position-btn-show', )} > <span>Show</span> - <Show when={props.localStore.hideDisabledQueries !== 'true'}> - <CheckCircle /> - </Show> </DropdownMenu.RadioItem> <DropdownMenu.RadioItem value="true" class={cx( styles().settingsSubButton, 'tsqd-settings-menu-position-btn', 'tsqd-settings-menu-position-btn-hide', )} > <span>Hide</span> - <Show when={props.localStore.hideDisabledQueries === 'true'}> - <CheckCircle /> - </Show> </DropdownMenu.RadioItem>If you want to keep the icon for extra visual clarity, consider adding it via CSS using the
data-checkedselector to maintain consistency with the RadioGroup pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/slick-clubs-lay.md(1 hunks)packages/query-devtools/src/Devtools.tsx(8 hunks)packages/query-devtools/src/Explorer.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-devtools/src/Devtools.tsx (2)
packages/query-devtools/src/contexts/QueryDevtoolsContext.ts (1)
DevtoolsPosition(6-6)packages/query-devtools/src/icons/index.tsx (8)
ArrowUp(63-81)ArrowDown(83-101)ArrowLeft(103-124)ArrowRight(126-147)Sun(149-167)Moon(169-187)Monitor(189-207)CheckCircle(432-450)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (7)
packages/query-devtools/src/Explorer.tsx (1)
335-335: LGTM! aria-expanded enhances screen reader support.Adding
aria-expandedto the expander button properly communicates the expanded/collapsed state to assistive technologies.packages/query-devtools/src/Devtools.tsx (5)
1003-1003: LGTM! Good accessibility improvements.Removing unnecessary template literals and adding
aria-labelto icon-only buttons improves code clarity and screen reader support.Also applies to: 1016-1017
1066-1117: Excellent refactor to RadioGroup pattern.Replacing imperative
onSelecthandlers with the declarativeRadioGroup/RadioItempattern is both more maintainable and semantically correct for mutually exclusive options. The controlled value-based selection withonChangeandaria-labelproperly communicates the menu's purpose to assistive technologies.
1147-1187: LGTM! Consistent RadioGroup implementation.The theme preference menu follows the same accessible pattern as the position settings, with appropriate
aria-labeland controlled selection.
1953-1955: Excellent use of ARIA live regions.Adding
role="status",aria-live="polite", andaria-atomic="true"to the details panels properly announces query/mutation state changes to screen readers without interrupting other announcements.Also applies to: 2367-2369
3519-3528: LGTM! Appropriate styling for RadioItem selection.The
[data-checked]selector is the standard wayRadioItemcommunicates selection state. The styling provides clear visual feedback for the selected option..changeset/slick-clubs-lay.md (1)
1-5: LGTM! Changeset properly documents the accessibility improvements.The patch release type is appropriate for these non-breaking accessibility enhancements.
mattcosta7
changed the title
1 participant
🎯 Changes
An a11y review we ran internally found some improvements could be made to our usage of the devtools panel.
Making a few tweaks to those items here to help close those on our end.
This pull request refactors the settings menus in
Devtools.tsxto use accessible radio groups for mutually exclusive options, improving both accessibility and code maintainability. It also adds ARIA attributes to key interactive elements throughout the UI for better screen reader support, and updates styling to visually indicate selected options using thedata-checkedattribute.Slightly clearer when viewed without whitespace diffs - https://github.com/TanStack/query/pull/9806/files?diff=split&w=1
I don't see any tests related to these currently, and haven't added here, but happy to try to do something like that with some guidance - although the changes here should not substantially change any behaviors as they're mostly semantic
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit