Skip to content

feat: DH-21829: add invisible accessibility layer to Grid for e2e testing#2630

Open
mofojed wants to merge 10 commits intodeephaven:mainfrom
mofojed:grid-a11y-layer
Open

feat: DH-21829: add invisible accessibility layer to Grid for e2e testing#2630
mofojed wants to merge 10 commits intodeephaven:mainfrom
mofojed:grid-a11y-layer

Conversation

@mofojed
Copy link
Member

@mofojed mofojed commented Feb 28, 2026

  • Add GridAccessibilityLayer component that renders DOM elements overlaid on canvas
  • Data cells use data-testid="grid-cell-{column}-{row}"
  • Column headers use data-testid="grid-column-header-{column}-{depth}"
  • Row headers use data-testid="grid-row-header-{row}" (when rowHeaderWidth > 0)
  • Add enableAccessibilityLayer prop to Grid and IrisGrid
  • All elements have pointer-events: none to allow clicks through to canvas
  • Add e2e tests for accessibility layer functionality

- Add GridAccessibilityLayer component that renders DOM elements overlaid on canvas
- Data cells use data-testid="grid-cell-{column}-{row}"
- Column headers use data-testid="grid-column-header-{column}-{depth}"
- Row headers use data-testid="grid-row-header-{row}" (when rowHeaderWidth > 0)
- Add enableAccessibilityLayer prop to Grid and IrisGrid
- All elements have pointer-events: none to allow clicks through to canvas
- Add e2e tests for accessibility layer functionality
@mofojed mofojed requested a review from dsmmcken February 28, 2026 02:19
@mofojed mofojed self-assigned this Feb 28, 2026
- Should write a proper utility script for this
- Works by just calling .click() on that cell
- Put the accessibility layer behind the canvas so that it doesn't take any pointer events from real clicking
@mofojed mofojed changed the title feat: add invisible accessibility layer to Grid for e2e testing feat: DH-21829: add invisible accessibility layer to Grid for e2e testing Mar 2, 2026
@dsmmcken
Copy link
Contributor

dsmmcken commented Mar 2, 2026

My 3 concerns:

  1. https://www.w3.org/WAI/ARIA/apg/patterns/grid/ should come as close as possible to adhering to this ARIA spec, I see you already correctly have role=grid.
  2. If it's on by default it should thoroughly tested to check for performance regressions. Check against main fully screen grids too, multiple small grids. Fast updating raw grids.
  3. Should it be nested INSIDE the canvas instead of adjacent. I don't know if there's a benefit one way or the other, just asking.

mofojed added 4 commits March 2, 2026 17:32
Add Playwright tests that measure FPS during grid scrolling operations
with and without the accessibility layer enabled.

Performance Comparison Results (accessibility layer ON vs OFF):
- WITH Layer:    60.2 FPS, 16.61ms avg frame time, 0 dropped frames
- WITHOUT Layer: 60.1 FPS, 16.64ms avg frame time, 0 dropped frames
- FPS difference: -0.13 fps (-0.21%)
- Frame time overhead: -0.036ms

Conclusion: Accessibility layer has NEGLIGIBLE performance impact (<1%).

Test suite includes:
- simple_table scroll performance
- all_types table scroll performance
- Rapid scroll test
- A/B comparison: accessibility layer ON vs OFF
- Sustained scrolling stress test (3 seconds)
- Combined horizontal + vertical scroll test
@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 93.54839% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.58%. Comparing base (efd24a7) to head (232fb64).

Files with missing lines Patch % Lines
packages/grid/src/GridAccessibilityLayer.tsx 94.64% 3 Missing ⚠️
packages/grid/src/Grid.tsx 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2630      +/-   ##
==========================================
- Coverage   49.77%   49.58%   -0.20%     
==========================================
  Files         774      775       +1     
  Lines       43770    43832      +62     
  Branches    11258    11091     -167     
==========================================
- Hits        21788    21735      -53     
- Misses      21936    22079     +143     
+ Partials       46       18      -28     
Flag Coverage Δ
unit 49.58% <93.54%> (-0.20%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

mofojed added 2 commits March 2, 2026 23:24
Performance tests are flaky in CI due to resource constraints.
They can be run explicitly with RUN_PERF_TESTS=1.
@mofojed mofojed requested review from a team and dgodinez-dh and removed request for a team March 3, 2026 22:13
Copy link
Contributor

@dgodinez-dh dgodinez-dh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out the branch:

  • ran e2e tests which passed
  • ran the ui
  • confirmed I see the accessibility layer in inspect
  • confirmed it filters as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants