feat: grid accessibility content on demand, playwright-grid testing package by mofojed · Pull Request #2742 · deephaven/web-client-ui

mofojed · 2026-08-24T19:33:24Z

The grid draws to a canvas, so there were no per-cell elements for screen readers or browser automation to inspect, and the fallback content was a single line telling the user to update their browser.
Grid now renders a summary of its size and selection there, plus a button that describes the viewport as a table of the visible column headers and cells. Walking the viewport is too expensive to do every frame, so the description is generated on request and discarded as soon as the grid scrolls. Each cell carries its position on screen, which lets external tooling locate it without knowing the grid's layout maths.
- Did it this way rather than exposing an object on the Grid element - for a testing library like Playwright, which is run in a separate process, it was not able to interact with that object directly; instead it had to do evaluate calls in the context of the UI for that to work. By doing it this way, it is also actually useful as an accessibility guide, as screen readers can be instructed to show all the details
Also adds @deephaven/playwright-grid, which wraps that description in helpers so e2e tests can address cells by column name rather than by pixel offset, and covers both with e2e tests that verify grid contents instead of comparing screenshots.
Added table-content.spec.ts for testing with the playwright-grid package and copying many of the tests from table.spec.ts; also added table-a11y.spec.ts to ensure the accessibility layer makes sense

The grid draws to a canvas, so there were no per-cell elements for screen readers or browser automation to inspect, and the fallback content was a single line telling the user to update their browser. Grid now renders a summary of its size and selection there, plus a button that describes the viewport as a table of the visible column headers and cells. Walking the viewport is too expensive to do every frame, so the description is generated on request and discarded as soon as the grid scrolls. Each cell carries its position on screen, which lets external tooling locate it without knowing the grid's layout maths. Also adds @deephaven/playwright-grid, which wraps that description in helpers so e2e tests can address cells by column name rather than by pixel offset, and covers both with e2e tests that verify grid contents instead of comparing screenshots.

codecov · 2026-08-24T19:36:19Z

Codecov Report

❌ Patch coverage is 96.81529% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.24%. Comparing base (5875e7f) to head (09bb94c).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/grid/src/GridA11yUtils.ts	95.96%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2742      +/-   ##
==========================================
+ Coverage   52.08%   52.24%   +0.16%     
==========================================
  Files         797      810      +13     
  Lines       45928    46485     +557     
  Branches    11942    11901      -41     
==========================================
+ Hits        23922    24287     +365     
- Misses      21986    22178     +192     
  Partials       20       20

Flag	Coverage Δ
unit	`52.24% <96.81%> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Adds on-demand accessibility content for canvas grids and a Playwright helper package for content-based grid testing.

Changes:

Renders grid summaries and viewport snapshots as accessible tables.
Adds Playwright helpers for querying and interacting with grid cells.
Adds unit/E2E coverage, documentation, and workspace wiring.

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`tsconfig.json`	References the new package.
`tests/table-contents.spec.ts`	Adds content-based grid E2E tests.
`tests/table-a11y.spec.ts`	Tests the accessibility contract.
`packages/playwright-grid/tsconfig.json`	Configures package compilation.
`packages/playwright-grid/src/index.ts`	Exports grid helpers.
`packages/playwright-grid/src/gridHelpers.ts`	Implements Playwright grid APIs.
`packages/playwright-grid/README.md`	Documents helper usage.
`packages/playwright-grid/package.json`	Defines the new package.
`packages/grid/src/index.ts`	Exports accessibility APIs.
`packages/grid/src/GridA11yUtils.ts`	Builds summaries and snapshots.
`packages/grid/src/GridA11yUtils.test.ts`	Tests snapshot generation.
`packages/grid/src/GridA11yFallback.tsx`	Renders accessible fallback markup.
`packages/grid/src/GridA11yFallback.test.tsx`	Tests fallback rendering.
`packages/grid/src/Grid.tsx`	Integrates snapshots into the grid.
`packages/grid/src/Grid.test.tsx`	Tests grid-level integration.
`packages/grid/docs/sidebar.json`	Adds accessibility documentation.
`packages/grid/docs/accessibility.md`	Documents the accessibility contract.
`package.json`	Registers the workspace dependency.
`package-lock.json`	Updates workspace lock data.

Suppressed comments (2)

packages/grid/src/GridA11yUtils.ts:230

allRows likewise appends floating rows to the scrollable rows. A top/bottom floating row can therefore be duplicated when it overlaps the current range, and floating rows are not emitted in their on-screen order; the resulting table can contain duplicate row keys/indexes and the helper's cell locator can match multiple elements. Construct a deduplicated row list ordered by its rendered Y position and exclude rows obscured by floating regions.

  metrics.allRows.forEach(row => {

packages/grid/src/GridA11yFallback.tsx:77

Cells also need their absolute grid column position. Renumbering every viewport from 1 conflicts with the full aria-colcount and misrepresents horizontally virtualized/hidden columns to assistive technology.

                      aria-colindex={columnIndex + 1}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

dsmmcken

We should ensure the default state displays no ticking data updates, featuring only a button that activates the desired behaviour without the description triggering dom changes. The only automatic dom change I think we should do is a single one for the initial render complete, so that playwright can easily watch for that for easy screenshots.

Copilot

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.

Suppressed comments (9)

Previously missed (3) — in code that hasn't changed since the last review.

packages/playwright-grid/src/gridHelpers.ts:221

This readiness condition can never pass for a valid zero-row grid because its snapshot has no data-grid-row elements. waitForGrid will poll until timeout even though the empty grid is fully rendered. Wait for the generated snapshot itself rather than requiring a data cell.

      const snapshot = await refreshSnapshot(canvas);
      return snapshot.locator(`[${ATTRIBUTES.row}]`).count();
    })
    .toBeGreaterThan(0);

packages/playwright-grid/src/gridHelpers.ts:329

A partially visible first row or column can have a negative rect.x/rect.y because metrics subtract the scroll offset. If more than half the cell is clipped, this computed center lies outside the canvas, so the returned page location does not identify a visible point in the requested cell. Compute the center of the rect's intersection with the canvas bounds.

This issue also appears on line 347 of the same file.

  return {
    x: box.x + rect.x + rect.width / 2,
    y: box.y + rect.y + rect.height / 2,
  };

packages/playwright-grid/src/gridHelpers.ts:389

Column headers can also be partially clipped horizontally, making this full-rect center fall outside the canvas. In that state a header returned by the snapshot is “visible” but clickColumnHeader cannot reliably click it. Clamp to the visible intersection before choosing the click point.

  await canvas.click({
    ...options,
    position: { x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 },
  });

packages/grid/src/Grid.tsx:624

The snapshot is invalidated only for scroll offsets, so it remains exposed after other changes that make it stale. For example, selecting a cell after generating a snapshot updates the summary but leaves the snapshot's status text with the old selection; resize, model updates, and column moves can likewise leave stale cells and rectangles. Invalidate an existing snapshot on every render-affecting update while exempting the state transition that creates the snapshot.

      a11ySnapshot != null &&
      (prevTop !== currentTop ||
        prevLeft !== currentLeft ||
        prevTopOffset !== currentTopOffset ||
        prevLeftOffset !== currentLeftOffset)

packages/grid/src/GridA11yUtils.ts:193

Selection ranges are not guaranteed to be disjoint—the IrisGrid selection callback explicitly observes duplicate ranges—so summing them directly can announce one selected row or cell twice. Consolidate the ranges before formatting the count.

  return selectedRanges.length > 0
    ? `${size} ${getSelectionDescription(selectedRanges)}`
    : size;

packages/grid/src/GridA11yUtils.ts:217

visibleColumns contains the scrolling section only; right-floating columns are held in floatingColumns/allColumns and are painted on screen, but never enter this snapshot. This makes pinned columns inaccessible and prevents the Playwright helpers from addressing them. Include all displayed columns, de-duplicate them, and order them by their on-screen x coordinate.

  const snapshotColumns = metrics.visibleColumns.filter(
    column => (metrics.allColumnWidths.get(column) ?? 0) > 0
  );

packages/grid/src/GridA11yUtils.ts:234

visibleRows excludes bottom-floating rows even though GridRenderer paints those rows on screen. Consequently the accessible table and Playwright API cannot inspect pinned bottom rows. Include all displayed rows, de-duplicate them, and order them by their on-screen y coordinate.

  metrics.visibleRows.forEach(row => {
    if ((metrics.allRowHeights.get(row) ?? 0) <= 0) {
      return;

packages/grid/src/GridA11yUtils.ts:258

topVisible and bottomVisible denote fully visible rows, while the snapshot includes partially visible rows from metrics.visibleRows. After a fractional scroll, the status therefore announces a different range from the rows in its table; an empty grid also becomes “Showing rows 1 to 1.” Derive this text from the rows actually emitted.

  const { topVisible, bottomVisible } = metrics;
  const headers = columns.map(({ text }) => text).filter(text => text !== '');
  const visibleRows = `Showing rows ${topVisible + 1} to ${bottomVisible + 1}`;

packages/playwright-grid/src/gridHelpers.ts:350

For a partially clipped row or column, the full cell center can lie outside the canvas. Playwright then cannot click the requested cell (or can target an element outside the grid) even though the helper reports that cell as visible. Use the center of the cell rect intersected with the canvas bounds.

  await canvas.click({
    ...options,
    position: { x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 },
  });

@@ -0,0 +1,428 @@
+import { expect, type Locator } from '@playwright/test';
+import type { GridA11yAttributes, GridA11yRect } from '@deephaven/grid';


+  selectedRanges: readonly GridRange[] = []
+): string {
+  const { rowCount, columnCount } = model;
+  const size = `Grid with ${rowCount} rows and ${columnCount} columns.`;


The fallback component now owns the held snapshot, the revision counter, generating the snapshot, and discarding it once the grid scrolls. Grid just renders it with the model, a metrics getter, the viewport, and the selection. The revision attribute moves from the canvas to the fallback root, as Grid no longer holds it. Updated the playwright-grid helpers and docs to match. Also fixes the metrics helper in GridA11yUtils.test, which set allColumns/allRows instead of visibleColumns/visibleRows.

Copilot

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

packages/playwright-grid/package.json:28

The emitted public declaration for getCellRect references GridA11yRect from @deephaven/grid, but that package is only a dev dependency. An external consumer that does not independently install/hoist grid cannot resolve the helper's typings. Either declare @deephaven/grid as a compatible peer dependency (and update the lockfile), or expose a local structural rect type so the published declarations are self-contained.

    "@deephaven/grid": "file:../grid",

packages/grid/src/GridA11yUtils.ts:217

Frozen columns are excluded because visibleColumns contains only the scrollable viewport; GridMetrics keeps columns pinned on screen in floatingColumns/allColumns (and Iris uses this for user/layout-hint frozen columns). As a result, the accessibility table and @deephaven/playwright-grid cannot see or click a frozen column even though it is painted. Build the snapshot from all painted columns, de-duplicate any dragged column, and order them by allColumnXs.

  const snapshotColumns = metrics.visibleColumns.filter(
    column => (metrics.allColumnWidths.get(column) ?? 0) > 0
  );

packages/grid/src/GridA11yUtils.ts:258

For an empty model, the metrics use topVisible = bottomVisible = 0, so this produces the contradictory announcement “Grid with 0 rows… Showing rows 1 to 1.” Handle an empty snapshot explicitly so assistive technology reports that no rows are shown.

  const { topVisible, bottomVisible } = metrics;
  const headers = columns.map(({ text }) => text).filter(text => text !== '');
  const visibleRows = `Showing rows ${topVisible + 1} to ${bottomVisible + 1}`;

+  await expect
+    .poll(async () => {
+      const snapshot = await refreshSnapshot(canvas);
+      return snapshot.locator(`[${ATTRIBUTES.row}]`).count();
+    })
+    .toBeGreaterThan(0);


Copilot

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:217

visibleColumns contains only the scrollable viewport; pinned/floating columns are represented in allColumns. After horizontal scrolling, pinned columns remain painted but disappear from this accessibility snapshot and cannot be addressed by the Playwright helpers. Include all rendered columns, deduplicated and ordered by their X coordinate.

This issue also appears on line 256 of the same file.

  const snapshotColumns = metrics.visibleColumns.filter(
    column => (metrics.allColumnWidths.get(column) ?? 0) > 0
  );

packages/playwright-grid/src/gridHelpers.ts:223

A valid rendered grid may have zero rows, but this condition can never succeed for it, so waitForGrid always times out on empty tables. Poll for the generated snapshot itself (or another render-ready marker) rather than requiring at least one cell.

      const snapshot = await refreshSnapshot(canvas);
      return snapshot.locator(`[${ATTRIBUTES.row}]`).count();
    })
    .toBeGreaterThan(0);

packages/grid/src/GridA11yFallback.tsx:63

refreshSnapshot records data-grid-a11y-revision, dispatches this click, and waits for the attribute to increase. This handler only toggles showSnapshot; the revision prop is unchanged, so every helper and the accessibility E2E helper time out. Repeated refreshes would also hide the snapshot. Make this action always regenerate the snapshot and increment a snapshot revision.

        onClick={() => setShowSnapshot(prevShowSnapshot => !prevShowSnapshot)}

packages/grid/src/GridA11yUtils.ts:232

visibleRows excludes floating rows once the scrollable viewport moves (and always excludes floating bottom rows), although those rows remain painted. As a result, the accessibility table omits visible pinned rows and their cells. Build rows from all rendered row metrics, deduplicated and ordered by Y position.

  metrics.visibleRows.forEach(row => {

packages/grid/src/GridA11yUtils.ts:258

This status range uses the first/last fully visible rows, while the snapshot above includes every visibleRows entry, including partially visible edge rows. After a partial scroll, assistive technology is told a different row range from the rows actually present. Derive the announced range from the generated snapshot rows, or filter the table to the announced range.

  const { topVisible, bottomVisible } = metrics;
  const headers = columns.map(({ text }) => text).filter(text => text !== '');
  const visibleRows = `Showing rows ${topVisible + 1} to ${bottomVisible + 1}`;

@@ -0,0 +1,430 @@
+import { expect, type Locator } from '@playwright/test';
+import type { GridA11yAttributes, GridA11yRect } from '@deephaven/grid';


Copilot

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated 4 comments.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:217

Frozen/floating columns are painted on screen but omitted here because visibleColumns excludes floatingColumns (GridMetrics.ts:132-150). As a result, screen readers and the Playwright helpers cannot inspect or click pinned columns. Build the snapshot from both viewport and floating columns, ordered by their on-screen coordinates.

  const snapshotColumns = metrics.visibleColumns.filter(
    column => (metrics.allColumnWidths.get(column) ?? 0) > 0
  );

packages/playwright-grid/src/gridHelpers.ts:221

This readiness condition can never succeed for a valid empty grid because its snapshot has no elements carrying data-grid-row. waitForGrid will time out even though the empty grid and its headers are fully rendered; poll for the snapshot root instead.

      return snapshot.locator(`[${ATTRIBUTES.row}]`).count();

packages/grid/src/GridA11yUtils.ts:232

Frozen/floating rows are rendered by GridRenderer.drawFloatingRows, but iterating only visibleRows leaves them out of the accessibility table. Include floatingRows as well, in on-screen order, so the snapshot actually describes every displayed row.

  metrics.visibleRows.forEach(row => {

Copilot

Pull request overview

Copilot reviewed 17 out of 19 changed files in this pull request and generated 4 comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:216

visibleColumns excludes floating/frozen columns. Once the grid scrolls horizontally, frozen columns remain painted on screen but disappear from this snapshot, so screen readers and playwright-grid cannot inspect or click them. Include metrics.floatingColumns as well, deduplicate overlaps, and preserve on-screen x order.

This issue also appears on line 258 of the same file.

  const snapshotColumns = metrics.visibleColumns.filter(
    column => (metrics.allColumnWidths.get(column) ?? 0) > 0

packages/grid/src/GridA11yUtils.ts:233

visibleRows excludes floating rows such as IrisGrid totals rows. After scrolling, those rows are still painted at the top or bottom but are absent from the accessibility table. Include metrics.floatingRows, deduplicate overlaps, and order the resulting rows by their on-screen y coordinate.

  metrics.visibleRows.forEach(row => {
    if ((metrics.allRowHeights.get(row) ?? 0) <= 0) {

packages/grid/src/GridA11yUtils.ts:258

For a valid empty grid, topVisible and bottomVisible are both 0 while rows is empty, so the status incorrectly announces “Showing rows 1 to 1.” Branch on the generated row list so zero-row tables are announced accurately.

  const visibleRows = `Showing rows ${topVisible + 1} to ${bottomVisible + 1}`;

+        type="button"
+        tabIndex={-1}
+        aria-pressed={showSnapshot}
+        onClick={() => setShowSnapshot(prevShowSnapshot => !prevShowSnapshot)}


+  await expect
+    .poll(async () => {
+      const snapshot = await showSnapshot(canvas);
+      return snapshot.locator(`[${ATTRIBUTES.row}]`).count();
+    })
+    .toBeGreaterThan(0);


Copilot

🟡 Changes recommended

Snapshot geometry, floating-row coverage, empty-grid handling, and off-state rendering introduce correctness and performance issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

packages/grid/src/GridA11yFallback.tsx:39

The fallback subscribes even while the snapshot is off, so every canvas draw calls setRenderState with a new object and forces a React render. Grid drawing is otherwise an imperative hot path, and mouse movement/scrolling can trigger it every frame; this adds the per-frame React work that the on-demand design is meant to avoid. Subscribe only while showSnapshot is true and clear the cached state when it is turned off.

  useEffect(() => registerDrawListener(setRenderState), [registerDrawListener]);

packages/grid/src/GridA11yUtils.ts:123

These bounds are clipped only at the top/left. The metric calculator intentionally includes a final row/column that can extend past the canvas, and floating rows/columns or scrollbars can cover additional portions; the snapshot therefore advertises centers outside the canvas or behind an overlay, causing clickCell to fail or hit a different cell. Clip all four edges to the actual interactable viewport, accounting for floating sections and scrollbar sizes.

  const clippedX = Math.max(x, 0);
  const clippedY = Math.max(y, 0);
  const clippedWidth = width - (clippedX - x);
  const clippedHeight = height - (clippedY - y);

packages/grid/src/GridA11yUtils.ts:159

Column-header rectangles are returned at their full metric width without viewport clipping. A partially visible first or last header can therefore have its reported center outside the canvas (or under a pinned column), and clickColumnHeader will fail or click the wrong target. Apply the same interactable horizontal clipping used for cells, including floating columns and the vertical scrollbar.

  return {
    x: metrics.gridX + x,
    // The bottom level header sits directly above the grid content
    y: metrics.gridY - columnHeaderHeight,
    width,
    height: columnHeaderHeight,
  };

packages/grid/src/GridA11yUtils.ts:261

Only visibleRows are serialized. After scrolling, pinned top/bottom rows remain painted via metrics.floatingRows and GridRenderer.drawFloatingRows, but disappear from this snapshot, so assistive technology and Playwright cannot address visible pinned cells. Build the snapshot from a deduplicated, Y-ordered union of visible and floating rows, while preserving which column intersections are actually drawn.

  metrics.visibleRows.forEach(row => {

Files reviewed: 17/19 changed files
Comments generated: 2
Review effort level: Balanced

Copilot

🔵 Needs a closer look

Snapshot geometry can target off-canvas content, floating rows are omitted, and disabled snapshots still cause React work on every draw.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:157

These raw header bounds do not match what is painted. GridRenderer.drawColumnHeaders clips scrollable headers between the floating-left and floating-right panes, while a partially scrolled header can also begin before the content area; exporting its full rect can make clickColumnHeader click a pinned header or outside the canvas. Apply the renderer's pane-specific horizontal clipping here and omit a header when no clickable area remains.

packages/grid/src/GridA11yUtils.ts:261

This omits every floating row even though GridRenderer.drawFloatingRows paints them. IrisGrid uses those rows for top/bottom totals, so visible totals disappear from the accessible table and cannot be queried by the Playwright helpers. Merge and deduplicate metrics.floatingRows with metrics.visibleRows, then sort by allRowYs, mirroring the floating-column handling above.

  metrics.visibleRows.forEach(row => {

packages/grid/src/GridA11yFallback.tsx:39

This subscribes setRenderState even while the snapshot is disabled, so every canvas draw schedules a React render of the fallback. Grid redraws on animation frames while scrolling, and the surrounding draw path is explicitly performance-sensitive; defer the subscription until the on-demand snapshot is enabled, relying on registerDrawListener to replay the latest frame when it is turned on.

  useEffect(() => registerDrawListener(setRenderState), [registerDrawListener]);

packages/grid/src/GridA11yUtils.ts:123

The grid's visibleRows/visibleColumns metric arrays deliberately contain render overscan, but this clips only negative starts. Consequently trailing cells can extend beyond—or lie entirely outside—the canvas (for example, row generation runs to height before gridY is added), so the accessibility table exposes non-visible cells and clickCell can calculate an out-of-canvas position. Clip the right/bottom edges to the actual drawable pane bounds as well, including scrollbar and floating-pane boundaries, and return null for zero-area cells.

  const clippedX = Math.max(x, 0);
  const clippedY = Math.max(y, 0);
  const clippedWidth = width - (clippedX - x);
  const clippedHeight = height - (clippedY - y);

Files reviewed: 19/21 changed files
Comments generated: 0 new
Review effort level: Balanced

Include floating rows in the snapshot alongside floating columns, and clip every cell and header rect against the canvas, scroll bars, and frozen panes so reported bounds always land on the item they describe. Rows and rects with no interactable area left are omitted. Also only register the draw listener while the snapshot is shown.

Copilot

🟡 Changes recommended

Snapshot activation alters grid focus, and announced row ranges can omit visible pinned or partial rows.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/grid/src/GridA11yFallback.tsx:64

This click bubbles to the parent <canvas>'s handleClick, which always focuses the canvas and can invoke registered grid click handlers. Because the Playwright helpers dispatch this button click merely to read the snapshot, the first read unexpectedly changes application focus. Stop propagation in this handler before toggling the snapshot.

        onClick={() => setShowSnapshot(prevShowSnapshot => !prevShowSnapshot)}

Files reviewed: 19/21 changed files
Comments generated: 1
Review effort level: Balanced

Group the visible rows into consecutive ranges instead of spanning from topVisible to bottomVisible, so hidden rows and rows scrolled underneath floating rows are not claimed as visible.

Copilot

🔵 Needs a closer look

Header geometry and text-based column resolution fail for valid grid configurations.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:219

The header rectangle assumes the bottom header row sits immediately above gridY. In IrisGridMetricCalculator.getGridY, an expanded or collapsed filter bar is added below the headers, so this rectangle lands in that filter bar; clickColumnHeader will click a filter control instead of the sortable header whenever filters are shown. Derive the header position from columnHeaderMaxDepth so optional bars between the headers and cells do not shift it.
packages/playwright-grid/src/gridHelpers.ts:59
This only escapes quotes and backslashes, but CSS quoted strings cannot contain a literal newline. Iris grid display names are explicitly allowed to be any string, so a header containing a line break makes snapshot.locator(...) fail to parse instead of resolving the column. Escape CSS control characters as code points as well.
packages/playwright-grid/src/gridHelpers.ts:120
Header text is not a unique column identifier: DisplayColumn.displayName is explicitly allowed to be non-unique, but this silently resolves duplicate matches with first(). A test can therefore click/assert against the wrong column. Either reject multiple matches with an error directing callers to the numeric visible index, or expose a unique column identifier in the snapshot.

Files reviewed: 19/21 changed files
Comments generated: 0 new
Review effort level: Balanced

jnumainville

Just one thing, maybe.

jnumainville · 2026-09-04T14:40:35Z

+  }, [registerDrawListener, showSnapshot]);
+
+  const snapshot = useMemo(() => {
+    if (renderState == null) {


renderState is initialized as {}. Is it ever possible to hit that? Not sure, this was flagged by my local AI review as a possible edge case.
A bit more detail:
If the describe button is toggled on before the grid has ever drawn (e.g. a zero-size container where drawCanvas never completes past assertNotNull(metrics)), renderState is {} (not null), so createGridA11ySnapshot(undefined, undefined, undefined) runs and throws on const { rowCount, columnCount } = model

Fixed to make sure the renderState of {} never comes through.

- Casting the `{}` was a little misleading

Copilot

🔵 Needs a closer look

Cell geometry can overlap row footers, and name-based helpers mishandle valid multiline or duplicate headers.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:172

The Iris theme paints a 90px row footer over the right edge of the body (packages/iris-grid/src/IrisGridTheme.ts:155), but this clipping boundary excludes only the scrollbar. A partially visible final cell can therefore advertise a rectangle extending under the footer, causing clickCell to click the footer instead of the painted cell. Exclude rowFooterWidth from the body boundary; column-header clipping should remain unchanged because the footer starts below the headers.
packages/playwright-grid/src/gridHelpers.ts:59
Valid Iris column displayName values are unrestricted, but this escapes only backslashes and quotes. A header such as Bid\nPrice produces an invalid CSS attribute selector in resolveColumn, so every name-based helper throws rather than locating the column. Escape CSS string line terminators as well, or avoid interpolating the attribute value into a selector.
packages/playwright-grid/src/gridHelpers.ts:120
Rendered Iris headers prefer displayName, which is explicitly allowed to be non-unique (packages/iris-grid/src/IrisGridModel.ts:28-34). Calling .first() silently resolves duplicate labels to the wrong column, so a name-based assertion or click can pass against an unintended column. Detect ambiguous matches and direct callers to the numeric visible index instead.

Files reviewed: 19/21 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

🔵 Needs a closer look

Selection counts can be inaccurate, duplicate header names resolve ambiguously, and the implemented snapshot lifecycle conflicts with the PR description.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

packages/grid/src/GridA11yUtils.ts:228

GridRange.cellCount sums each range independently, but the grid can retain partially overlapping ranges after Ctrl-drag because GridRange.consolidate only merges ranges with matching row or column extents. For example, overlapping 2×2 ranges are announced as 8 selected cells although their union contains 7. Count the union of selected cells (or normalize ranges to a disjoint representation) before announcing it.
packages/playwright-grid/src/gridHelpers.ts:120
This silently chooses the first matching header, but IrisGrid renders displayName, which explicitly need not be unique (packages/iris-grid/src/IrisGridModel.ts:28-34). A string lookup can therefore read or click the wrong column. Detect multiple matches and require the caller to use the visible numeric index instead.

packages/grid/src/GridA11yUtils.ts:267

The summary is grammatically incorrect when the grid has exactly one row or column (for example, “Grid with 1 rows and 1 columns”), which is user-facing screen-reader text. Pluralize each noun independently.

  const size = `Grid with ${rowCount} rows and ${columnCount} columns.`;

packages/grid/src/GridA11yFallback.tsx:53

This implements a persistent snapshot that is absent until requested and then regenerated after every draw. The PR description instead promises an always-present size/selection summary plus a one-shot viewport description discarded on scroll. Please align the implementation with that stated contract, or update the PR description to describe the persistent on-demand behavior.

  const snapshot = useMemo(() => {
    if (renderState == null) {
      return null;
    }
    const { model, metrics, selectedRanges } = renderState;

Files reviewed: 19/21 changed files
Comments generated: 0 new
Review effort level: Balanced

mofojed requested a review from dsmmcken August 24, 2026 19:33

mofojed self-assigned this Aug 24, 2026

Copilot AI balanced review requested due to automatic review settings August 24, 2026 19:33

Copilot started reviewing on behalf of mofojed August 24, 2026 19:33

View session

Copilot AI reviewed Aug 24, 2026

View reviewed changes

dsmmcken requested changes Aug 24, 2026

View reviewed changes

mofojed changed the title ~~feat: grid accessbility content on demand, playwright-grid testing package~~ feat: grid accessability content on demand, playwright-grid testing package Aug 25, 2026

mofojed changed the title ~~feat: grid accessability content on demand, playwright-grid testing package~~ feat: grid accessibility content on demand, playwright-grid testing package Aug 25, 2026

Fix based on copilot review

c7b8d22

Copilot AI review requested due to automatic review settings August 25, 2026 15:49

Copilot started reviewing on behalf of mofojed August 25, 2026 15:49

View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 25, 2026 17:09

Copilot started reviewing on behalf of mofojed August 25, 2026 17:10

View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Change it to be a toggle button instead

66b7170

Copilot AI review requested due to automatic review settings August 25, 2026 18:04

Copilot started reviewing on behalf of mofojed August 25, 2026 18:04

View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Clean up tests

a1bbc3e

Copilot AI review requested due to automatic review settings August 25, 2026 20:36

Copilot started reviewing on behalf of mofojed August 25, 2026 20:36

View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Comment thread packages/grid/src/Grid.tsx Outdated

Comment thread packages/grid/src/GridA11yFallback.tsx

Comment thread packages/grid/src/GridA11yFallback.tsx Outdated

Comment thread packages/playwright-grid/package.json

Update docs and playwright helpers for the snapshot toggle

851be7d

Copilot AI review requested due to automatic review settings August 26, 2026 18:33

Copilot started reviewing on behalf of mofojed August 26, 2026 18:34

View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

Copilot started reviewing on behalf of mofojed September 2, 2026 15:16

View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread packages/grid/src/GridA11yUtils.ts Outdated

Comment thread packages/playwright-grid/src/gridHelpers.ts

test: describe a grid with no rows

80024dc

Copilot AI review requested due to automatic review settings September 2, 2026 18:32

Copilot started reviewing on behalf of mofojed September 2, 2026 18:32

View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

dsmmcken previously approved these changes Sep 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 3, 2026 19:15

mofojed dismissed dsmmcken’s stale review via e061523 September 3, 2026 19:15

Copilot started reviewing on behalf of mofojed September 3, 2026 19:15

View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread packages/grid/src/GridA11yUtils.ts Outdated

fix: describe only the rows the grid actually paints

38eaa3c

Group the visible rows into consecutive ranges instead of spanning from topVisible to bottomVisible, so hidden rows and rows scrolled underneath floating rows are not claimed as visible.

Copilot AI review requested due to automatic review settings September 3, 2026 20:00

Copilot started reviewing on behalf of mofojed September 3, 2026 20:01

View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

jnumainville requested changes Sep 4, 2026

View reviewed changes

Fix up renderState to be more accurate

634986f

- Casting the `{}` was a little misleading

mofojed requested review from Copilot and jnumainville September 9, 2026 15:14

Copilot started reviewing on behalf of mofojed September 9, 2026 15:14

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Fix typing

09bb94c

Copilot AI review requested due to automatic review settings September 9, 2026 18:35

Copilot started reviewing on behalf of mofojed September 9, 2026 18:35

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

jnumainville approved these changes Sep 9, 2026

View reviewed changes

mofojed merged commit 7111518 into deephaven:main Sep 9, 2026
12 checks passed

mofojed deleted the grid-a11y branch September 9, 2026 19:01

github-actions Bot locked and limited conversation to collaborators Sep 9, 2026

		@@ -0,0 +1,428 @@
		import { expect, type Locator } from '@playwright/test';
		import type { GridA11yAttributes, GridA11yRect } from '@deephaven/grid';

		@@ -0,0 +1,430 @@
		import { expect, type Locator } from '@playwright/test';
		import type { GridA11yAttributes, GridA11yRect } from '@deephaven/grid';

Conversation

mofojed commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dsmmcken left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (4)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (3)

Uh oh!

jnumainville left a comment

Choose a reason for hiding this comment

Uh oh!

jnumainville Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

mofojed commented Aug 24, 2026 •

edited

Loading

codecov Bot commented Aug 24, 2026 •

edited

Loading