Repository navigation
Accessibility fixes: text editor, group toggle, resize handle, reduced motion - #4246
Open
amanmahajan7 wants to merge 4 commits into
Open
amanmahajan7 wants to merge 4 commits into
amanmahajan7 wants to merge 4 commits into
Conversation
… and motion - Text editor: name the input after the column, and raise the border and placeholder contrast in light mode to 3.23:1 and 4.54:1 - Group toggle: render a native button instead of a focusable span, so it has a role and Enter/Space activation; the group row keeps aria-expanded - Resize handle: limit touch-action: none to the handle, so the header can be scrolled by touch - Only animate the sort and group carets without a reduced motion preference Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4246 +/- ##
==========================================
- Coverage 99.27% 99.23% -0.05%
==========================================
Files 44 44
Lines 2216 2221 +5
Branches 974 975 +1
==========================================
+ Hits 2200 2204 +4
- Misses 16 17 +1
🚀 New features to boost your workflow:
|
amanmahajan7
marked this pull request as ready for review
October 27, 2026 14:36
nstepien
reviewed
Oct 27, 2026
|
|
||
| return ( | ||
| <span className={groupCellContentClassname} tabIndex={tabIndex} onKeyDown={handleKeyDown}> | ||
| <button |
Collaborator
There was a problem hiding this comment.
The button does not cover the entire cell, could this be an accessibility issue?
| const caretClassname = `rdg-caret ${caret}`; | ||
|
|
||
| function handleKeyDown(event: React.KeyboardEvent<HTMLButtonElement>) { | ||
| // Shift + Space selects the row, so prevent it from activating the button |
Collaborator
There was a problem hiding this comment.
When the row is selected instead of the cell, we don't de/select the row, is that expected?
| function handleClick(event: React.MouseEvent<HTMLButtonElement>) { | ||
| // the group cell also toggles the group on click | ||
| event.stopPropagation(); | ||
| toggleGroup(); |
Collaborator
There was a problem hiding this comment.
- click on button
- toggles the group
- press space
- toggles the group
- arrow up + arrow down, back on the same row
- press space
- grid scrolls
The behavior is unexpected since visually it's the row that's focused, not the button.
Should we handle this on the cell itself?
Then we wouldn't need a button at all.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Small accessibility fixes from an audit of the grid against Chrome's Modern Web Guidance accessibility guides and the WAI-ARIA treegrid pattern. None of them change public APIs.
Changes
Text editor (
renderTextEditor)Accessible name: the input had none. It's now labeled with the column's
namewhen that name is a string. Columns with a React element as their name still get no label, since there's no text to use.Contrast in light mode:
#ccc, 1.61:1#8f8f8f, 3.23:1#999, 2.85:1#767676, 4.54:1Dark mode keeps
#cccfor the border (already 10:1) and#999for the placeholder (already 5.65:1), usinglight-dark().Group toggle (
ToggleGroup/renderToggleGroup)<span>with no role, toggled only by Enter.<button type="button">withall: unset. It gets the button role, focusability, and Enter/Space activation from the browser. The group cell still draws the focus outline.aria-expandedstays on the group row only, where the treegrid pattern puts it ("on either the row element or on a cell"). Checked in Chromium's accessibility tree on the row grouping demo: the button reports no expanded state, and the rows do.ToggleGroupis public, so it can't rely on the cell's handler: custom group cell renderers may use it in cells without one..rdg-group-cell-contentnow style a<button>instead of a<span>.Resize handle
touch-action: noneon the whole resizable header cell, so the grid couldn't be scrolled by touch from those headers.::afterpseudo-element of the header cell. Today the handle is its own element with its own pointer handlers.Reduced motion
transition: d 0.1s) now run only under@media (prefers-reduced-motion: no-preference).Testing
vp checkandvp run typecheckpass.New assertion: the editor's accessible name, in
renderTextEditor.test.tsx.Existing tests:
TreeDataGrid,renderTextEditorandresizablepass in Chromium, Firefox and WebKit. The full Chromium browser suite passes, except 3 sorting tests that also fail onmainon the same machine.Group toggle, temporary test: checked every way to toggle, in all three browsers:
I didn't commit it, because Space isn't required by the treegrid pattern; it works only because the toggle is now a native button.
Not tested automatically:
touch-actionmove and the reduced-motion rules, which are CSS-only;--disable-renderer-accessibility.🤖 Generated with Claude Code