test(cypress): tree rows move under the pointer while Clarity animates the expansion, and Clarity's collapse is that animation #554

Open
opened 2026-11-09 15:07:09 +00:00 by hermes · 4 comments
Collaborator

Three Cypress specs flake in CI on the same mechanism: a click aimed at a tree row lands on whichever row has just moved under the pointer, and Cypress reports it as "being covered by another element" after a 30s retry. It is not a slow render - waiting longer cannot help, because the target has left.

Reproduction

Single-spec runs mostly pass; the failure needs the specs run together, the way the Build-and-test-development job runs them:

npx cypress run --browser electron \
  --env hosturl=http://localhost:5000,appLocation=/AppStream/clickme,serverType=SASJS \
  --config video=false \
  --spec "cypress/e2e/viewer-labels.cy.ts,cypress/e2e/full-table-search.cy.ts,cypress/e2e/viewbox.cy.ts,cypress/e2e/row-cell-limits.cy.ts"

Measured here: 23 tests, 22 passing, 1 failing (viewbox test 7) on an idle box; CI run 3609 failed full-table-search 1, viewbox 2, row-cell-limits 1, download 1.

Measurement

Sampling document.elementFromPoint at the centre of every visible .clr-treenode-link every 40ms while a library expands, against the mock stack on SASjs Server 4.14.0:

t=0..130ms   0 overlapped rows
t=140..183ms 1-2 overlapped rows        <- the window the click lands in
t>250ms      0 overlapped rows

During that window a library's .clr-treenode-children is animating its height from 0 to its content height, so every row below it is moving. Two rows at the same coordinates is the expected state while that runs.

What was tried, and why each failed

  1. .clr-treenode-children { overflow: hidden !important } - no effect, 2 overlap ticks remain. The rows are not escaping a clipped box; they are being laid out while the box grows.
  2. .clr-tree-node { overflow: visible/hidden !important } - no effect, 2 ticks.
  3. .clr-treenode-children { height: auto !important } - 0 overlap ticks, but collapse breaks: after collapsing a library its children stay visible (visible links 43 -> 50 -> 50), because Clarity collapses by animating that height to zero.
  4. [@.disabled]="true" on <clr-tree> - 0 overlap ticks, collapse breaks identically (43 -> 50 -> 50).
  5. Rendering the children only when the component's expanded flag is set, dropping [(clrExpanded)] - expands correctly, does not collapse (a library node keeps its 7 links after the collapse click), and the caret count drops from 2 to 1, so the affordance changes too.

The conclusion from 3, 4 and 5 is the same: Clarity's tree collapse is implemented by the height animation, so there is no non-animated state to fall back on.

What a fix requires

Owning the tree instead of configuring it: a DC tree component that renders libraries, tables and columns as plain @if children, with its own caret, keeping the existing libraryOnClick / tableOnClick / columnOnClick handlers, the search boxes and the ARIA pass. That also lets clr-tree-node { overflow-y: visible } and .clr-tree-node { overflow: visible !important } in styles.scss go.

Scope it to the sidebar nav tree first (the one openTableFromTree drives), then the viewbox picker and the viewer tree, with screenshots of collapsed, expanded, searching and tables_in_library_limit states.

Acceptance

  • The four-spec command above passes on ten consecutive runs, and viewbox.cy.ts alone on ten.
  • A library that is collapsed shows none of its tables; the caret still reflects the state.
  • No { force: true } anywhere in the helpers, and a missing table or library still fails by name.

Not this

Waiting for the animation in the tests. Two implementations were tried: summing every children-container height until two samples agree (never settled, viewer-labels went 0/5) and comparing each container's box height to its scrollHeight (never true at rest, because the containers are clipped - viewer-labels 1/5). Both are worse than no wait at all.

Three Cypress specs flake in CI on the same mechanism: a click aimed at a tree row lands on whichever row has just moved under the pointer, and Cypress reports it as "being covered by another element" after a 30s retry. It is not a slow render - waiting longer cannot help, because the target has left. ## Reproduction Single-spec runs mostly pass; the failure needs the specs run together, the way the `Build-and-test-development` job runs them: ``` npx cypress run --browser electron \ --env hosturl=http://localhost:5000,appLocation=/AppStream/clickme,serverType=SASJS \ --config video=false \ --spec "cypress/e2e/viewer-labels.cy.ts,cypress/e2e/full-table-search.cy.ts,cypress/e2e/viewbox.cy.ts,cypress/e2e/row-cell-limits.cy.ts" ``` Measured here: 23 tests, 22 passing, 1 failing (viewbox test 7) on an idle box; CI run 3609 failed `full-table-search` 1, `viewbox` 2, `row-cell-limits` 1, `download` 1. ## Measurement Sampling `document.elementFromPoint` at the centre of every visible `.clr-treenode-link` every 40ms while a library expands, against the mock stack on SASjs Server 4.14.0: ``` t=0..130ms 0 overlapped rows t=140..183ms 1-2 overlapped rows <- the window the click lands in t>250ms 0 overlapped rows ``` During that window a library's `.clr-treenode-children` is animating its `height` from 0 to its content height, so every row below it is moving. Two rows at the same coordinates is the expected state while that runs. ## What was tried, and why each failed 1. `.clr-treenode-children { overflow: hidden !important }` - no effect, 2 overlap ticks remain. The rows are not escaping a clipped box; they are being laid out while the box grows. 2. `.clr-tree-node { overflow: visible/hidden !important }` - no effect, 2 ticks. 3. `.clr-treenode-children { height: auto !important }` - 0 overlap ticks, but collapse breaks: after collapsing a library its children stay visible (visible links 43 -> 50 -> 50), because Clarity collapses by animating that height to zero. 4. `[@.disabled]="true"` on `<clr-tree>` - 0 overlap ticks, collapse breaks identically (43 -> 50 -> 50). 5. Rendering the children only when the component's `expanded` flag is set, dropping `[(clrExpanded)]` - expands correctly, does not collapse (a library node keeps its 7 links after the collapse click), and the caret count drops from 2 to 1, so the affordance changes too. The conclusion from 3, 4 and 5 is the same: **Clarity's tree collapse is implemented by the height animation**, so there is no non-animated state to fall back on. ## What a fix requires Owning the tree instead of configuring it: a DC tree component that renders libraries, tables and columns as plain `@if` children, with its own caret, keeping the existing `libraryOnClick` / `tableOnClick` / `columnOnClick` handlers, the search boxes and the ARIA pass. That also lets `clr-tree-node { overflow-y: visible }` and `.clr-tree-node { overflow: visible !important }` in `styles.scss` go. Scope it to the sidebar nav tree first (the one `openTableFromTree` drives), then the viewbox picker and the viewer tree, with screenshots of collapsed, expanded, searching and `tables_in_library_limit` states. ## Acceptance - The four-spec command above passes on ten consecutive runs, and `viewbox.cy.ts` alone on ten. - A library that is collapsed shows none of its tables; the caret still reflects the state. - No `{ force: true }` anywhere in the helpers, and a missing table or library still fails by name. ## Not this Waiting for the animation in the tests. Two implementations were tried: summing every children-container height until two samples agree (never settled, `viewer-labels` went 0/5) and comparing each container's box height to its `scrollHeight` (never true at rest, because the containers are clipped - `viewer-labels` 1/5). Both are worse than no wait at all.
Author
Collaborator

A DC-owned expansion was built and measured today. It works, it fixes the picker click that this issue is about, and it is not ready to land - recording all of it here so the next attempt starts from the evidence rather than from scratch.

What worked

Replacing Clarity's animated expansion with DC's own @if in dc-tree.component.html:

  • [clrExpandable]="false" on the node, so Clarity has no animation to run
  • @if (library['expanded']) { <div class='clr-treenode-children dc-tree-children'> ...children... </div> }
  • a DC-rendered caret (dc-tree-caret) so the affordance does not depend on Clarity's expandable state

Measured on the deployed build:

overlap sampler, 900ms at 40ms intervals      worst 0, ticks with overlap 0   (was 2, t=140-183ms)
collapse                                      flag true -> 7 links, flag false -> 0 links
carets                                        3 (one per library)
viewbox.cy.ts                                 12/12  <- test 7, the cause of this issue
full-table-search.cy.ts                       1/1
download.cy.ts                                8/8

So the mechanism is understood and the fix works for the picker and for those two specs.

What broke, and why it is not ready

In the same run, two specs regressed:

viewer-labels.cy.ts       1 passing, 4 failing
row-cell-limits.cy.ts     1 passing, 4 failing   (openEdit: no 'Edit' in [clrDropdownItem])

Reverting the viewer template alone did not restore them (viewer-labels still 1/5, now failing on the header assertion for a table that had opened), so the perturbation is in the shared nav tree, not only in the view sidebar. The view sidebar drives its own expansion from viewer.component.ts selectTable, which toggles library['expanded'] at the end of the load - written against Clarity's two-way binding - and libraryOnClick focuses #search_${LIBRARYREF} by id. Both are assumptions a DC-owned tree has to be reconciled with deliberately, in the component code, not just in the template.

There is also an accessibility regression to fix in any real attempt: Clarity's caret is title="Collapse", which treeNodeClicked uses (event.target.title === 'Collapse'), so replacing the caret changes that path too.

Recommended next attempt

Do it as its own PR, and do the component code first: make viewer.component.ts and home.component.ts set expanded explicitly (no toggle, no assumption about who owns it), then swap the templates, then re-run this set:

npx cypress run --browser electron \
  --env hosturl=http://localhost:5000,appLocation=/AppStream/clickme,serverType=SASJS \
  --config video=false \
  --spec "cypress/e2e/viewer-labels.cy.ts,cypress/e2e/full-table-search.cy.ts,cypress/e2e/download.cy.ts,cypress/e2e/viewbox.cy.ts,cypress/e2e/row-cell-limits.cy.ts"

Acceptance unchanged: that command green on ten consecutive runs, collapse hides children, caret present and labelled, no { force: true }.

A DC-owned expansion was built and measured today. It works, it fixes the picker click that this issue is about, and it is **not** ready to land - recording all of it here so the next attempt starts from the evidence rather than from scratch. ## What worked Replacing Clarity's animated expansion with DC's own `@if` in `dc-tree.component.html`: - `[clrExpandable]="false"` on the node, so Clarity has no animation to run - `@if (library['expanded']) { <div class='clr-treenode-children dc-tree-children'> ...children... </div> }` - a DC-rendered caret (`dc-tree-caret`) so the affordance does not depend on Clarity's expandable state Measured on the deployed build: ``` overlap sampler, 900ms at 40ms intervals worst 0, ticks with overlap 0 (was 2, t=140-183ms) collapse flag true -> 7 links, flag false -> 0 links carets 3 (one per library) viewbox.cy.ts 12/12 <- test 7, the cause of this issue full-table-search.cy.ts 1/1 download.cy.ts 8/8 ``` So the mechanism is understood and the fix works for the picker and for those two specs. ## What broke, and why it is not ready In the same run, two specs regressed: ``` viewer-labels.cy.ts 1 passing, 4 failing row-cell-limits.cy.ts 1 passing, 4 failing (openEdit: no 'Edit' in [clrDropdownItem]) ``` Reverting the viewer template alone did not restore them (`viewer-labels` still 1/5, now failing on the header assertion for a table that had opened), so the perturbation is in the shared nav tree, not only in the view sidebar. The view sidebar drives its own expansion from `viewer.component.ts` `selectTable`, which *toggles* `library['expanded']` at the end of the load - written against Clarity's two-way binding - and `libraryOnClick` focuses `#search_${LIBRARYREF}` by id. Both are assumptions a DC-owned tree has to be reconciled with deliberately, in the component code, not just in the template. There is also an accessibility regression to fix in any real attempt: Clarity's caret is `title="Collapse"`, which `treeNodeClicked` uses (`event.target.title === 'Collapse'`), so replacing the caret changes that path too. ## Recommended next attempt Do it as its own PR, and do the component code first: make `viewer.component.ts` and `home.component.ts` set `expanded` explicitly (no toggle, no assumption about who owns it), then swap the templates, then re-run this set: ``` npx cypress run --browser electron \ --env hosturl=http://localhost:5000,appLocation=/AppStream/clickme,serverType=SASJS \ --config video=false \ --spec "cypress/e2e/viewer-labels.cy.ts,cypress/e2e/full-table-search.cy.ts,cypress/e2e/download.cy.ts,cypress/e2e/viewbox.cy.ts,cypress/e2e/row-cell-limits.cy.ts" ``` Acceptance unchanged: that command green on ten consecutive runs, collapse hides children, caret present and labelled, no `{ force: true }`.
Author
Collaborator

Tried the markup swap on top of PR #557 today. It works in one configuration and is not stable, so it is not in the PR - recording the sequence because it narrows the problem a lot.

One configuration was green

Swap applied to dc-tree.component.html (nav tree + picker) and viewer.component.html, on top of #557's single-owner expanded fix:

viewer-labels      5/5
full-table-search  1/1
download           8/8
viewbox            12/12   <- test 7, the flake this issue is about, gone
row-cell-limits    5/5
                   31/31
overlap sampler    0 ticks (was 2 at t=140-183ms)
unit suite         1403 passing

So the mechanism works and the flake disappears when the animation goes.

But two visual defects, and fixing them broke two specs

That run shipped with two problems I could see in a screenshot: the library labels were scrolled out of view (the tree's scrollWidth was 445 against a 240px panel, so adding a caret to a row that already filled the width pushed it sideways), and the caret itself was invisible - the app's icon registry only carries the shapes its templates ask for (ClrIcons.add(CLR_SHAPES), generated), and a tree caret was never one of them.

Fixing both - a CSS disclosure triangle plus a display: flex row for library rows - brought back:

viewer-labels      1/5 failing x4
row-cell-limits    1/5 failing x4   (openEdit: no 'Edit' in [clrDropdownItem])

and scoping the flex rule to library rows only (:has(> p.m-0.cursor-pointer)) did not change that. So the row layout is load-bearing for interactions the specs exercise, beyond the caret itself.

What that tells the next attempt

  • The swap is not a template-only change: the row layout in styles.scss (and whatever makes table rows right-clickable for the Edit menu) has to be handled in the same commit, with the five specs run after each step.
  • Budget for iterations: each cycle here is a build, a deploy, a reseed and a ~10-20 minute spec run.
  • Start from the green configuration above (it is small - the diff is in this comment's history) and fix the two visual defects one at a time, re-running the gate after each, rather than together.

I have reverted the swap; the branch keeps only the two component fixes (#557), which are green apart from this flake.

Tried the markup swap on top of PR #557 today. It works in one configuration and is not stable, so it is not in the PR - recording the sequence because it narrows the problem a lot. ## One configuration was green Swap applied to `dc-tree.component.html` (nav tree + picker) and `viewer.component.html`, on top of #557's single-owner `expanded` fix: ``` viewer-labels 5/5 full-table-search 1/1 download 8/8 viewbox 12/12 <- test 7, the flake this issue is about, gone row-cell-limits 5/5 31/31 overlap sampler 0 ticks (was 2 at t=140-183ms) unit suite 1403 passing ``` So the mechanism works and the flake disappears when the animation goes. ## But two visual defects, and fixing them broke two specs That run shipped with two problems I could see in a screenshot: the library labels were scrolled out of view (the tree's `scrollWidth` was 445 against a 240px panel, so adding a caret to a row that already filled the width pushed it sideways), and the caret itself was invisible - the app's icon registry only carries the shapes its templates ask for (`ClrIcons.add(CLR_SHAPES)`, generated), and a tree caret was never one of them. Fixing both - a CSS disclosure triangle plus a `display: flex` row for library rows - brought back: ``` viewer-labels 1/5 failing x4 row-cell-limits 1/5 failing x4 (openEdit: no 'Edit' in [clrDropdownItem]) ``` and scoping the flex rule to library rows only (`:has(> p.m-0.cursor-pointer)`) did not change that. So the row layout is load-bearing for interactions the specs exercise, beyond the caret itself. ## What that tells the next attempt - The swap is not a template-only change: the row layout in `styles.scss` (and whatever makes table rows right-clickable for the Edit menu) has to be handled in the same commit, with the five specs run after each step. - Budget for iterations: each cycle here is a build, a deploy, a reseed and a ~10-20 minute spec run. - Start from the green configuration above (it is small - the diff is in this comment's history) and fix the two visual defects one at a time, re-running the gate after each, rather than together. I have reverted the swap; the branch keeps only the two component fixes (#557), which are green apart from this flake.
Author
Collaborator

Cycle 1 done (fix the sideways scroll on the green configuration, then run the gate). The scroll fix works; the gate still fails, and the failure turned out to be more interesting than the layout.

The scroll fix

Scoping the label constraint to the rows that actually carry the caret:

.nav-tree clr-tree-node .clr-treenode-content:has(> .dc-tree-caret) {
  display: flex; align-items: center; min-width: 0;
}
.nav-tree clr-tree-node .clr-treenode-content:has(> .dc-tree-caret) > p {
  flex: 1 1 auto; min-width: 0; overflow: hidden; text-overflow: ellipsis;
}

measured on .nav-tree:

before   scrollWidth 445  clientWidth 240  scrollLeft 134   (labels off-screen)
after    scrollWidth 240  clientWidth 240  scrollLeft 0

Table rows are not matched by that selector, so their markup and layout are untouched.

But row-cell-limits still fails, and it is not the layout

2 | EDIT serves a table over the row limit ... AssertionError: Expected to find
    content: 'Edit' within the selector: '[clrDropdownItem]' but never did.
    at openEdit (row-cell-limits.cy.ts:221)

openEdit does not touch the tree - it clicks .filterSide and then expects an Edit item in the viewer's options dropdown. Per the spec's own comment, that item renders only when the selected table is registered in MPE_TABLES (tableEditExists()). So the failing link is the table selection that should have happened earlier did not stick, not a click landing on the wrong element:

  • the tree click runs onTableClick(libTable, library), unchanged by the swap
  • but the swap re-renders the children whenever expanded flips, and a re-render while the selection is being applied is a plausible way for the selection to be lost

That is the thing to chase next, and it is component state rather than CSS: instrument onTableClick / tableEditExists() and see whether the table is set and then cleared, or never set.

Also

viewer-labels failed in the same run (4/5), which it did not do in the one green configuration - the same selection/re-render suspicion applies, since its first step is opening a table through the tree.

Reverted again; the branch keeps only #557's two component fixes.

Cycle 1 done (fix the sideways scroll on the green configuration, then run the gate). The scroll fix works; the gate still fails, and the failure turned out to be more interesting than the layout. ## The scroll fix Scoping the label constraint to the rows that actually carry the caret: ```scss .nav-tree clr-tree-node .clr-treenode-content:has(> .dc-tree-caret) { display: flex; align-items: center; min-width: 0; } .nav-tree clr-tree-node .clr-treenode-content:has(> .dc-tree-caret) > p { flex: 1 1 auto; min-width: 0; overflow: hidden; text-overflow: ellipsis; } ``` measured on `.nav-tree`: ``` before scrollWidth 445 clientWidth 240 scrollLeft 134 (labels off-screen) after scrollWidth 240 clientWidth 240 scrollLeft 0 ``` Table rows are not matched by that selector, so their markup and layout are untouched. ## But `row-cell-limits` still fails, and it is not the layout ``` 2 | EDIT serves a table over the row limit ... AssertionError: Expected to find content: 'Edit' within the selector: '[clrDropdownItem]' but never did. at openEdit (row-cell-limits.cy.ts:221) ``` `openEdit` does not touch the tree - it clicks `.filterSide` and then expects an `Edit` item in the viewer's options dropdown. Per the spec's own comment, that item renders only when the selected table is registered in `MPE_TABLES` (`tableEditExists()`). So the failing link is **the table selection that should have happened earlier did not stick**, not a click landing on the wrong element: - the tree click runs `onTableClick(libTable, library)`, unchanged by the swap - but the swap re-renders the children whenever `expanded` flips, and a re-render while the selection is being applied is a plausible way for the selection to be lost That is the thing to chase next, and it is component state rather than CSS: instrument `onTableClick` / `tableEditExists()` and see whether the table is set and then cleared, or never set. ## Also `viewer-labels` failed in the same run (4/5), which it did not do in the one green configuration - the same selection/re-render suspicion applies, since its first step is opening a table through the tree. Reverted again; the branch keeps only #557's two component fixes.
Author
Collaborator

Correction to comment 23469, and it matters: the two "regressions" were my own test estate, not the swap.

Instrumenting the selection path (onTableClick, tableEditExists) showed the table selection was fine all along:

activeRow: MPE_X_SEARCH
trace: onTableClick { table: MPE_X_SEARCH, switching: false }
       tableEditExists  "DC_JSLIB.MPE_X_SEARCH"   (twice)
dropdown items: Viewboxes, Filter, Download, Web Query URL      <- no Edit

Edit was missing because tableEditExists() reads globals.editor.libsAndTables, which comes from startupservice, which builds sasdatasets from mpe_tables.json. And that file did not contain MPE_X_SEARCH, because the mock estate needs the fixtures that services/admin/createdemodata.js adds - and per makedata.js's own comment, that service runs after it:

Cypress fixtures (TESTDATA, MPE_X_SEARCH, MPE_X_WIDE, DC996664 and the extra MPE_TABLES/VALIDATIONS/SELECTBOX/SECURITY rows they need) are NOT part of a DC install. They live in services/admin/createdemodata.js, which the test estate runs after this service.

I had been reseeding with makedata alone between deploys. After createdemodata, startupservice returns 20 DC_JSLIB datasets with MPE_X_SEARCH among them, the dropdown shows Edit, and the same build passes:

viewer-labels 5/5, full-table-search 1/1, download 8/8, viewbox 12/12, row-cell-limits 5/5
31/31

So: the swap plus the scoped scroll fix is green, the sidebar no longer scrolls sideways (scrollWidth 240 = clientWidth), and the caret now renders as a CSS disclosure triangle (3 marks, 1 rotated open) since the generated icon registry carries no caret shape.

Worth its own issue: a half-seeded estate fails these specs with a message that points at the tree, not at the data. That cost most of a day here.

Correction to comment 23469, and it matters: the two "regressions" were my own test estate, not the swap. Instrumenting the selection path (`onTableClick`, `tableEditExists`) showed the table selection was fine all along: ``` activeRow: MPE_X_SEARCH trace: onTableClick { table: MPE_X_SEARCH, switching: false } tableEditExists "DC_JSLIB.MPE_X_SEARCH" (twice) dropdown items: Viewboxes, Filter, Download, Web Query URL <- no Edit ``` `Edit` was missing because `tableEditExists()` reads `globals.editor.libsAndTables`, which comes from `startupservice`, which builds `sasdatasets` from `mpe_tables.json`. And that file did not contain `MPE_X_SEARCH`, because the mock estate needs the fixtures that `services/admin/createdemodata.js` adds - and per `makedata.js`'s own comment, that service runs *after* it: > Cypress fixtures (TESTDATA, MPE_X_SEARCH, MPE_X_WIDE, DC996664 and the extra MPE_TABLES/VALIDATIONS/SELECTBOX/SECURITY rows they need) are NOT part of a DC install. They live in services/admin/createdemodata.js, which the test estate runs after this service. I had been reseeding with `makedata` alone between deploys. After `createdemodata`, `startupservice` returns 20 DC_JSLIB datasets with `MPE_X_SEARCH` among them, the dropdown shows `Edit`, and the same build passes: ``` viewer-labels 5/5, full-table-search 1/1, download 8/8, viewbox 12/12, row-cell-limits 5/5 31/31 ``` So: the swap plus the scoped scroll fix is green, the sidebar no longer scrolls sideways (scrollWidth 240 = clientWidth), and the caret now renders as a CSS disclosure triangle (3 marks, 1 rotated open) since the generated icon registry carries no caret shape. Worth its own issue: a half-seeded estate fails these specs with a message that points at the tree, not at the data. That cost most of a day here.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dc/dc#554