fix(devtools): Handle some additional query devtools accessibility findings #9961
Conversation
🦋 Changeset detectedLatest commit: 7988ef3 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 |
WalkthroughBumps Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
mattcosta7
changed the title
|
View your CI Pipeline Execution ↗ for commit 7988ef3
☁️ Nx Cloud last updated this comment at |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9961 +/- ##
===========================================
- Coverage 45.89% 20.91% -24.99%
===========================================
Files 200 42 -158
Lines 8437 2458 -5979
Branches 1943 637 -1306
===========================================
- Hits 3872 514 -3358
+ Misses 4116 1697 -2419
+ Partials 449 247 -202
🚀 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)
592-651: Excellent keyboard accessibility implementation for panel resizing.The separator role with proper ARIA attributes and keyboard navigation is well done. The direction-aware logic correctly handles all four panel positions.
Consider adding
aria-valueminandaria-valuemaxattributes to provide screen readers with the full value range context, aligning with the minHeight/minWidth constraints already defined.aria-valuenow={ position() === 'top' || position() === 'bottom' ? Number(props.localStore.height || DEFAULT_HEIGHT) : Number(props.localStore.width || DEFAULT_WIDTH) } + aria-valuemin={ + position() === 'top' || position() === 'bottom' + ? convertRemToPixels(3.5) + : convertRemToPixels(12) + } tabindex="0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/silver-peas-end.md(1 hunks)packages/query-devtools/src/Devtools.tsx(20 hunks)
🔇 Additional comments (11)
.changeset/silver-peas-end.md (1)
1-5: LGTM!Minor version bump is appropriate for these accessibility enhancements since they add new functionality (keyboard navigation, ARIA attributes) without breaking changes.
packages/query-devtools/src/Devtools.tsx (10)
420-425: Good accessibility practice: auto-focusing close button on panel mount.This ensures keyboard and screen reader users have immediate focus context when the devtools panel opens.
857-865: LGTM!Clear and descriptive aria-label for the view toggle radio group.
931-957: LGTM!Appropriate aria-labels for both query and mutation sort selectors.
1280-1286: LGTM!The aria-label clearly describes the purpose of this setting control.
1476-1493: Good pattern: comprehensive aria-label with hidden decorative indicators.The aria-label correctly includes disabled/static states, and
aria-hidden="true"on the visual indicators prevents redundant announcements.
2013-2019: LGTM!Using
role="heading"witharia-level="2"provides proper document structure for assistive technologies navigating the Query Details section.
2249-2252: LGTM!Clear aria-label for the error type selector.
2317-2324: LGTM!The aria-label "Edit query data as JSON" clearly communicates both the purpose and expected format.
3006-3014: Good focus-visible implementation for the drag handle.The
:focus-visibleoutline ensures keyboard users see a clear focus indicator while avoiding focus rings on mouse interactions. The background color change on:focusprovides a fallback visual cue.
2445-2451: LGTM!Consistent heading semantics applied to MutationDetails sections, matching the QueryDetails pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/query-devtools/src/Devtools.tsx (1)
485-495: Bug:removeEventListeneruses'mouseUp'instead of'mouseup', leaving a dangling listenerIn the drag resize logic, the cleanup unsubscribes
mousemovecorrectly but attempts to remove a'mouseUp'listener, while the event was added as'mouseup'. This means themouseuplistener will never be removed.const unsubscribe = () => { if (isResizing()) { setIsResizing(false) } document.removeEventListener('mousemove', runDrag, false) - document.removeEventListener('mouseUp', unsubscribe, false) + document.removeEventListener('mouseup', unsubscribe, false) } document.addEventListener('mousemove', runDrag, false) document.addEventListener('mouseup', unsubscribe, false)Fixing the event name prevents unnecessary residual listeners and potential subtle bugs in long‑lived devtools sessions.
🧹 Nitpick comments (4)
packages/query-devtools/src/Devtools.tsx (4)
420-425: Close button auto‑focus on mount looks good, but consider a small guardFocusing the close button on mount is a solid accessibility improvement for keyboard and screen‑reader users. To be extra defensive against any lifecycle edge cases, you could guard the ref before calling
focus().// Focus the close button when the panel opens for screen reader accessibility onMount(() => { - closeBtnRef.focus() + closeBtnRef && closeBtnRef.focus() })This doesn’t change behavior in the normal case but avoids a possible runtime error if the ref isn’t set for any reason.
Also applies to: 654-662
592-605: Keyboard-resizable separator and focus styles are well done; consider adding value range ARIAThe separator + keyboard resizing logic lines up correctly with the panel position, and the focus/
focus-visiblestyles on the drag handle provide a clear visible focus indication without double outlines. This is a nice accessibility win.One improvement to make the control more self‑describing to assistive tech: expose the minimum (and optionally maximum) sizes via ARIA so
aria-valuenowisn’t “floating” without context.For example:
<div role="separator" aria-orientation={ position() === 'top' || position() === 'bottom' ? 'horizontal' : 'vertical' } aria-label="Resize devtools panel" aria-valuenow={ position() === 'top' || position() === 'bottom' ? Number(props.localStore.height || DEFAULT_HEIGHT) : Number(props.localStore.width || DEFAULT_WIDTH) } + aria-valuemin={ + position() === 'top' || position() === 'bottom' + ? convertRemToPixels(3.5) + : convertRemToPixels(12) + } + // Optionally expose a max value too, if you enforce one + // aria-valuemax={...} tabindex="0"This keeps the current UX but improves screen‑reader output about the control’s range.
Also applies to: 612-651, 3004-3012
1476-1476: Query row ARIA label and hidden indicators are a good pattern; consider a more human‑friendly keyEncoding the disabled/static state once in the button
aria-labeland marking the visual “disabled/static” indicators asaria-hidden="true"is a clean way to avoid redundant announcements.Depending on how readable
queryHashtends to be, you might want to expose a more human‑friendly description (e.g., derived fromqueryKey) in the label, and possibly fall back to the hash:- aria-label={`Query key ${props.query.queryHash}${isDisabled() ? ', disabled' : ''}${isStatic() ? ', static' : ''}`} + aria-label={`Query ${ + displayValue(props.query.queryKey, true) || props.query.queryHash + }${isDisabled() ? ', disabled' : ''}${isStatic() ? ', static' : ''}`}This will make screen‑reader output less cryptic when query hashes are long or opaque.
Also applies to: 1485-1487, 1490-1492
2013-2019: Section headings and live status regions are structured well; minor ARIA polish possibleUsing
<div role="heading" aria-level="2">for the various “Query Details”, “Actions”, “Data Explorer”, “Query Explorer”, and mutation detail sections gives assistive tech a clear document outline without disturbing existing layout/styles. Likewise, marking the status chips withrole="status"andaria-live="polite"will help announce state changes non‑intrusively.Small optional polish:
role="status"already impliesaria-live="polite, so you can drop the explicitaria-liveif you’d like to reduce redundancy, though it’s not harmful to keep.No required changes here.
Also applies to: 2032-2034, 2050-2055, 2271-2277, 2356-2360, 2445-2450, 2470-2472, 2487-2492, 2506-2511, 2525-2530, 2544-2549
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/query-devtools/src/Devtools.tsx(21 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/query-devtools/src/Devtools.tsx
⏰ 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). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (1)
packages/query-devtools/src/Devtools.tsx (1)
861-861: ARIA labels on toggles, selects, and textarea are clear and descriptiveThe added
aria-labels for:
- the view toggle radio group (“Toggle between queries and mutations view”),
- sort
<select>s (“Sort queries by” / “Sort mutations by”),- the “Hide disabled queries” radio group,
- the error‑type
<select>,- and the JSON edit
<textarea>,are all specific and accurately describe the control purpose. This should meaningfully improve screen‑reader navigation without changing visual behavior.
No changes requested here.
Also applies to: 935-935, 948-948, 1282-1283, 2250-2250, 2318-2318
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-devtools/src/Devtools.tsx (1)
593-656: Consider addingaria-valuemaxfor complete range information.The ARIA attributes and keyboard navigation implementation look excellent. However, the separator is missing an
aria-valuemaxattribute. While the maximum is dynamic (viewport size), you could set it to a reasonable approximation (e.g., 90% of viewport) or update it dynamically. This would help screen reader users understand the full resize range.Consider adding:
aria-valuenow={ position() === 'top' || position() === 'bottom' ? Number(props.localStore.height || DEFAULT_HEIGHT) : Number(props.localStore.width || DEFAULT_WIDTH) } + aria-valuemax={ + position() === 'top' || position() === 'bottom' + ? typeof window !== 'undefined' ? Math.floor(window.innerHeight * 0.9) : undefined + : typeof window !== 'undefined' ? Math.floor(window.innerWidth * 0.9) : undefined + } tabindex="0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/query-devtools/src/Devtools.tsx(29 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/query-devtools/src/Devtools.tsx
⏰ 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). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (11)
packages/query-devtools/src/Devtools.tsx (11)
420-425: Good accessibility enhancement with a minor consideration.Auto-focusing the close button when the panel opens helps keyboard and screen reader users quickly discover the panel and have a clear exit point. However, be aware this moves focus away from where the user was previously interacting, which could be disruptive in some workflows (e.g., if they triggered the panel via keyboard shortcut while focused on application content).
865-865: LGTM!The aria-label clearly describes the purpose of the radio group toggle.
939-939: LGTM!Clear aria-labels for the sort select elements, properly differentiated for queries vs mutations.
Also applies to: 953-953
1287-1287: LGTM!Descriptive aria-label for the disabled queries visibility setting.
1481-1481: Excellent accessibility pattern!The aria-label enhancement to include disabled and static states, combined with marking the visual indicators as
aria-hidden="true", is the correct approach. This prevents duplicate announcements while ensuring screen reader users receive all relevant information.Also applies to: 1490-1497
1786-1786: LGTM!Proper use of aria-label to convey status information and aria-hidden to suppress the decorative visual indicator.
Also applies to: 1818-1818
2020-2024: LGTM!Proper use of
role="heading"witharia-level="2"to provide semantic document structure for assistive technologies without changing the visual hierarchy. This significantly improves navigation for screen reader users.Also applies to: 2056-2060, 2284-2288, 2369-2373
2039-2040: LGTM!Correct use of
role="status"witharia-live="polite"for status indicators. The "polite" setting appropriately ensures status changes are announced without interrupting other screen reader output.Also applies to: 2484-2485
2080-2081: LGTM!Proper use of
aria-hidden="true"on decorative status indicators. The button labels already convey the action, so hiding the visual dots prevents redundant announcements.Also applies to: 2106-2106, 2131-2131, 2157-2157, 2207-2207, 2237-2237, 2256-2256
2332-2332: LGTM!The textarea aria-label clearly describes its purpose. The heading structure in MutationDetails consistently follows the same accessible pattern as QueryDetails, providing proper semantic navigation for screen reader users.
Also applies to: 2458-2561
3018-3026: Review focus management pattern.The
:focus-visibleimplementation is good and provides proper keyboard focus indication. However, the plain:focusrule (lines 3018-3021) removes the outline, which could cause accessibility issues in browsers that don't fully support:focus-visible. While the background color change provides some visual feedback, relying solely on it may not meet WCAG contrast requirements in all cases.Consider whether the plain
:focusrule is necessary, or if you can rely on default browser behavior +:focus-visibleoverride for better accessibility coverage.Since this is in the chill mode review and the background color change provides some visual feedback, this is more of a "good to verify" than a critical issue.
mattcosta7
changed the title
| } | ||
| document.removeEventListener('mousemove', runDrag, false) | ||
| document.removeEventListener('mouseUp', unsubscribe, false) | ||
| document.removeEventListener('mouseup', unsubscribe, false) |
There was a problem hiding this comment.
this was suggested by coderabbit - it's outside of the current PR's scope, but figured it worth updating while here
1 participant
🎯 Changes
This pull request introduces significant accessibility improvements to the TanStack Query Devtools panel. The changes mainly enhance keyboard navigation, screen reader support, and semantic markup for better usability and compliance with accessibility standards.
Accessibility improvements for screen readers and keyboard navigation:
aria-hiddenwhere appropriate, preventing redundant or confusing information for screen readers. [1] [2] [3] [4] [5] [6] [7] [8](References: [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21] [22] [23] [24] [25] [26] [27] [28] [29]
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.