[WC-3564]Fix: Tree node v2 loading sort expansion by gjulivan · Pull Request #2437 · mendix/web-widgets

gjulivan · 2026-09-22T09:09:16Z

Pull request type

Bug fix (non-breaking change which fixes an issue)

Description

This PR fixes several Tree Node v2 regressions related to data refresh, sorting, and expanded state handling. It keeps nodes expanded/collapsed correctly after datasource reloads, reapplies the correct sort order when the data source changes, and restores missing expand icons/loading behavior for deeper nodes and microflow/filter-driven trees. It also adds regression tests to cover the reported scenarios.

…ng sibling nodes

…very history The v2 preload filter was built by accumulating what had already been fetched (appendItems, bootstrap rounds, the startExpanded cascade and a late-arrival sweep, across five refs). When an app-level constraint replaced the whole result set - a gallery filtering the tree by department - those history gates had already locked in, so the filter stayed frozen at the first set's parents. Nodes from the new set arrived with no children, hence no expand icon, no aria-expanded and no keyboard handler. Replace all of it with a set derived from the current tree on every pass: the root level, every node whose subtree is rendered, and the children of those nodes. setFilter runs only when the derived set differs from the last applied one (sorted-id key). A root is strictly parentId === undefined, so an orphan renders at root level but is not a root for retrieval - that is what keeps the tree -> filter -> tree cycle terminating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ding-state Promote the change's delta specs into the package's main specs (tree-node-data-refresh: 3 requirements / 9 scenarios; tree-node-expand-state: 8 requirements / 25 scenarios) and move the change to openspec/changes/archive/2026-09-22-fix-tree-node-loading-state. Archived with tasks 13.1-13.5 unchecked: those are live-verification steps against the WC-3564 repro project, not code work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-09-29T12:15:40Z

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

What was reviewed

File	Change
`CHANGELOG.md`	5 new [Unreleased] entries for the fixed bugs
`CONTEXT.md`	New domain model / architecture documentation
`e2e/TreeNode.spec.js`	Improved collapse test helpers, role-based locators
`e2e/TreeNodeV2Reorder.spec.js`	New E2E spec for v2 reorder + expand regression scenarios
`e2e/TreeNodeV2Reorder.spec.js-snapshots/*.png`	Committed baselines
`e2e/TreeNode.spec.js-snapshots/treeNodeMultipleCollapsed-chromium-linux.png`	Updated baseline
`openspec/specs/...`	New capability specs
`package.json`	Test-project branch rename, Mendix version bump to 11.12.0
`src/TreeNode.editorPreview.tsx`	Fix `isUserDefinedLeafNode` + caption for v2
`src/components/v1/TreeNodeBranch.tsx`	Guard inline `height: 0` during LOADING to prevent flash
`src/components/v1/hooks/useAnimatedHeight.tsx`	Refactor animation: filter nested `transitionend`, measure target height correctly
`src/components/v2/TreeNode.tsx`	Spinner becomes render-time; hook now takes `treeData`; `syncPreloadFilter` replaces `appendItems`
`src/components/v2/__tests__/TreeNodeV2.spec.tsx`	Test helpers hoisted; new WC-3564 Bug 1 + Bug 2 regression tests
`src/components/v2/hooks/__tests__/helpers.spec.ts`	New direct unit tests for `deriveDesiredParentIds`
`src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts`	Tests updated to reflect no-LOADING-on-creation
`src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts`	Major rewrite to cover derived-set model, replaced result-set, cascade
`src/components/v2/hooks/helpers.ts`	New `deriveDesiredParentIds` pure function
`src/components/v2/hooks/useIncrementalTreeData.ts`	`resolveRestoredState` removed; LOADING never stored; state restored from map
`src/components/v2/hooks/useInfiniteTreeNode.ts`	Rewritten around derived parent set (D6): 5 history refs → 1 equality guard

Skipped (out of scope): pnpm-lock.yaml, openspec/changes/archive/

CI check status could not be retrieved automatically. Task log entries 15.10 and 16.8 record 95/95 unit tests passing and tsc --noEmit clean per the PR author.

Findings

⚠️ Low — Misleading comment on `makeDefaultProps` in `TreeNodeV2.spec.tsx`

File: src/components/v2/__tests__/TreeNodeV2.spec.tsx line ~1751
Problem: The JSDoc says "hasChildren (not the datasource) is what drives aria-expanded" — this implies hasChildren is independent of the datasource, but in v2 hasChildren = node.children.length > 0 is derived entirely from the datasource via parentAssociation relationships. This is a leftover from the reverted props.hasChildren design attempt (task 2.1) and will mislead future readers.
Fix:

/**
 * Default props for tests that need a node with children.
 * v2 derives hasChildren from node.children.length > 0, which is populated
 * by parentAssociation relationships in the datasource. Items 1 + 2 (child→parent)
 * exist so the expanded body renders real content.
 */

⚠️ Low — Mandatory live re-verification tasks 13.1–13.5 left unchecked after D6

File: openspec/changes/archive/.../tasks.md tasks 13.1–13.5
Problem: These were explicitly marked mandatory because CONTEXT.md records that mocked unit tests alone cannot catch the failure class that produced four prior regressions (transient empty items during initial load, real association graphs). After D6 rewrote setFilter timing a third time — exactly the category CONTEXT.md warns about — only 13.6 and 13.7 (the new department-switch scenario) were verified live. 13.1–13.5 are unchecked. The 95/95 unit suite and the E2E spinner-count assertions in TreeNodeV2Reorder.spec.js provide strong automated coverage, so this is not a hard blocker, but the design doc's own lesson says this category of change warrants live proof.
Fix: Before merging, confirm the four original scenarios against the repro project after D6 — or add a note to CONTEXT.md acknowledging that the E2E suite now substitutes for tasks 13.1–13.5.

⚠️ Low — Double navigation in reordering `beforeEach`

File: e2e/TreeNodeV2Reorder.spec.js lines ~317–340
Problem: The outer beforeEach (on the "v2: reordering (advanced page)" describe) calls openAdvancedPage(page) for every test. The inner beforeEach of the "reordering" sub-describe immediately navigates away (to set up data) and then calls openAdvancedPage(page) again. Reordering tests therefore do two full page navigations per test run.
Fix: Either move openAdvancedPage from the outer beforeEach directly into the two visual-regression tests, or restructure the describe groups so the outer hook only runs for visual tests:

test("visual regression: start-collapsed tree", async ({ page }) => {
    await openAdvancedPage(page);
    // ...
});