fix(a11y): resolve accessibility issues in JsonViewer, SearchBox, and SelectOnClick - #2800
fix(a11y): resolve accessibility issues in JsonViewer, SearchBox, and SelectOnClick#2800ApurveKaranwal wants to merge 2 commits into
Conversation
- Replace generic list markup with role='tree', role='group', role='treeitem' - Set aria-expanded on li[role='treeitem'] elements for collapsible nodes - Sync aria-expanded on both collapser button and parent treeitem on toggle - Add Space key support alongside Enter for collapser keyboard activation - Add unit tests for ARIA roles, Enter key toggle, and Space key toggle - Keep SearchBox clear button and SelectOnClick as separate button/focus fixes
|
@AlexVarchuk please review my PR, and let me know if anything needs to be changed. |
There was a problem hiding this comment.
(🤖 Claude Opus 5) Not a maintainer — just a fellow contributor who has been working in jsonToHtml.ts on #2697, so this landed in familiar territory. Thanks for taking this on; a11y work on a JSON tree renderer is genuinely fiddly and there is real substance here. I checked the branch out and ran it, so the notes below are verified rather than guesses.
Things this gets right
It fixes a real bug that nobody had noticed. On main, the collapser aria-label uses level > maxExpandLevel + 1 while the collapsed class uses level > maxExpandLevel — off by one. Rendering {name:'x', nested:{a:1}} on main gives aria-label="collapse" on both collapsers, even though nested really is collapsed. Aligning both to the same condition is the correct fix, and expand object / collapse array is a nice improvement over the bare verb.
It deletes genuinely dead code. The old focus listener tested event.key === 'Enter', but FocusEvent has no key, and focus does not bubble up to the container either. It could never have fired.
A few other details I appreciated: isCollapsible correctly omits aria-expanded on leaf nodes rather than emitting false (easy to get wrong); type="button" avoids accidental form submission; and the non-null assertions became real null guards. Full suite passes — 279 tests, 27 suites.
Two larger things
The two new keyboard tests do not exercise the handler. Details inline, but briefly: the root collapser already carries aria-label="collapse object" and aria-expanded="true" on mount, and those are exactly the two assertions. I commented out the keydown listener registration and all 8 tests still passed.
role="tree" commits to an interaction model that is not implemented here. Details inline. Short version: li[role="treeitem"] has no tabindex, the only focusable element is the button.collapser nested inside the treeitem, and there is no Arrow/Home/End navigation. The pre-existing "arrow key navigation" test covers horizontal scrolling of the container, not tree traversal. My suggestion is to drop the tree/treeitem/group roles and keep the <button aria-expanded> disclosure pattern, which is well supported and which the rest of this PR already implements correctly. That would also shrink the diff a fair bit.
Smaller notes
aria-live="polite"on the no-results box (line 201) probably will not announce. Live regions generally need to be in the DOM before their content changes; here the element and its text are created together. An always-rendered empty region that you fill in tends to work better.role="menu"on the results box implies a command menu with arrow-key navigation. Search suggestions are conventionally combobox +role="listbox"/optionwitharia-activedescendant. The children do renderrole="menuitem", so it is at least internally consistent — just probably not the pattern you want.- Hardcoded English.
"Clear search","Select all text","Search results: N results found", andexpand object/collapse arraybypassl()insrc/services/Labels.ts, which is how Redoc localizes UI text (noResultsFoundis already in there). Since these strings are now the primary text screen-reader users hear, being untranslatable matters more than it would for a tooltip. Theresult${n !== 1 ? 's' : ''}pluralization has the same issue. Worth adding label keys. - Scope: the README troubleshooting section and CDN rewrite are unrelated to a11y, and the rewrite also removes the 1.x CDN URLs. Splitting them into their own PR would likely get both merged faster.
- Minor:
isCollapsiblere-implements theconstructor === Array/constructor === Date/typeof === 'object'dispatch thatvalueToHTMLalready does — could share a helper. AndcollapsedClassinside both loops is the same expression as the outercollapsed, sinceleveldoes not change in the loop body.
None of this touches the core insight, which is sound: these controls were missing aria-expanded and the labels were wrong. Mostly I would scope down to that and land it.
| }); | ||
| component.update(); | ||
|
|
||
| expect(component.html()).toContain('aria-label="collapse object"'); |
There was a problem hiding this comment.
(🤖 Claude Opus 5) These two assertions hold on a freshly-mounted component, before any key is dispatched — the root collapser is expanded by default, so aria-label="collapse object" and aria-expanded="true" are both already present somewhere in component.html().
I confirmed it: I commented out node?.addEventListener('keydown', keydownListener) in JsonViewer.tsx and re-ran this file. All 8 tests still passed, including this one and the Space variant.
The fix is to assert against the specific element you dispatched on rather than the whole HTML string:
const collapser = component.getDOMNode()
.querySelector('button.collapser[aria-label="expand object"]')!;
// ... dispatch Enter on `collapser` ...
expect(collapser.getAttribute('aria-expanded')).toBe('true');
expect(collapser.getAttribute('aria-label')).toBe('collapse object');Worth noting for the manual check too: jsdom does not implement a button's activation behavior for synthetic KeyboardEvents, so this suite cannot detect a double-toggle if preventDefault() ever fails to suppress the browser's click.
| level = 1; | ||
| let output = ''; | ||
| output += '<div class="redoc-json">'; | ||
| output += '<div class="redoc-json" role="tree">'; |
There was a problem hiding this comment.
(🤖 Claude Opus 5) I would push back gently on adopting the tree roles at all in this PR.
The ARIA tree pattern expects treeitems to be focusable via a roving tabindex with Arrow/Home/End navigation, and expects treeitems not to contain nested focusable widgets. What actually renders here is:
<div class="redoc-json" role="tree">
<code>
<button class="collapser" aria-expanded="true"></button>{
<ul class="obj collapsible" role="group">
<li role="treeitem" aria-expanded="false">
"nested": <button class="collapser" aria-expanded="false"></button>{ ... }Three issues with that:
- No
li[role="treeitem"]gets atabindex, and there is no arrow-key traversal in this PR or onmain— the existing "arrow key navigation" test covers horizontal scrolling of thePrismDiv. So the only way to move around is Tab between the nested buttons, which is not how a screen reader user will expect a tree to behave once it is announced as one. - The generic
<code>sits betweenrole="tree"androle="group", which breaks the owned-element relationship the tree role requires. aria-expandedends up on both theli[role="treeitem"]and thebuttoninside it, so the expanded/collapsed state gets announced twice.
Announcing a tree that does not behave like one is often worse for AT users than a plain set of disclosure buttons. Since the rest of the PR already implements the disclosure pattern correctly (<button> + aria-expanded + accurate label), my suggestion is to drop role="tree", role="group" and role="treeitem" here and keep the parts that fix the labels and state. If you do want the full tree pattern, it is a good follow-up PR on its own — it needs roving tabindex and key handling to be worth having.
| }} | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-label="Select all text" |
There was a problem hiding this comment.
(🤖 Claude Opus 5) This one I think is a net regression, and it is subtle.
This div already had role="button" before your change, so its accessible name was being computed from its contents — which is the server URL. Adding aria-label replaces that name, so a screen reader user now hears "Select all text, button" and never hears the URL itself. The visible text is still there, but for role="button" the name wins.
Since the whole point of this wrapper is to expose the server URL, I would either leave the name coming from content, or keep the label as a description instead:
<div ... role="button" title="Select all text">or an aria-describedby pointing at visually-hidden helper text. The onKeyDown addition alongside it is a good catch though — role="button" on a div genuinely does need it.
| onClick={this.clear} | ||
| aria-label="Clear search" | ||
| role="button" | ||
| tabIndex={0} |
There was a problem hiding this comment.
(🤖 Claude Opus 5) Making this focusable is the right instinct, but it creates an odd tab order. ClearIcon is styled.i with position: absolute; right: ..., and it is rendered before SearchIcon and SearchInput in the DOM. So with tabIndex={0}, keyboard users tab onto the clear button before reaching the input — visually jumping from the right edge back to the field.
Two birds with one stone: make it an actual <button type="button"> and render it after SearchInput. That gives you correct tab order, Enter/Space activation for free (so the manual onKeyDown and role="button" can both go away), and real focus styling. The absolute positioning keeps it visually where it is today.
What/Why/How?
aria-expandedand descriptivearia-labelattributes. Handledkeydown(Enter) triggers insideJsonViewer.tsxand calledevent.preventDefault()to prevent duplicate execution from the browser's synthetic click on<button>elements. Wrapped handlers inReact.useCallbackto satisfy ESLint warnings.tabIndex={0},role="button", andaria-label="Clear search"to the clear search icon, alongside anonKeyDownhandler listening forEnter/Space. Annotated search result box withrole="menu", an dynamic results-foundaria-label, andaria-live="polite"for the empty-results state.tabIndex={0},role="button", andaria-label="Select all text", enabling keyboard activation viaEnterorSpace.README.mdto help developers easily diagnose rendering or search issues.Reference
Resolves accessibility gaps (WCAG 2.1 AA) in interactive components.
Tests
src/components/__tests__/JsonViewer.tsxto verify:aria-expandedandaria-labelvalues.Entercorrectly toggles the state of collapsers.npm run unit).##Fixes #2799
Screenshots (optional)
N/A (semantic markup changes)
Check yourself