feat: DH-21829: add invisible accessibility layer to Grid for e2e testing#2630
feat: DH-21829: add invisible accessibility layer to Grid for e2e testing#2630mofojed wants to merge 11 commits intodeephaven:mainfrom
Conversation
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
- 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
This reverts commit 28c9186.
|
My 3 concerns:
|
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 Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Performance tests are flaky in CI due to resource constraints. They can be run explicitly with RUN_PERF_TESTS=1.
dgodinez-dh
left a comment
There was a problem hiding this comment.
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
- Add tests/grid-perf-app/ standalone app for testing Grid performance without a Deephaven server, using MockGridModel - Support query params: rows, cols, a11y (to toggle accessibility layer) - Add grid-perf-app.spec.ts for accessibility layer performance comparison - Remove FPS threshold assertions - tests now just output results - Add npm run e2e:grid-performance script - Update README with Grid Performance Testing section
|
Looks like there is a decent performance impact after writing some further tests |
|
Performance is a concern now. Maybe we could add the methods onto the grid itself, and just expose something like:
I'm not particularly a fan of that, as then one of the main things (reading data) becomes more complicated. |