Merge pull request #88 from jeyabbalas/fix/keyboard-trap-grid-pattern

Finish the keyboard trap fix and prove it in a real browser

Author: jeyabbalas

.changeset/keyboard-trap-and-grid-pattern.md

@@ -6,16 +6,22 @@ Tab now moves through the table instead of getting stuck in it, every per-column
 
 `KeyboardNavigator` called `e.preventDefault()` on every `Tab` before deciding anything, from a bubble-phase listener on `.dt-root` — so it swallowed Tab from the root and from every descendant, and `moveFocusTab` then returned at the grid boundary with the default already suppressed. `.dt-root` also carried `tabindex="0"` under a `.dt-root:focus { outline: none }` rule, so Tab from the page landed on the table invisibly and the next Tab went nowhere. On a 266-column table, `document.querySelector('.dt-root :focus')` stayed `null` across 900+ consecutive Tab presses — a WCAG 2.1.2 "No Keyboard Trap" (Level A) failure that also took 2.1.1 (the ~1,600 header buttons were unreachable) and 2.4.7 (the grid's only tab stop hid its focus ring) with it.
 
-The `Tab` branch and `moveFocusTab` are deleted outright — no boundary logic that can regress — and the ARIA grid moves onto a new inner `.dt-grid[role="grid"][tabindex="0"]` that wraps only the header area and the body scroller. `.dt-root` keeps its class, border and container name but sheds its role, `tabindex` and `aria-*`: it hosts the grid *and* the toolbar filter bar, the status live region and the toolbar hidden-columns gutter, none of which a grid may own. The whole table now contributes exactly three tab stops — the grid cursor plus the header and body scroll regions, which WCAG 2.1.1 requires to be keyboard-reachable — and all three disappear before data is loaded. Everything else inside is `tabindex="-1"`, and the cursor is published through `aria-activedescendant` rather than by moving DOM focus, because the body's pooled row recycler would otherwise carry real focus into the pool. Since a click parks real focus on whatever it hit, the grid takes focus back on the next cursor keystroke rather than on the click, which keeps pointer interactions — and the annotation popovers that open on `focusin` — untouched. The cursor now spans the header row too (`focusedCell.row === -1`), so `↑` from body row 0 reaches the column headers, `←`/`→` walk them, `Enter`/`Space` sorts, and `F2` hands real focus to that header's buttons with `←`/`→` to cycle and `Escape` to come back. Body cells become `role="gridcell"`, `aria-rowcount` becomes `totalRows + 1` and body `aria-rowindex` becomes `row + 2`, since the header is row 1 under `role="grid"`.
+Deleting the `preventDefault()` turned out not to be enough on its own. Focus ownership simply moved from `preventDefault()` to a `focus()` call: the dispatcher reclaimed `.dt-grid` for _every_ key that got past its cursor-key gate, `Tab` included, so by the time the browser looked for the next element in sequential order it was starting from the grid again — and walked straight back into the grid's first tabbable descendant. Forward `Tab` looped on `.dt-header-scroll` indefinitely (80 consecutive presses, no escape, at 4 columns and at 266); `Shift+Tab` still got out, because backwards from `.dt-grid` lands before it rather than inside it. The invariant that closes it for good is now the first thing `KeyboardNavigator`'s header comment says: **a branch that does not act on a key must not move focus either.** `claimGridFocus()` is called from the individual action paths — the cursor moves, the Enter/Space sort, the row-select toggle, Escape — instead of once up front, so an unhandled key is inert by construction rather than because somebody remembered to enumerate it.
 
-Two visual changes come with it. The filter bar moves above the column headers, since it cannot live inside the grid element. And the light theme's `--dt-text-secondary` / `--dt-text-tertiary` darken to `#374151` / `#4b5563`, with dark-mode `--dt-text-tertiary` lifting to `#b8bfc9`: the old values were chosen to read on dark `#1f2937` but were used in both themes, leaving the column-header stats at 2.31:1 against a hovered header where AA wants 4.5:1. `.dt-col-stats`'s second line swaps `opacity: 0.8` for an explicit colour — opacity composites against whatever is behind the text, which is why that line failed contrast in *dark* mode too. The tertiary/secondary distinction is quieter than before in both themes; that is the cost of clearing AA at 11.2px.
+The `Tab` branch and `moveFocusTab` are deleted outright — no boundary logic that can regress — and the ARIA grid moves onto a new inner `.dt-grid[role="grid"][tabindex="0"]` that wraps only the header area and the body scroller. `.dt-root` keeps its class, border and container name but sheds its role, `tabindex` and `aria-*`: it hosts the grid *and* the toolbar filter bar, the status live region and the toolbar hidden-columns gutter, none of which a grid may own. The whole table now contributes exactly five tab stops — the filter bar, the grid cursor, the header and body scroll regions, which WCAG 2.1.1 requires to be keyboard-reachable, and the hidden-columns gutter — and that count holds at any column count, with any number of columns hidden and any number of filters active. The filter bar and the gutter reach one stop each by being `role="toolbar"`s with the APG roving-tabindex treatment rather than plain rows of buttons: the gutter used to emit a focusable button per hidden column, so hiding 250 of 266 columns added 251 stops, most of them clipped out of sight by the gutter's own `max-height`, and the filter bar added one per chip. The three grid stops disappear before data is loaded. Everything else inside is `tabindex="-1"`, and the cursor is published through `aria-activedescendant` rather than by moving DOM focus, because the body's pooled row recycler would otherwise carry real focus into the pool. Since a click parks real focus on whatever it hit, the grid takes focus back on the next cursor keystroke rather than on the click, which keeps pointer interactions — and the annotation popovers that open on `focusin` — untouched. The cursor now spans the header row too (`focusedCell.row === -1`), so `↑` from body row 0 reaches the column headers, `←`/`→` walk them, `Enter`/`Space` sorts, and `F2` hands real focus to that header's buttons with `←`/`→` to cycle and `Escape` to come back. Body cells become `role="gridcell"`, `aria-rowcount` becomes the rendered row count plus 1 and body `aria-rowindex` becomes `row + 2`, since the header is row 1 under `role="grid"`.
 
-Two trade-offs are deliberate. Column resize and drag-to-reorder stay mouse-only and are excluded from the F2 cycle rather than being given a focus stop that does nothing on Enter — keyboard resize and reorder need designed gestures, which is a feature rather than this fix. And an unloaded table now carries no grid semantics at all: the empty shell owns no rows, so `role="grid"` on it would be an `aria-required-children` violation, and a tab stop with nothing to navigate is noise.
+Two visual changes come with it. The filter bar moves above the column headers, since it cannot live inside the grid element. And a colour-token sweep clears the contrast bars the shipped defaults were missing. The light theme's `--dt-text-secondary` / `--dt-text-tertiary` darken to `#374151` / `#4b5563`, with dark-mode `--dt-text-tertiary` lifting to `#b8bfc9`: the old values were chosen to read on dark `#1f2937` but were used in both themes, leaving the column-header stats at 2.31:1 against a hovered header where AA wants 4.5:1. `.dt-col-stats`'s second line swaps `opacity: 0.8` for an explicit colour — opacity composites against whatever is behind the text, which is why that line failed contrast in *dark* mode too. `--dt-arrow-default` / `--dt-arrow-hover` move to `#6b7280` / `#4b5563` in light and `#9ca3af` / `#d1d5db` in dark: they paint the only indicator that a column is sortable, pinnable or filterable, which is non-text content under WCAG 1.4.11 at a 3:1 floor, and gray-300 managed 1.41:1 on a resting header. Dark-mode `--dt-primary` / `--dt-primary-hover` lift a full step to `#60a5fa` / `#93c5fd` for the same reason — the cursor ring read 2.80:1 against a hovered header — so filled buttons take `color: var(--dt-bg)` instead of white, because no blue light enough to serve as an indicator can also carry white text at 4.5:1. `--dt-success` and `--dt-syntax-string` darken to `#15803d` and light `--dt-error` to `#dc2626`, since all three are painted as text on near-white; `--dt-error-dark` / `--dt-error-darker` stay at `#dc2626` / `#b91c1c` in both themes, because they are fills carrying white `--dt-on-error` rather than text. The tertiary/secondary distinction is quieter than before in both themes; that is the cost of clearing AA at 11.2px.
+
+Two trade-offs are deliberate. Column resize and drag-to-reorder stay mouse-only and are excluded from the F2 cycle rather than being given a focus stop that does nothing on Enter — keyboard resize and reorder need designed gestures, which is a feature rather than this fix, tracked as issue #87. And an unloaded table now carries no grid semantics at all: the empty shell owns no rows, so `role="grid"` on it would be an `aria-required-children` violation, and a tab stop with nothing to navigate is noise.
 
 Three smaller ARIA corrections ride along, all in code this change was already rewriting. `aria-rowcount` now counts the filtered rows rather than the total, so a five-row result no longer announces "row 3 of 5,001". The grid picks up its semantics even when a caller sets the table name after the schema, which previously left it permanently roleless. And filtering from the header row no longer wipes the cursor — the header exists regardless of how many data rows survive.
 
+Tab stops also have to survive the table redrawing itself. The cursor rides on `aria-activedescendant`, but that only resolves while real DOM focus sits inside the grid — and the body is a pooled virtual scroller that detaches the row you are standing on at the slightest provocation. Five places in `TableBody` removed a node that could be holding focus, and none of them said anything about it: the full re-render returning every row to the pool, the scroll recycler evicting rows that left the visible range, the pooled-row replacement when a recycled row has the wrong cell count, the surplus-cell trim on a row that itself survives, and `destroy()` detaching the whole viewport subtree — which `TableContainer.render()` triggers on every schema or `visibleColumns` change. Detaching a focused node drops focus to `<body>`, so from that moment every keystroke goes to the page instead of the grid, with nothing on screen to say the keyboard layer is gone. Each of those sites now hands focus back to `.dt-grid` before the node leaves the tree. `TableContainer` had the mirror-image bug on the way back in: it restored focus after a render whenever focus had been inside the table before it, so a `Tab` that landed *outside* the table during the render's animation frame got reeled straight back — trapping by rescue rather than by `preventDefault()`. It now remembers the specific element rather than a boolean, and restores only when that element is gone from the table *and* focus has fallen to nothing.
+
+Four more ARIA corrections come out of the same pass. Unselected rows carry `aria-selected="false"` rather than nothing, because inside a `role="grid"` an absent `aria-selected` announces "not selectable at all" for rows that answer to click, ctrl-click and shift-click; the grid pairs it with `aria-multiselectable="true"`, without which those same rows announce a single-select grid. Loading placeholder rows carry `aria-busy="true"` — a placeholder is one cell against a grid advertising N columns, and padding it out to N is not an option, since cell count is exactly how the renderer tells a placeholder from a data row. The header row is mounted only once it actually owns headers, because a childless `role="row"` is a critical `aria-required-children` violation and an empty visible set is reachable both permanently, through `setColumnOrder([])`, and transiently, whenever `schema` and `visibleColumns` land as separate signal writes. And `instanceId` now always picks up a random suffix, including one you supply: two tables handed the same value used to mint identical cell ids and publish an `aria-activedescendant` that resolves document-wide to whichever grid comes first. `DataTable.instanceId` reports the resolved value, which is the one actually in the DOM.
+
 `aria-required-children` is no longer disabled in the axe suite — leaving it off is what let the original violation sit unnoticed. Two new source-level tests cover what neither jsdom nor axe can see: `tests/styles/contrast.test.ts` computes WCAG ratios straight from the token declarations, and `tests/styles/focusIndicator.test.ts` asserts that the cursor ring is re-composed against every annotation and filter tint that sets `box-shadow` on the same element at the same specificity — an omission that would silently erase the focus indicator on exactly the columns a user is most likely to inspect.
 
-Verified in a real browser against the reported setup. On a 266-column table with 1,330 header buttons, the whole `.dt-root` subtree holds eight tabbable elements — three of them the table proper, the rest filter-bar and gutter chrome — so a Tab from the control before the table reaches the one after it in eight presses, Shift+Tab retraces exactly, and the walk passes *through* the grid rather than around it. The issue's own probe, `document.querySelector('.dt-root :focus')`, resolves to `.dt-grid` with a visible 2px ring where it used to stay `null`. `axe.run('.dt-root')` with every rule enabled — contrast included, which jsdom cannot compute — reports zero violations in both themes, with `aria-required-children`, `scrollable-region-focusable` and `color-contrast` all landing in the passes bucket, including with the hovered-header and selected-hovered-row backgrounds forced.
+Verified in a real browser against the reported setup. On a 266-column table with 1,330 header buttons, the whole `.dt-root` subtree holds five tabbable elements — the filter bar, `.dt-grid`, the two scroll regions and the hidden-columns gutter — and the same five at 4 columns, with six of eight columns hidden, and with three filters applied. A Tab from the control before the table reaches the one after it in six presses — five to walk the stops, one to leave — Shift+Tab retraces in the same six, and the walk passes *through* the grid rather than around it. Neither held before: forward Tab never escaped at all, looping on `.dt-header-scroll` for as long as it was pressed, and the census stood at six elements at rest, thirteen after hiding six of eight columns, ten with three filters applied. The issue's own probe, `document.querySelector('.dt-root :focus')`, resolves to `.dt-grid` with a visible 2px ring where it used to stay `null`. `axe.run('.dt-root')` with every rule enabled — contrast included, which jsdom cannot compute — reports zero violations in both themes, with `aria-required-children`, `scrollable-region-focusable` and `color-contrast` all landing in the passes bucket, including with the hovered-header and selected-hovered-row backgrounds forced.
 
-Symptom this fixes: pressing Tab with focus just before the table never got past it — focus vanished, no control inside ever showed a ring, and neither Tab nor Shift+Tab could get back out without reloading the page.
+Symptom this fixes: pressing Tab with focus just before the table never got past it — focus vanished, no control inside ever showed a ring, and neither Tab nor Shift+Tab could get back out without reloading the page. Tabbing past the table now takes six presses in either direction — five stops inside it, one to step off — and stays six as columns are hidden and filters pile up.

.github/workflows/ci.yml

@@ -84,6 +84,38 @@ jobs:
       - name: Build library
         run: npm run build
 
+  browser:
+    name: Browser accessibility suite
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+
+      - name: Setup Node.js
+        uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
+        with:
+          node-version: '20'
+          cache: 'npm'
+
+      - name: Install dependencies
+        run: npm ci
+
+      # Chromium only. These specs guard focus order, real paint contrast,
+      # and scrollable-region detection — none of which jsdom can observe,
+      # and none of which need a second engine to be meaningful.
+      - name: Install Chromium
+        run: npx playwright install --with-deps chromium
+
+      - name: Browser accessibility tests
+        run: npm run test:browser
+
+      - name: Upload Playwright report
+        if: failure()
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        with:
+          name: playwright-report
+          path: playwright-report/
+          retention-days: 7
+
   size:
     name: Bundle size budgets
     runs-on: ubuntu-latest

.gitignore

@@ -27,6 +27,12 @@ npm-debug.log*
 # Test coverage
 coverage/
 
+# Playwright
+test-results/
+playwright-report/
+blob-report/
+.playwright/
+
 # Cache
 .cache/
 .parcel-cache/

.prettierignore

@@ -2,6 +2,8 @@ node_modules
 dist
 demo-dist
 coverage
+playwright-report
+test-results
 docs/api
 package-lock.json
 tests/fixtures/datasets

.size-limit.cjs

@@ -21,9 +21,9 @@
  * inlined the shared ModalHost code into each modal consumer and shifted some
  * helper code into VisualizationRegistry):
  *   root entry · ESM                7.65 kB   →   8.1 kB cap (5.9 %)
- *   advanced entry · ESM            2.42 kB   →   2.6 kB cap (7.4 %)
- *   stylesheet                     16.94 kB   →  17.8 kB cap (5.1 %)
- *   lazy ExportDialog chunk        67.58 kB   →  71 kB   cap (5.1 %)
+ *   advanced entry · ESM            2.41 kB   →   2.6 kB cap (7.9 %)
+ *   stylesheet                     18.66 kB   →  19.6 kB cap (5.0 %)
+ *   lazy ExportDialog chunk        69.91 kB   →  74 kB   cap (5.8 %)
  *   lazy SQLFilterModal chunk       2.49 kB   →   2.6 kB cap (4.4 %)
  *   lazy DerivedColumnModal         3.60 kB   →   3.8 kB cap (5.5 %)
  *   lazy DerivedColumnEditPanel     2.97 kB   →   3.1 kB cap (4.5 %)
@@ -34,10 +34,22 @@
  * shared ModalHost helpers into each modal consumer. The per-modal caps above
  * already cover the added bytes.
  *
- * The stylesheet baseline moved 16.22 → 16.94 kB with the ARIA-grid keyboard
- * fix (issue #84): a `.dt-grid` layout block, a `:focus-visible` ring for it,
- * and the header-row cursor ring. Its cap moved with it to keep the usual
- * ~5 % headroom rather than leaving the next CSS change to trip the gate.
+ * Stylesheet history. The line above previously read 16.94 kB; that figure was
+ * never measured — the real size at that commit was 17.11 kB, so the gate had
+ * ~0.7 kB less headroom than it advertised. The accessibility follow-up to
+ * issue #84 then took it 17.11 → 18.66 kB: darker contrast tokens, the
+ * roving-tabindex toolbars, and clipping the two scrollable regions that had
+ * no focusable content.
+ *
+ * Worth knowing before the next CSS budget conversation: essentially all of
+ * that 1.55 kB is *comment prose*, not rules. `buildStylesPlugin` in
+ * `vite.config.ts` concatenates `src/styles/*.css` verbatim with no
+ * minification, so every explanatory comment ships to users. Stripped of
+ * comments the same stylesheet is 7.98 kB brotli — 57 % smaller, and the
+ * rule payload actually shrank by ~30 bytes across this change. Minifying in
+ * that concat step would make this budget a measure of CSS rather than of
+ * documentation; it is deliberately left alone here because it changes
+ * published output.
  *
  * Phase-9 baseline pre-refactor (kept for diff context):
  *   root entry · ESM        7.33 kB   →   7.7 kB cap
@@ -58,12 +70,12 @@ module.exports = [
   {
     name: 'stylesheet (dist/data-table.css)',
     path: 'dist/data-table.css',
-    limit: '17.8 kB',
+    limit: '19.6 kB',
   },
   {
     name: 'lazy ExportDialog chunk · ESM',
     path: 'dist/VisualizationRegistry-*.js',
-    limit: '71 kB',
+    limit: '74 kB',
   },
   {
     name: 'lazy SQLFilterModal chunk · ESM',