fix: xy_plot_tool_spec.js to use rdg-focus-sink to open cell editor#2784
fix: xy_plot_tool_spec.js to use rdg-focus-sink to open cell editor#2784dougmartin wants to merge 2 commits intomasterfrom
Conversation
The typeInTableCell method used a click to open the cell editor, which
was unreliable and caused flaky test failures (timeout waiting for
.rdg-text-editor). Updated to use the same rdg-focus-sink + type('{enter}')
approach already proven reliable in typeInTableCellXY.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2784 +/- ##
==========================================
- Coverage 86.19% 85.71% -0.48%
==========================================
Files 847 847
Lines 46352 46352
Branches 12043 12043
==========================================
- Hits 39951 39730 -221
- Misses 5995 6195 +200
- Partials 406 427 +21
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:
|
collaborative-learning
|
||||||||||||||||||||||||||||
| Project |
collaborative-learning
|
| Branch Review |
fix-flaky-xy-plot-test
|
| Run status |
|
| Run duration | 03m 15s |
| Commit |
|
| Committer | Doug Martin |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Pull request overview
This PR updates Cypress table-cell editing interactions to avoid flaky editor-opening behavior by switching from click-to-edit to using React Data Grid’s keyboard focus sink + {enter} approach (mirroring the existing typeInTableCellXY strategy).
Changes:
- Update
TableToolTile.typeInTableCell()to open the editor via.rdg-focus-sink+type('{enter}')instead of an additional click. - Remove an outdated “flaky” comment in the XY plot tool Cypress spec.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cypress/support/elements/tile/TableToolTile.js | Changes table cell edit helper to use RDG keyboard activation via .rdg-focus-sink. |
| cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js | Removes a stale comment about flakiness. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
The change by itself looks fine to me. However, based on my comment from the other usage of rdg-focus-sink in this file, and the commit message when it was added: 312e291 I suspect this will break other tests.
I'd recommend running the full test suite locally, or at least running ~11 or so specs that call this typeInTableCell.
The fix might be to add "within" to all of the tests that fail. Or you could do what I proposed in the commit message and make the table utility for cypress a class that you have to instantiate with a specific table model. I think this is why I didn't make this change originally. ;)
The global cy.get('.rdg-focus-sink') selector matches elements in all
tables on the page, causing cy.type() to fail when multiple tables are
present (e.g., duplicate_tile_spec.js). Scope the selector by traversing
from the target cell to its parent .rdg container.
Good catch, but after looking at all the tests that use this function (with Claude's help) I scoped the lookup from the cell itself (in commit #29ebaf0d8). This way typeInTableCell self-scopes like typeInTableCellXY does, without requiring callers to change. To save Cypress runs I ran all 11 the specs that use typeInTableCell locally and they all pass. |
The typeInTableCell method used a click to open the cell editor, which was unreliable and caused flaky test failures (timeout waiting for .rdg-text-editor). Updated to use the same rdg-focus-sink + type('{enter}') approach already proven reliable in typeInTableCellXY.