Fix a11y cell navigation - #11
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…wame/Tablecraft into fix-a11y-cell-navigation
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
useTableA11ydeclaresrole="grid"androle="gridcell"and advertises theWAI-ARIA Grid pattern, but did
not keep that contract. This fixes four defects and adds the 2D keyboard
navigation the role promises, as an opt-in flag so nothing breaks.
Also cuts the release:
package.json→3.1.0, lockfile synced, CHANGELOGdated.
The bugs
1. The roving tabindex never roved. Nothing ever called
.focus()— onlytabIndexchanged. A keyboard user pressed ArrowDown and focus physically didnot move; they had to Tab again to reach the row the hook considered current.
This was the real defect, and the missing arrow keys were a symptom of it.
getRowProps/getCellPropsnow return areffor element registration, and akeypress focuses the newly-current element. Mounting a grid never steals focus —
only a keypress moves it.
2.
aria-rowindexrestarted on every page. It was computed againstgetRowModel()(the current page) whilearia-rowcountused the whole filteredset, so on page 2 of a paginated grid screen readers announced
"row 1 of 50, row 2 of 50…" for rows that were actually 11–20. It now reads
getPrePaginationRowModel()— deliberately notgetFilteredRowModel(),which would report positions in original data order and discard the user's sort.
There is a test that fails against that wrong fix specifically.
3. Cells were not focusable and could not know their row.
getCellPropsreturned only
roleandaria-colindex— notabIndex— so there was nowherefor horizontal navigation to land, and
getCellProps(columnIndex)had no way tocompute a per-cell tabindex.
4.
Home/Endwere spec-incorrect, not merely incomplete. In the gridpattern they move to the first/last cell within the current row, and
Ctrl+Home/Endmove to the first/last cell in the grid. They were jumpingrows.
Fixing bug 2 immediately broke the roving tabindex on paginated tables — zero
tabbable rows on page 2, a keyboard trap — because
tabIndexhad been derivedfrom the same index. Caught by a test written to prove it. The two are now
separate concerns:
aria-rowindexis global, the roving tabindex ispage-relative, because you can only focus a row that is actually rendered.
The new option
ArrowRight/ArrowLeftArrowDown/ArrowUpHome/EndCtrl+Home/EndPageDown/PageUpEnter/SpaceIn cell mode the roving tabindex moves from the row to the cell, so exactly one
cell in the grid is tabbable, and the row's
onKeyDowngoes inert — withoutthat, a keypress bubbling from cell to row would move twice. There is a test for
that specific interaction.
Breaking changes
None.
cellNavigationdefaults tofalse, which preserves the previousrow-level behaviour and row-scoped
Home/End.getCellProps(columnIndex)still works with one argument; the
rowIdsecond argument is optional.The one behaviour change is that focus now actually moves on arrow keys in row
mode too. That is the bug fix, not a new feature — a roving tabindex that
doesn't rove was never working as documented.
Testing
New DOM-level tests in
tests/a11y-focus.test.tsxrender a real grid and assertdocument.activeElementafter keypresses — the state-level tests could not havecaught bug 1, since
tabIndexwas updating correctly the whole time.Two tests initially passed vacuously (a sort-order assertion and a
selection toggle-off) and were rewritten so they fail against the wrong
implementation.
Known trade-off
registerElementreturns a fresh closure per render, so React detaches andreattaches refs each render. Correct, but not free on very large grids;
memoizing is a follow-up if it ever shows up in a profile.
Merge order
This branch carries the
3.1.0version bump and a[3.1.0] — 2026-09-02CHANGELOG section. #export-PR also has a
3.1.0 — unreleasedsection, so thetwo will conflict in
CHANGELOG.md(and inpackage.jsonif that branch isbumped too). Merge the export PR first, then this one, and collapse the two
CHANGELOG sections into a single 3.1.0 entry.