diff --git a/packages/pluggableWidgets/tree-node-web/CHANGELOG.md b/packages/pluggableWidgets/tree-node-web/CHANGELOG.md index 102f086aeb..1b008b81eb 100644 --- a/packages/pluggableWidgets/tree-node-web/CHANGELOG.md +++ b/packages/pluggableWidgets/tree-node-web/CHANGELOG.md @@ -11,6 +11,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - We fixed an issue where the tree did not reflect a new data source sort order (for example after changing a sequence attribute) until the page was reopened. - We fixed an issue where all nodes collapsed when the data source refreshed. Expanded and collapsed nodes now keep their state, and the tree no longer clears while the data source is reloading. - We fixed an issue where a node did not show that it has children after a child was added to it. Expanding a node now always pre-loads one level ahead, also for children that arrive after the expansion, which restores the missing expand icon on nodes deeper than two levels. +- We fixed an issue where a tree node's loading spinner would never disappear when using a microflow data source with "Start expanded" set to yes. +- We fixed an issue where expanding one node could permanently remove the expand icon from an unrelated, unexpanded node elsewhere in the tree when using a microflow data source. +- We fixed an issue where a node's expand icon for a deeper tier would not appear until that node was collapsed and expanded again. +- We fixed an issue where, with "Start expanded" set to yes, tree nodes deeper than the second level would not show an expand icon until a parent node was manually collapsed and expanded again. +- We fixed an issue where nodes shown after the data source was filtered elsewhere on the page (for example by a gallery or a list view acting as a filter) had no expand icon and could not be opened at all, even though they had children. ## [3.11.0] - 2026-05-27 diff --git a/packages/pluggableWidgets/tree-node-web/CONTEXT.md b/packages/pluggableWidgets/tree-node-web/CONTEXT.md new file mode 100644 index 0000000000..ed2a150801 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/CONTEXT.md @@ -0,0 +1,77 @@ +# Tree Node widget — domain model + +## v1 vs v2 + +`TreeNode.tsx` (root dispatcher) routes by whether `parentAssociation` is configured: + +- **`parentAssociation` set → v2** (`src/components/v2/`): self-referencing "infinite tree" mode. One widget instance renders the whole tree; each item's own association tells the engine its parent. Added in 3.11.0. +- **`parentAssociation` unset → v1** (`src/components/v1/`): manual-nesting mode. Each tree level is a separately-configured widget instance, nested inside the parent level's "children" slot in Studio Pro. + +### `hasChildren` prop is v1-only, architecturally + +`hasChildren: ListExpressionValue` (XML caption "Has children") exists so a Studio Pro developer can declare per-item whether a node has children, when there's no other way to know (v1: no association, no structural signal). `TreeNode.editorConfig.ts:38-39` hides this property from Studio Pro whenever `parentAssociation` is configured — i.e. whenever v2 is in use. It has no XML default value, so on a v2 widget instance `props.hasChildren` is `undefined` at runtime, not just "possibly misconfigured." **v2 must never read `props.hasChildren` — it will crash.** (Confirmed live during WC-3564: `TypeError: Cannot read properties of undefined (reading 'get')`.) + +For v2, "does this node have children" is derived structurally: `node.children.length > 0`, where `node.children` is populated by matching real association values across the full item set the datasource has delivered so far (`useIncrementalTreeData.ts`). `useInfiniteTreeNode.ts`'s preload mechanism fetches one level ahead of everything rendered, specifically so a node's own children are already known by the time that node is rendered as clickable — this is what keeps `node.children.length > 0` from being stale/late in practice. + +There is no cheaper way to know this without fetching. Mendix's pluggable-widget `ListValue` API is one shared, filter-driven list — no per-item "does this have children" or count-only primitive exists for pluggable widgets. Answering "does node N have children" means asking the datasource for items whose parent is N and seeing what comes back; the preload mechanism exists because that's the only way this API surface allows for v2's configuration mode, not because of a missed optimization. + +## `TreeNodeState.LOADING` (v2) — spinner, not a stored per-node fact (WC-3564) + +Originally (pre-WC-3564) a freshly-created node started in `LOADING` and only left it once its own id reappeared in a _later_ datasource delivery — meant to avoid the expand icon "popping in" late for nodes that would turn out to have children. That resolution criterion was wrong: a microflow datasource always redelivers its full flattened result regardless of the filter passed to `setFilter`, so _any_ later delivery could match, not just one that actually answered a given node's question. Two bugs followed: + +- Bug 1: with "Start expanded" = Yes, no later delivery route existed at all for the frozen microflow case — permanent spinner. +- Bug 2: with "Start expanded" = No, expanding any node caused a full redelivery that could wrongly resolve an unrelated, unexpanded node into a childless state — permanently killing its icon. + +Fix: `LOADING` is no longer stored on the node at all. A node is created directly as `EXPANDED`/`COLLAPSED_WITH_JS` (per `startExpanded`) — never `LOADING`. The spinner is a pure render-time decision in `TreeNode.tsx`: show it when a node has no known children yet (`node.children.length === 0`) **and** `props.datasource.status === ValueStatus.Loading` — Mendix's own, real-time, first-party "is this datasource actually fetching right now" signal. No per-node bookkeeping, so nothing can be "stuck" (the flag is never persisted) and nothing about one node's resolution can affect another's (it's a single global flag, not a per-node mutation). + +A node's expanded/collapsed state, on the other hand, _is_ stored — and is remembered across rebuilds. `useIncrementalTreeData.ts` keeps a `statesByIdRef` map of id → `TreeNodeState` and snapshots every node into it just before a rebuild clears the node map, so a re-created node comes back in the state it had rather than in the `startExpanded` default. This is needed because a rebuild happens far more often than "the configuration changed": `isConfigChanged` compares prop instances by reference and the Mendix client hands over fresh instances on every refresh, and any single item deletion legitimately trips the removed-ids check. Neither is avoidable, so the rebuild is made non-destructive instead. A remembered state deliberately beats `startExpanded` in both directions — a node the user collapsed under "Start expanded" = Yes must stay collapsed across a refresh. The map is never pruned: an id that disappears and comes back within the same session (a filter change, a microflow round-trip) is exactly the case it exists to serve. + +Do not reintroduce a `LOADING` arm into that restore path. An earlier version of the state-restore work routed it through a helper whose "nothing remembered" arm returned `TreeNodeState.LOADING` — which is Bug 1's exact mechanism. The restore is a plain fallback: `statesByIdRef.current.get(nodeId) ?? (startExpanded ? EXPANDED : COLLAPSED_WITH_JS)`. + +## The incremental node map reuses nodes across updates — two things that has to re-derive + +`useIncrementalTreeData.ts` does not rebuild the tree from `datasource.items` on each update; it keeps a node map and reuses nodes across deliveries. That is what makes v2's "children of anything expanded so far, accumulated over many filtered fetches" model work at all, but it means anything derived from a _delivery_ rather than from an _item_ has to be re-derived explicitly, or it silently keeps whatever the first delivery said. Two such things, both fixed under WC-3564: + +- **Sibling and root order.** A node used to be appended to `rootsRef`/`parent.children` only on the first delivery its id appeared in; later deliveries took the "already exists" branch, which refreshes `item`/`title`/`parentId` but never touches sibling arrays. So a re-sorted datasource (the real case: a microflow updates the sequence attribute the datasource sorts on) left the tree in its first-load order until the widget remounted. Now an `orderById` map is built from the current delivery's indices and applied as a sort to `rootsRef` and every node's `children` at the _end_ of each update — placement itself is order-independent by design (a child can arrive before its parent, and gets re-placed when the parent shows up), so the end of the pass is the only point where the full delivery order is actually known. A node the current delivery does not mention sorts after every node it does, rather than being shuffled. +- **`items === undefined` is "not delivered yet", not "empty".** The hook used to read `items ?? []`, which made every known id look removed, tripped the removed-ids rebuild, and flashed "no data available" mid-refresh. It now returns early and keeps the tree. This is also what makes the `datasource.status`-driven spinner above correct during a load — the tree stays mounted, so there are real nodes to render a spinner on. + +Contrast with v1, which is unaffected by both: it rebuilds from `datasource.items` on every update, so it re-derives order and emptiness for free. + +## `useInfiniteTreeNode.ts`'s one-level-lookahead preload — derived from the tree, not accumulated from deliveries + +**The rule, and it is the whole file:** for every node that is visible, or one expand away from visible, the widget must know whether it has children. So the parents whose children the datasource has to deliver are + +``` +desiredParents = { undefined } // the root level, always + ∪ { N : N is visible } // so N's children render + ∪ { N : parent(N) is visible } // so N's own icon is already correct +``` + +`deriveDesiredParentIds` (in `hooks/helpers.ts`, pure and unit-tested) computes that set from `treeData` on every datasource update and on every expand click. `useInfiniteTreeNode.ts` then calls `setFilter` — but only when the set differs from the last one it applied, compared by a sorted id key (`lastAppliedKeyRef`). That guard is the only state the hook keeps. + +Two definitions carry weight: + +- **"Visible" recurses through `COLLAPSED_WITH_CSS` as well as `EXPANDED`.** That state means a node was opened and then closed, so its subtree is still in the DOM, hidden by CSS — it still has to be kept correct. Recursing only through `EXPANDED` would shrink the set on a collapse, drop already-fetched grandchildren out of the result set, trip the removed-ids rebuild, and leave the re-expanded node's children iconless. `COLLAPSED_WITH_JS` (never opened) stops the recursion. +- **A root is a node with no parent at all (`parentId === undefined`).** An orphan — its parent exists but the current delivery does not carry it — is promoted to root level _for rendering_, but is deliberately not a root _for retrieval_. Treating orphans as roots would make the derived set depend on which items happen to be delivered, and the filter that changes what is delivered would then depend on that: a loop with no fixed point. Membership as defined depends on **ancestry only**, which is what makes the tree → filter → tree cycle provably terminating. Only objects the _current_ delivery carries go into the filter; a retained node's `ObjectItem` is a stale reference. + +**This replaced three separate mechanisms**, all of which were the same idea reached by accumulating delivery history: `appendItems` (click-driven), bootstrap rounds 1+2 / the unbounded `startExpanded = Yes` cascade (update-driven), and a late-arrival sweep over `expandedIdsRef`. Each reached nodes the others could not, each carried its own gate, and together they kept five refs of "what have I fetched." The derivation subsumes all three — a click, a late arrival, and a root all change the tree, and the set is re-derived from the tree. + +It also fixes what none of them could: **a replaced result set.** When an app-level constraint swaps the whole result set (the live repro: a gallery filtering the tree by department), history-based 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, and therefore no icon, no `aria-expanded`, and no keyboard handler — completely inert, with no affordance to recover. A sibling widget with `startExpanded = Yes` looked correct only incidentally, because its unbounded cascade had already grown its filter wide enough to cover the new roots. + +**One thing survives from the old shape**: the init block. On the very first pass there is no tree to derive from. Under `startExpanded = No` it applies the root-only filter and lets the derivation take over from the first real delivery; under `startExpanded = Yes` it applies **no** filter at all, because an unfiltered first delivery returns the whole (non-microflow) table in one retrieve, whereas starting from the root level would cost one retrieve per level of depth. + +**What a result-set replacement actually costs, measured live** (`treenodev2_advanced`, department gallery, filter read off `extraXpath` per `runtimeOperation`): per widget per switch, one retrieve still carries the _previous_ parent list — the app-level constraint changes before the widget can re-derive, which is unavoidable — then the filter drops to `[not(...Category_Parent/...Category)]` alone, then roots + one level, then one more. Four retrieves, and the list _resets_ on every switch rather than growing, because the rebuild empties `treeData` and the derived set falls back to `{undefined}`. That reset is the self-healing property the old accumulating maps could not have. + +Convergence for the microflow case (where `setFilter` is ignored and the full flattened result comes back regardless): the tree is already full, so the derived set is stable, one `setFilter` is issued and ignored, the next delivery is identical, and the key guard suppresses everything after that. Strictly better than the old cascade's "stop when a round finds nothing new." + +**Lesson, still worth keeping in mind** — it is why this section reads the way it does. The pre-D6 code went through four attempts, and each one failed in a way inspection and mocked unit tests could not catch: a fire-count cap locked in while `datasource.items` was still transiently empty during load (root-caused only with per-widget-tagged debug logging, since three tree widgets mount on that page at once); a content-gated 2-round cap fixed that but was too shallow against 4-tier data; an unbounded cascade fixed depth but was still blind to a replaced result set. The through-line: **derive what you need from current state, don't log what you have done.** Every one of those bugs was a stale record of history. And still — any change to `setFilter` timing or call count here needs live verification against a real repro project at real depth, because mocks do not reproduce the transient loading window or a real association graph. + +## Decisions log + +- 2026-09-15 (WC-3564): initially decided to wire up `props.hasChildren` as the icon-visibility source. **Reverted after live testing crashed the widget** — discovered `hasChildren` is architecturally unavailable in v2 (see above). Corrected to derive `hasChildren` from `node.children.length > 0` (the pre-existing, structurally-correct signal) and drive the spinner from `datasource.status` instead of any per-node stored state. +- 2026-09-15 (WC-3564, found during manual verification): discovered and fixed two further pre-existing bugs in `useInfiniteTreeNode.ts`'s one-level-lookahead preload (see above) — bundled into the same change with explicit user sign-off, since found/understood/fixed during the same verification pass. The second one required three attempts: a fire-count-based cap broke live testing and was reverted; a content-based 2-round cap fixed that but was itself too shallow against deeper data; an unbounded content-gated cascade, scoped to `startExpanded = Yes` only (confirmed via explicit question — user rejected applying it to `startExpanded = No` too, since that would eagerly prefetch descendants of still-collapsed, not-yet-visible branches), fixed it correctly at real depth, verified live. +- 2026-09-17 (WC-3564): folded the parallel branch `tmp/treenode-fix1` into this change rather than merging it separately. It fixed two more v2 bugs (datasource order never re-applied; every node collapsing on refresh) plus the late-arrival preload gap, but rewrote the same two functions this change rewrote, so reconciling them was a design decision rather than a merge conflict resolution. Re-applied as fresh commits on top of this change instead of rebasing, because a conflict resolution would have silently landed a shape neither design intended. +- 2026-09-17 (WC-3564): dropped the parallel branch's `resolveRestoredState` helper when folding in its expansion-state fix. Its "nothing remembered" arm returned `TreeNodeState.LOADING`, reintroducing Bug 1's exact mechanism, and its "remembered is `LOADING`" arm was dead code once `LOADING` stopped being stored. Replaced by a plain `?? ` fallback on the node-creation branch. +- 2026-09-22 (WC-3564, found live): replaced the whole delivery-history preload machinery in `useInfiniteTreeNode.ts` (`appendItems`, bootstrap rounds, the `startExpanded = Yes` cascade, the late-arrival sweep, five refs) with a set derived from the tree on every pass (see above). Trigger: a gallery filtering the tree by department replaced the result set, and every node in the new set rendered with no expand affordance at all, because the filter was still frozen at the first set's parents. Rejected adding a fourth heuristic ("was the result set replaced?") — `incoming ∩ previous === ∅` is wrong the moment two departments share an item, and it would have been one more record of history in a mechanism whose every bug so far was a stale record of history. Root defined strictly as `parentId === undefined` (user's call, option 2.a): an orphan renders at root level but is not a root for retrieval, which is what keeps the tree → filter → tree cycle terminating. +- 2026-09-22 (WC-3564): `deriveDesiredParentIds` deliberately takes no `startExpanded` argument, although the task list specified one. `treeNodeState` already encodes it (a node is created `EXPANDED` under `startExpanded = Yes`), so reading the prop again would double-source the same truth and let the two disagree after a user collapse. +- 2026-09-17 (WC-3564): removed a stale `NOT YET IMPLEMENTED` requirement from the change's `tree-node-expand-state` spec. It described the reverted first attempt at the bootstrap fix and contradicted the unbounded-cascade requirement in the same file, which the final attempt delivered. diff --git a/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js b/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js index f9866caebd..fbb7c448fd 100644 --- a/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js +++ b/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js @@ -13,6 +13,20 @@ function getTreeNodeHeaders(page) { return page.locator(".mx-name-treeNode1 .widget-tree-node-branch-header-value"); } +function getTreeNodeItem(page, name) { + return page.locator(".mx-name-treeNode1").getByRole("treeitem", { name, exact: true }); +} + +async function toggleNode(item, expanded) { + const body = item.locator(":scope > .widget-tree-node-body"); + await item.locator(".widget-tree-node-branch-header").first().click(); + // Clicks are ignored while the node is still loading its children + await expect(item).toHaveAttribute("aria-expanded", String(expanded)); + // Inline height is removed once the animation ends + await expect(body).not.toHaveAttribute("style", /height/); + await (expanded ? expect(body).toBeVisible() : expect(body).toBeHidden()); +} + test.describe("capabilities: expand", () => { test.beforeEach(async ({ page }) => { await page.goto("/"); @@ -40,21 +54,21 @@ test.describe("capabilities: collapse", () => { }); test("collapses a node", async ({ page }) => { - const headers = await getTreeNodeHeaders(page); - await headers.first().click(); - await headers.first().click(); + const africa = getTreeNodeItem(page, "Africa"); + + await toggleNode(africa, true); + await toggleNode(africa, false); await expect(page.locator(".mx-name-treeNode1")).toHaveScreenshot(`treeNodeCollapsed.png`, 0.1); }); test("collapses multiple nodes", async ({ page }) => { - const headers = await getTreeNodeHeaders(page); - await headers.nth(1).click(); - await headers.first().click(); - await headers.nth(11).click(); - await headers.nth(11).click(); - await headers.first().click(); - // Second header has become the 5th cuz first header was opened and introduces 3 headers. - await headers.nth(4).click(); + const africa = getTreeNodeItem(page, "Africa"); + const europe = getTreeNodeItem(page, "Europe"); + + await toggleNode(europe, true); + await toggleNode(africa, true); + await toggleNode(africa, false); + await toggleNode(europe, false); await expect(page.locator(".mx-name-treeNode1")).toHaveScreenshot(`treeNodeMultipleCollapsed.png`, 0.1); }); }); diff --git a/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js-snapshots/treeNodeMultipleCollapsed-chromium-linux.png b/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js-snapshots/treeNodeMultipleCollapsed-chromium-linux.png index 5eeae093e1..494c54ccfd 100644 Binary files a/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js-snapshots/treeNodeMultipleCollapsed-chromium-linux.png and b/packages/pluggableWidgets/tree-node-web/e2e/TreeNode.spec.js-snapshots/treeNodeMultipleCollapsed-chromium-linux.png differ diff --git a/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js b/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js new file mode 100644 index 0000000000..661ba2f50b --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js @@ -0,0 +1,254 @@ +import { test, expect } from "@mendix/run-e2e/fixtures"; +import { waitForDataReady } from "@mendix/run-e2e/mendix-helpers"; + +/** + * Reordering tests for the v2 (self-referencing) tree on MyFirstModule.TreeNodeV2_Advanced. + * + * Page contract (test project branch `tree-node-web/v2`): + * - gallery1 lists MyFirstModule.Department, single selection. Both trees live in a + * data view bound to that selection, so a department must be selected first. The shipped + * categories span departments; the reorder tests temporarily assign two IT categories to + * HR so Electronics has a sibling and a second child, then restore their department. + * - treeNode1 = "Start expanded: No", treeNode2 = "Start expanded: Yes". + * Both sort their datasource on MyFirstModule.Category/Order ascending. + * - Each node's custom header renders "{Order}) {Name}" plus two action buttons: + * actionButton1 / actionButton3 -> ACT_MoveCategory(IsIncrement = true) => Order + 1 + * actionButton2 / actionButton4 -> ACT_MoveCategory(IsIncrement = false) => Order - 1 + * + * Data contract: all sibling categories start from the same Order value (the shipped + * data seeds every Category with Order = 0). One "+" click therefore moves a node past + * all of its siblings to the last position, and one "-" click on it restores the + * original position. `assertSingleOrderBaseline` fails loudly if that stops holding. + * + * Order is persisted, and the whole suite shares one Mendix runtime, so this file runs + * serially and every test restores the Order value it changed. + * + * Note: the action buttons sit inside the node header, which is also the expand/collapse + * target ("Open node on: Header click"), so a reorder click toggles the clicked node as + * well. That is configuration, not a defect — assertions below deliberately only depend + * on the state of ancestors and siblings of the clicked node. + */ + +const TREES = { + startCollapsed: { + root: ".mx-name-treeNode1", + label: ".mx-name-text2", + moveDown: ".mx-name-actionButton1", + moveUp: ".mx-name-actionButton2" + }, + startExpanded: { + root: ".mx-name-treeNode2", + label: ".mx-name-text3", + moveDown: ".mx-name-actionButton3", + moveUp: ".mx-name-actionButton4" + } +}; + +function rows(scope) { + return scope.locator(":scope > li"); +} + +function header(row) { + return row.locator(":scope > .widget-tree-node-branch-header"); +} + +function labels(scope, tree) { + return scope.locator(`:scope > li > .widget-tree-node-branch-header ${tree.label}`); +} + +function group(row) { + return row.locator(":scope > .widget-tree-node-body > ul[role='group']"); +} + +function escapeForRegExp(value) { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** Matches a header label by node name only, ignoring the "{Order}) " prefix. */ +function byName(name) { + return new RegExp(`\\)\\s*${escapeForRegExp(name)}$`); +} + +function nameOf(labelText) { + return labelText.replace(/^\s*-?\d+\)\s*/, "").trim(); +} + +function orderOf(labelText) { + const match = /^\s*(-?\d+)\)/.exec(labelText); + return match ? match[1] : null; +} + +/** + * Reads the sibling labels once the list is rendered. Used to derive the expected + * permutation; every assertion afterwards runs through a retrying locator assertion. + */ +async function readSiblings(labelsLocator) { + await expect(labelsLocator.first()).toBeVisible(); + return (await labelsLocator.allTextContents()).map(text => text.trim()); +} + +function assertSingleOrderBaseline(siblingLabels) { + const orders = new Set(siblingLabels.map(orderOf)); + expect( + orders.size, + `Siblings must all start from the same Order value for a single +/- click to move a node ` + + `past them. Found: ${siblingLabels.join(" | ")}` + ).toBe(1); +} + +function assertEnoughSiblings(siblingLabels, what) { + expect(siblingLabels.length, `The advanced page needs at least 2 ${what} to test reordering`).toBeGreaterThan(1); +} + +/** Moves the first sibling to the end, then back, asserting both states. */ +async function moveFirstToEndAndBack(scope, tree, names, extraAssertions) { + const siblingLabels = labels(scope, tree); + + await header(rows(scope).first()).locator(tree.moveDown).click(); + await expect(siblingLabels).toHaveText([...names.slice(1), names[0]].map(byName)); + if (extraAssertions) { + await extraAssertions(); + } + + await header(rows(scope).last()).locator(tree.moveUp).click(); + await expect(siblingLabels).toHaveText(names.map(byName)); +} + +async function openAdvancedPage(page) { + await page.goto("/p/treenodev2_advanced"); + + const gallery = page.locator(".mx-name-gallery1"); + await expect(gallery).toBeVisible(); + await gallery.getByRole("option", { name: "HR" }).click(); + await waitForDataReady(page); + await expect(labels(page.locator(TREES.startCollapsed.root), TREES.startCollapsed).first()).toHaveText( + byName("Electronics") + ); +} + +async function openCategoryOverview(page) { + await page.getByRole("menuitem", { name: "Master Data" }).click(); + await page.getByRole("button", { name: "Category", exact: true }).click(); + await expect(page.getByRole("button", { name: "New Category" })).toBeVisible(); +} + +async function setCategoryDepartment(page, category, department) { + await page + .getByRole("row", { name: new RegExp(`^${category}\\s+`) }) + .locator(".mx-name-actionButton2") + .click(); + const dialog = page.getByRole("dialog", { name: "Edit Category" }); + await expect(dialog).toBeVisible(); + await dialog.getByRole("combobox", { name: "Department" }).click(); + await dialog.getByRole("listbox", { name: "Department" }).getByRole("option", { name: department }).click(); + await dialog.getByRole("button", { name: "Save" }).click(); + await expect(dialog).toBeHidden(); +} + +test.describe.configure({ mode: "serial" }); + +test.describe("v2: reordering (advanced page)", () => { + test.beforeEach(async ({ page }) => { + await openAdvancedPage(page); + }); + + test("visual regression: start-collapsed tree", async ({ page }) => { + const widget = page.locator(TREES.startCollapsed.root); + await expect(widget).toBeVisible(); + await expect(widget).toHaveScreenshot("treeNodeV2AdvancedCollapsed.png"); + }); + + test("visual regression: start-expanded tree", async ({ page }) => { + const widget = page.locator(TREES.startExpanded.root); + await expect(widget).toBeVisible(); + await expect(widget).toHaveScreenshot("treeNodeV2AdvancedExpanded.png"); + }); + + test.describe("reordering", () => { + test.beforeEach(async ({ page }) => { + await openCategoryOverview(page); + await setCategoryDepartment(page, "Laptops", "HR"); + await setCategoryDepartment(page, "Clothing", "HR"); + await openAdvancedPage(page); + await expect(rows(page.locator(TREES.startCollapsed.root))).toHaveCount(2); + await expect(rows(group(rows(page.locator(TREES.startExpanded.root)).first()))).toHaveCount(2); + }); + + test.afterEach(async ({ page }) => { + await openCategoryOverview(page); + await setCategoryDepartment(page, "Laptops", "IT"); + await setCategoryDepartment(page, "Clothing", "IT"); + }); + + test("reorders root nodes while the tree is collapsed @smoke", async ({ page }) => { + const tree = TREES.startCollapsed; + const treeRoot = page.locator(tree.root); + + await expect(rows(treeRoot).first()).toHaveAttribute("aria-expanded", "false"); + + const baseline = await readSiblings(labels(treeRoot, tree)); + assertEnoughSiblings(baseline, "root categories"); + assertSingleOrderBaseline(baseline); + + await moveFirstToEndAndBack(treeRoot, tree, baseline.map(nameOf)); + }); + + test("reorders children of an expanded node without collapsing it", async ({ page }) => { + const tree = TREES.startCollapsed; + const treeRoot = page.locator(tree.root); + const parentRow = rows(treeRoot).first(); + const parentHeader = header(parentRow); + + await expect( + parentHeader.locator(".widget-tree-node-branch-header-icon-container"), + "The first root category must have children for this test" + ).toBeVisible(); + + // Expand through the icon container — it carries no action button of its own. + await parentHeader.locator(".widget-tree-node-branch-header-icon-container").click(); + await expect(parentRow).toHaveAttribute("aria-expanded", "true"); + + const childGroup = group(parentRow); + const baseline = await readSiblings(labels(childGroup, tree)); + assertEnoughSiblings(baseline, "child categories under the first root"); + assertSingleOrderBaseline(baseline); + + // Every root that showed an expand affordance must still show it after the + // datasource redelivery triggered by the reorder microflow. + const rootIcons = treeRoot.locator( + ":scope > li > .widget-tree-node-branch-header .widget-tree-node-branch-header-icon-container" + ); + const rootIconCount = await rootIcons.count(); + + await moveFirstToEndAndBack(childGroup, tree, baseline.map(nameOf), async () => { + await expect(parentRow).toHaveAttribute("aria-expanded", "true"); + await expect(rootIcons).toHaveCount(rootIconCount); + }); + + await expect(parentRow).toHaveAttribute("aria-expanded", "true"); + }); + + test("reorders children of an auto-expanded node when start expanded is on", async ({ page }) => { + const tree = TREES.startExpanded; + const treeRoot = page.locator(tree.root); + const parentRow = rows(treeRoot).first(); + + await expect(parentRow).toHaveAttribute("aria-expanded", "true"); + + const childGroup = group(parentRow); + const baseline = await readSiblings(labels(childGroup, tree)); + assertEnoughSiblings(baseline, "child categories under the first root"); + assertSingleOrderBaseline(baseline); + + const spinners = treeRoot.locator(".widget-tree-node-loading-spinner"); + + await moveFirstToEndAndBack(childGroup, tree, baseline.map(nameOf), async () => { + await expect(parentRow).toHaveAttribute("aria-expanded", "true"); + await expect(spinners).toHaveCount(0); + }); + + await expect(parentRow).toHaveAttribute("aria-expanded", "true"); + await expect(spinners).toHaveCount(0); + }); + }); +}); diff --git a/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js-snapshots/treeNodeV2AdvancedCollapsed-chromium-linux.png b/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js-snapshots/treeNodeV2AdvancedCollapsed-chromium-linux.png new file mode 100644 index 0000000000..4d4ae870a5 Binary files /dev/null and b/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js-snapshots/treeNodeV2AdvancedCollapsed-chromium-linux.png differ diff --git a/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js-snapshots/treeNodeV2AdvancedExpanded-chromium-linux.png b/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js-snapshots/treeNodeV2AdvancedExpanded-chromium-linux.png new file mode 100644 index 0000000000..3a14033e9b Binary files /dev/null and b/packages/pluggableWidgets/tree-node-web/e2e/TreeNodeV2Reorder.spec.js-snapshots/treeNodeV2AdvancedExpanded-chromium-linux.png differ diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/.openspec.yaml b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/.openspec.yaml new file mode 100644 index 0000000000..96db9a43b6 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-15 diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/design.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/design.md new file mode 100644 index 0000000000..f1c5b93f74 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/design.md @@ -0,0 +1,250 @@ +## Context + +`TreeNode.tsx` (root dispatcher) routes to v2 (`src/components/v2/`) whenever `parentAssociation` is configured — the self-referencing "infinite tree" mode this ticket's repro projects use. `TreeNode.editorConfig.ts:38-39` hides the `hasChildren` widget property from Studio Pro whenever `parentAssociation` is set, and it has no XML default value — so on every v2 instance, `props.hasChildren` is `undefined` at runtime. (Confirmed live: an initial attempt to read it crashed the widget with `TypeError: Cannot read properties of undefined (reading 'get')`.) v2 must derive "does this node have children" structurally, from `node.children.length > 0` — there is no other signal available to it. + +Why this requires fetching at all, rather than a cheap existence check: Mendix's pluggable-widget data API (`ListValue`) gives a widget exactly one shared, filter-driven list — there is no lighter-weight "does association X have any related record" or per-item count primitive exposed to pluggable widgets. Determining "does node N have children" means asking the datasource for items whose parent is N and checking whether anything comes back; there's no way to get that answer without the datasource actually returning (at least) the matching item(s). This is why the whole preload mechanism (`loadedParentsByIdRef`/`loadedChildsByIdRef`, both in `appendItems` and the bootstrap effect below) exists at all — it's not a workaround for a missed optimization, it's the only mechanism this API surface provides for v2's configuration mode. + +`useIncrementalTreeData.ts:114-119` (pre-fix) flipped a node out of `TreeNodeState.LOADING` the moment its id reappeared in _any_ subsequent `items` delivery — not specifically a delivery meant to answer "does this node have children." A microflow datasource (`useInfiniteTreeNode.ts`) always redelivers its full flattened result regardless of the filter passed to `datasource.setFilter(...)`, since microflow datasources ignore filters entirely. Two consequences, confirmed via live instrumented repro against customer-attached repro projects for WC-3564: + +- **Bug 1**: with "Start expanded" = Yes, a node stays `LOADING` forever if the only mechanism meant to resolve it (a filtered re-delivery) never distinguishes "answered" from "not yet answered" — the spinner never clears. +- **Bug 2**: with "Start expanded" = No, expanding any node triggers `appendItems` → `setFilter`, which (because the microflow ignores the filter) redelivers everything, including the ids of _unrelated_, not-yet-clicked nodes. Those nodes flip out of `LOADING` per lines 114-119, landing in `COLLAPSED_WITH_JS` with an empty `children` array — permanently killing their icon, since the icon-render condition (`TreeNode.tsx:49`, pre-fix) was `hasChildren || treeNodeState === LOADING` and neither is true anymore. + +Git history (`dcbf9bdb51`, "fix: add empty message, loading, and keyboard nav") shows `LOADING` was added later, purely for polish: before that commit, a new node was created directly as `EXPANDED`/`COLLAPSED_WITH_JS`, and a node that would turn out to have children simply showed no icon for one render until its children got placed — a minor "icon pops in" flicker. `LOADING` was introduced to bridge that instant with a spinner instead of nothing, but the _resolution_ criterion it shipped with was wrong, and that wrong criterion is the root cause of both WC-3564 bugs. + +## Goals / Non-Goals + +**Goals:** + +- Expand-icon visibility for v2 must never depend on a widget property that is architecturally unavailable in v2's own configuration mode (`hasChildren`). +- A node's spinner state must never be "stuck" (Bug 1) or capable of corrupting an unrelated node's state (Bug 2). +- Preserve the original polish goal (avoid an abrupt icon pop-in) using a signal that cannot exhibit either bug. +- Both WC-3564 bugs fixed by the same underlying mechanism (not two separate patches). +- The preload filter must stay correct after the datasource's entire result set is replaced by an app-level constraint (a gallery filtering by department, a changed page parameter), not only across incremental deliveries of the same set. + +**Non-Goals:** + +- Not changing v1 behavior — v1 correctly reads `hasChildren` (it has no association-based alternative) and is untouched by this change. +- Not fixing incorrect Studio Pro configuration of `hasChildren` — irrelevant to v2 now, since v2 never reads it. +- Not changing how `useIncrementalTreeData` renders an orphan — an item whose `parentId` is set but whose parent the datasource does not deliver. It is still promoted to root level, because that promotion is what makes out-of-order delivery (child before parent) work. D6 records the consequence. +- Not guaranteeing a spinner ever shows for every conceivable timing gap — the fix only guarantees the spinner is never stuck and never corrupts a sibling; if the datasource's `status` never reports `Loading` for a given fetch, no spinner shows for it (functionally harmless, matches original pre-`LOADING`-commit behavior). + +## Decisions + +### D1 (revised): `hasChildren` stays derived from `node.children.length > 0`; `props.hasChildren` is never read in v2 + +Initial design used `props.hasChildren.get(node.item).value` as the icon-visibility source, on the premise that the prop was simply unused, not unusable. Live testing against the actual WC-3564 repro project crashed the widget (`props.hasChildren` is `undefined` for any v2 instance — see Context). Reverted: `TreeNode.tsx`'s `hasChildren` local goes back to `node.children.length > 0`, exactly as before this ticket. `renderRecursiveNode` no longer threads a `hasChildren` expression parameter at all. + +This is safe against both bugs once D2 (below) lands, because `node.children` is only ever mutated by real, structurally-correct placement (`useIncrementalTreeData.ts`'s `placeNode`) — there is no longer a spurious "resolve" step that can zero out a node's children array based on unrelated data. + +### D2 (revised): `LOADING` is a render-time-only spinner decision, never stored per node + +A newly-created node is created directly as `EXPANDED`/`COLLAPSED_WITH_JS` (per `config.startExpanded`) in `useIncrementalTreeData.ts` — `TreeNodeState.LOADING` is never assigned to `treeNodeState` anywhere in that file, and the click handler in `TreeNode.tsx` goes back to unconditionally setting `EXPANDED` (matching pre-`LOADING`-commit behavior; no guard needed since nothing sets `LOADING` on click anymore). + +The spinner is computed fresh on every render in `TreeNode.tsx`: `showSpinner = node.children.length === 0 && props.datasource.status === ValueStatus.Loading`. `datasource.status` is Mendix's own first-party, real-time "is this datasource actually fetching right now" signal (`ListValue.status: ValueStatus`) — not bookkeeping we maintain ourselves. `renderHeaderIcon` receives `showSpinner ? TreeNodeState.LOADING : node.treeNodeState`, so `LOADING` still exists as an icon-rendering signal (satisfying the original "repurpose, don't remove" call), just never persisted on the node. + +This is structurally immune to both bugs: + +- **Bug 1 can't recur**: nothing is ever "waiting" in a stored sense. The spinner shows exactly while `status` says `Loading`, and clears the instant it doesn't — including the case where a microflow's `setFilter` call is a genuine no-op and `status` never even transitions to `Loading` (spinner correctly never shows, rather than showing forever). +- **Bug 2 can't recur**: there's no per-node mutable "resolved" flag to wrongly flip. Every render recomputes `hasChildren` fresh from the current `children` array and `showSpinner` fresh from the single global `status` flag — one node's expand action can only ever change _its own_ children array (via real placement) or the shared `status` (which, if it flips, affects all unresolved nodes' spinners equally and correctly, not selectively/incorrectly). + +**Alternatives considered**: + +- _Track per-node fetch-request ids from `useInfiniteTreeNode.ts` and gate resolution on that._ Rejected — solves the wrong layer; still lets a node start `LOADING` and wait indefinitely if the tracked request never resolves (Bug 1's actual mechanism), and adds bookkeeping complexity for no additional correctness over the `datasource.status` approach. +- _Resolve `LOADING` immediately whenever a node's first child is placed, entered via click when `children.length === 0`._ Rejected — requires guarding the click handler against genuine leaves (ambiguous: `children.length === 0` means both "confirmed leaf" and "real parent not yet preloaded," indistinguishable from node state alone), and turned out to be moot anyway once `hasChildren` was reverted to being structurally-derived (a node is only ever clickable once its children are already known, via the existing one-level-lookahead preload — see Context). +- _Drop `LOADING`/the spinner entirely._ Considered when it looked like the preload design left no genuine "waiting" window at all. Rejected per explicit user request to keep a spinner "just in case" — `datasource.status` gives a correct way to do that without reintroducing either bug. + +### D3 (added — found during manual verification, not part of the original two bugs): the one-level-lookahead preload had two gaps, both closed + +While verifying D1/D2 live, manual testing surfaced that a node's expand affordance for a _deeper_ tier sometimes didn't appear until the user collapsed and re-expanded a node — a real, pre-existing bug on `main`, unrelated to the `LOADING`/`hasChildren` mechanism above (it lives entirely in `useInfiniteTreeNode.ts`, which D1/D2 never touch). Two separate gaps in the same "preload one level past what's currently expanded" mechanism: + +- **Click-driven gap** (`appendItems`): the grandchildren-preload step (`children.forEach(...)`, adding a node's children to `loadedChildsByIdRef` so _their_ children get fetched too) was nested inside `if (loadedParentsByIdRef.current.has(parentId))` — true only from a node's _second_ expand onward. A node's first-ever expand skipped preloading its children's children, so a deeper tier's expand icon only appeared after a collapse+re-expand. **Fix**: removed that outer gate — the preload step now always runs when children are passed in, regardless of whether this is the first or a later expand. Verified live: a single click now reveals a 4th tier that previously needed collapse+re-expand. +- **Bootstrap-path gap** (the second `useEffect`, `startExpanded = Yes` specifically): root nodes auto-expand via a separate path that populates `loadedParentsByIdRef` directly from `datasource.items`, bypassing `appendItems` entirely — so the click-driven fix above doesn't reach them. This path was also capped at exactly one automatic round (`if (loadedParentsByIdRef.current.size === 0)`, true only once ever), so it preloaded roots' children but never went one level further. + + **First attempt, reverted after breaking live**: replaced the one-shot gate with a `bootstrapRoundRef` counter that advanced unconditionally on every effect firing, capped at 2. Passed unit tests against a mocked datasource, but broke the real "Expanded bug" tab live — the tree stopped rendering anything past the root level. Root-caused with temporary per-widget-tagged debug instrumentation (three tree widgets mount simultaneously on that page regardless of which tab is active, so untagged logs were unreadable): both rounds fired, and _locked themselves in_, while `datasource.items` was still transiently empty during initial load — before the real root items ever arrived. A blind counter can't distinguish "this effect fired" from "this effect fired with something worth preloading"; it burned both capped rounds on nothing, permanently disabling the mechanism. + + **Second attempt**: replaced the counter with two content-based flags (`round1DoneRef`, `round2DoneRef`) that only flip once real, not-yet-tracked items are actually found — mirroring the pre-existing round-1 gate's own self-correcting semantics (`loadedParentsByIdRef.current.size === 0`, checked _after_ attempting to populate: harmlessly retries on an empty delivery, locks in only once real data lands). Round 2 only locks in once its scan finds at least one item that isn't already a known parent or child. Verified live: the "Expanded bug" tab (2-tier test data at the time) showed all tiers automatically on load, with no regressions to Bug 1/Bug 2/the `appendItems` fix. + + **Third attempt (final, kept)**: against a deeper (4-tier) dataset, the 2-round cap turned out insufficient — the 3rd tier appeared as visible content but without its own expand icon, needing one more real click on an ancestor to reveal, one level deeper than the original repro exposed. Root insight: under `startExpanded = Yes`, _every_ level defaults to `EXPANDED`, not just roots (`useIncrementalTreeData.ts`'s node-creation branch — see D2) — so every level needs the same automatic one-level-lookahead treatment, not a fixed count of 2. The original "avoid eagerly walking the whole tree" concern (below) doesn't actually apply to `startExpanded = Yes`: since nothing is collapsed in that mode, walking the whole tree _is_ the correct, intended behavior — bounded by the tree's real depth (a finite, self-terminating cascade), not an unbounded/runaway one. Fixed by replacing the 2-round cap with an unbounded cascade, gated specifically on `startExpanded === true`: keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants keep appearing; stop once a round finds nothing new. `startExpanded = false` keeps the original capped round1+round2 behavior — for that mode, deeper tiers are still collapsed by default and resolve correctly via a single real click already (group 6's fix), so auto-cascading further would only be wasted eager-fetching of not-yet-visible content. Verified live: the "Expanded bug" tab (now the real 4-tier dataset) shows every tier automatically, matching the exact result the user originally showed as expected; the "Collapsed bug" tab's 2-round-capped behavior is unchanged and still passes. + +**Alternative considered**: apply the unbounded cascade regardless of `startExpanded`. Rejected per explicit user decision — would eagerly prefetch descendants of branches that are still collapsed and not visible under `startExpanded = false`, for no user-visible benefit (those tiers already resolve correctly in a single click once actually expanded). +**Alternative considered** (superseded by the "why can't we just know without fetching" question — see Context below): skip preloading and derive "has children" from a cheaper existence check. There is no such check available — see Context. + +### D4 (added — folding in `tmp/treenode-fix1`): the incremental map re-applies datasource order and remembers expansion state; `LOADING` is not reintroduced to carry it + +A parallel branch fixed two more bugs in `useIncrementalTreeData.ts`, both consequences of the same "build incrementally, reuse nodes across updates" design that D2 also lives in. Folding them in here rather than merging separately, because both branches rewrote the same two functions — the reconciliation below is a design decision, not a textual merge. + +**(a) Datasource order was captured once, never re-applied.** A node is appended to `rootsRef`/`parent.children` only on the first update its id appears in; on later updates it takes the "already exists" branch, which refreshes `item`, `title` and `parentId` but never touches sibling arrays. So a datasource that re-delivers the same items in a new order (the real case: a microflow updates a sequence attribute the datasource sorts on) leaves the tree in its first-load order until the page is reopened and the widget remounts. **Fix**: build an `orderById` map from the current delivery's index, then sort `rootsRef` and every node's `children` by it at the end of each update. Nodes whose id is absent from the current delivery sort to the end (`?? sourceItems.length`), keeping their relative order among themselves rather than being shuffled — a delivery that expresses no order for a node should not reorder it. + +Chosen over re-inserting at the right index during placement: placement is order-independent by design (children can arrive before parents — see the out-of-order handling), so a positional insert would need to be re-derived anyway once the parent shows up. One sort at the end is both simpler and the only point where the full delivery order is actually known. + +**(b) A refresh collapsed every node.** Three things trigger a full rebuild of the node map, and a rebuilt node started collapsed: + +1. `items === undefined` while the datasource loads. The hook read `items ?? []`, which made every previously-known id look removed, tripping `removedIdsDetected` → rebuild → empty tree → "no data available" flashes mid-load. +2. `isConfigChanged` compares prop instances by reference, and the Mendix client hands over new instances on every refresh — so this fires on _every_ refresh, not just genuine configuration changes. +3. Any single item deletion legitimately trips `removedIdsDetected`. + +Cause 1 is fixed at the root: return early when `items` is undefined and keep the tree, which also removes the mid-load empty message. Causes 2 and 3 are _not_ avoided — rebuilding is correct for them, and making `isConfigChanged` structural would mean deep-comparing `ListExpressionValue`/`ListReferenceValue` instances that have no meaningful value equality. Instead the rebuild is made non-destructive: snapshot every node's `treeNodeState` into a `statesByIdRef` map keyed by item id just before clearing, and have node creation prefer a remembered state over the `startExpanded` default. + +**Reconciliation with D2 — the one real conflict.** The parallel branch expressed the restore through a `resolveRestoredState(remembered, startExpanded)` helper whose "nothing remembered" arm returned `TreeNodeState.LOADING`, and whose second arm resolved a _remembered_ `LOADING` into `EXPANDED`/`COLLAPSED_WITH_JS`. Both arms are wrong here, in opposite ways: the first reintroduces stored `LOADING` — exactly WC-3564 Bug 1's mechanism — and the second is dead code, since after D2 no node ever holds `LOADING` to remember. Resolved by dropping the helper entirely; the restore collapses to a single expression on the node-creation branch: + +``` +treeNodeState: statesByIdRef.current.get(nodeId) ?? (config.startExpanded ? EXPANDED : COLLAPSED_WITH_JS) +``` + +The remembered state deliberately wins over `startExpanded` in both directions: a node the user collapsed under "Start expanded" = Yes must stay collapsed across a refresh, which is the whole point of remembering. + +Note that (b) and D2 are complementary rather than overlapping, and it is worth being precise about which bug each one owns, since both are "the tree looks wrong after a refresh": D2 stops a _spinner_ from being stuck on a node; (b) stops a node's _expansion_ from being lost. D2 alone still collapsed the tree on refresh; (b) alone still left spinners stuck. Together, the early return from (b) also makes D2's spinner correct during the load window — the tree stays mounted, so `status === Loading` has real nodes to render a spinner on instead of an empty message. + +### D5 (added — folding in `tmp/treenode-fix1`): the bootstrap effect's round gates must not `return`, or they swallow the late-arrival sweep + +The parallel branch also extended `useInfiniteTreeNode.ts` with a third preload mechanism: track the ids of nodes the user has expanded (`expandedIdsRef`, populated in `appendItems`), and on each subsequent update sweep `datasource.items` for any item whose `parentId` is an expanded id and which isn't tracked in either map yet, preloading it. This covers children that were still in flight when `appendItems` ran (so it received none) and children created later by a microflow — neither of which any existing mechanism reached. + +That sweep and D3's `round1DoneRef`/`round2DoneRef` gates collide, and the collision is invisible to inspection. D3 replaced the original `if (loadedParentsByIdRef.current.size === 0)` gate — which was skipped whenever `appendItems` had already populated the map — with a flag that fires on the _first_ post-init update regardless, and `return`s. The sweep sits after that return. Concretely, for `startExpanded = false`: + +``` +render init -> setFilter(root-only) +appendItems("parent") -> setFilter([root, "parent"]) ("parent" now a loaded parent) +items = [parent, child] arrives + round1Done? no + loadedParents.size === 0? no -> skip populate, size > 0, round1Done = true, setFilter, RETURN + sweep never runs -> filter still [root, "parent"], "child" never preloaded +``` + +**Fix**: no early `return` from round1 or round2. Each mechanism sets a single `shouldRefilter` flag, and one `setFilter` call happens at the end of the pass. Round1 and round2 stay mutually exclusive (`else if`) since round2's premise is that round1's fetch has landed; the sweep runs unconditionally after them. + +The three mechanisms stay distinct and none is redundant: round1 fetches roots' children (roots never pass through `appendItems`, so the sweep cannot see them), round2 preloads one level past that, and the sweep handles everything arriving after a real expand. D3's cascade for `startExpanded = true` is untouched and still returns early — it already treats every newly-arrived item as a loaded parent, which subsumes the sweep for that mode. + +**`appendItems` reconciliation.** Both branches rewrote it and removed the same outer gate; the end state is equivalent apart from two details. Kept D3's shape (it is what this change's `CONTEXT.md` documents) plus the parallel branch's `if (!loadedParentsByIdRef.current.has(childId))` guard on the child loop — without that guard, a child that was expanded earlier and then collapsed ends up in `loadedParentsByIdRef` _and_ `loadedChildsByIdRef`, producing a duplicate parent id in the preload filter. Harmless today, but it makes the filter's contents no longer a set, which is what one of the new tests asserts. + +**Known gap, deliberately not widened.** Root nodes are never added to `expandedIdsRef` under `startExpanded = false`, since they auto-expand via the bootstrap path rather than `appendItems`. So a child added to a _root_ after round2 has locked in is not swept. Pre-existing in the parallel branch too, out of scope here; fixing it means deciding whether the bootstrap path should register roots as "expanded", which changes what the sweep costs on wide trees. + +### D6 (added — found live during this change's own verification pass; supersedes D3's round mechanism and D5's restructure): the preload parent set is _derived_ from the current tree, never accumulated from delivery history + +**How it was found.** Live on the `treenodev2_advanced` test page: two v2 trees over the same data side by side (`.mx-name-treeNode1` with "Start expanded" = No, `.mx-name-treeNode2` with Yes), plus a gallery that filters the datasource by department. Selecting a department replaces the datasource's entire result set. After any such switch, every node in `treeNode1` renders permanently without an expand affordance — and inert in the full sense: no icon, no `aria-expanded`, no `widget-tree-node-branch-header-clickable`, and `onKeyDownHandler` gated on `hasChildren`. The user has no way left to open it. `treeNode2`, over the same data, is correct. + +``` +FRESH LOAD (no department selected) +treeNode1 (startExpanded = No) treeNode2 (startExpanded = Yes) +[false] Electronics :: chevron [true] Electronics :: chevron + [false] Phones :: chevron [true] Phones :: chevron + [null] iOS :: no icon [null] iOS :: no icon + [null] Tablets :: no icon [null] Tablets :: no icon + +AFTER selecting department "Finance" +t1: Books | Tablets | Android | Sports t2: Books | Tablets | Android | Sports[chevron] + ^ all four inert, all at root level └ Fitness Equipment + +AFTER selecting department "IT" +t1: Clothing | Laptops | Home & Garden t2: Clothing[chevron] | Laptops | Home & Garden + ^ all three inert ├ Women | Non-Fiction | Team Sports + └ Men's Clothing +``` + +`treeNode1`'s post-switch result set is exactly `department = X AND parent ∈ {undefined, Electronics, Phones}` — the filter from the _initial_ load, frozen. `treeNode2` is correct only incidentally: its unbounded "Start expanded" = Yes cascade (D3) had already grown its own filter to include `Books`/`Sports`, so its retrieve happened to cover the new roots. Same widget, same data, different filter age. + +**Two defects, one cause.** Every ref in `useInfiniteTreeNode.ts` is an append-only log scoped to the widget's mount: + +- **(A) The rounds never re-arm.** `round1DoneRef`/`round2DoneRef` are one-shot for the widget's lifetime. On a replaced result set the effect skips both, the late-arrival sweep finds nothing (nothing was user-expanded), `shouldRefilter` stays false, and `setFilter` is never called — so the new roots' children are never requested and `node.children.length === 0` forever. This is the reported bug. +- **(B) The maps are never pruned.** `loadedParentsByIdRef`/`loadedChildsByIdRef`/`expandedIdsRef` only ever grow, so the filter is simultaneously stale and monotonically larger. Beyond the wasted retrieve, this is why `Tablets`/`Android`/`Laptops` appear at all under a department that excludes their parents — and, their parents being absent from the delivery, `useIncrementalTreeData`'s orphan promotion renders them at root level next to genuine roots. + +**Why not simply re-arm the rounds.** That would be the fourth delivery-history heuristic in the same mechanism. This change's own `## Context` condemns the original bug as "resolve a node's state when its id reappears in some later delivery" — a history signal standing in for a derived fact. D3 and D5 then replaced it with two more history flags (`round1DoneRef`, `round2DoneRef`) and a third map (`expandedIdsRef`). Bolting a "was the result set replaced?" detector on top preserves the exact shape that has now produced four bugs, and every such detector is a heuristic in its own right (`incoming ∩ previous === ∅` is wrong the moment two departments share an item). The lesson this decision records: **derive the filter from the current tree; do not log what has already been fetched.** + +**The rule.** One invariant replaces all of it: + +> For every node that is **visible**, or **one expand away from visible**, the widget must know whether it has children. + +``` +rendered(N) := N.treeNodeState is EXPANDED or COLLAPSED_WITH_CSS +visible(N) := every ancestor of N is rendered (a true root is always visible) + +desiredParents = {undefined} <- always: fetch true roots + ∪ {N : visible(N)} <- so every visible node's icon is correct + ∪ {N : visible(parent(N))} <- lookahead: icons are right before the expand lands +``` + +`setFilter` is called only when `desiredParents` differs from the set last applied. + +**Why `rendered` and not simply `EXPANDED`** (found while implementing, not while designing): `COLLAPSED_WITH_CSS` means a node was opened and then closed, and per D1 its body stays in the DOM, hidden by CSS. If a collapse narrowed `visible`, the derived set would shrink, the next delivery would no longer carry the already-fetched grandchildren, the removed-ids check would rebuild the tree without them, and re-expanding the node would show its children with no expand icons — a fresh instance of the very defect this decision fixes. A collapse must therefore not change the set at all, which also means the click handler only re-derives on expand. `COLLAPSED_WITH_JS` (never opened, body never rendered) does stop the recursion, and that is the term that keeps `startExpanded = No` from eagerly walking the whole tree. + +What that single rule subsumes: + +| Mechanism it replaces | Why the rule already covers it | +| --------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `round1DoneRef` (preload roots' children) | nothing expanded ⇒ visible = roots ⇒ set is `{undefined} ∪ roots ∪ children(roots)`. Identical result, no flag. | +| `round2DoneRef` (one level past that) | the same term of the same expression. | +| D3's `appendItems` grandchildren preload | expanding R makes R's children visible ⇒ their children enter the set. Falls out. | +| D5's late-arrival sweep | the set is recomputed from the current delivery every time, so a late child is picked up by the delivery that carries it. Falls out. | +| D3's unbounded cascade for "Start expanded" = Yes | every level is `EXPANDED` ⇒ every delivered node is visible ⇒ set = all delivered ∪ their children. Same cascade, same self-termination, no special case. | +| D5's known gap (roots never enter `expandedIdsRef`) | there is no `expandedIdsRef`, and root-ness is not a special case of expandedness. | +| **(A) and (B) above** | the set is a function of the current tree: a replaced result set yields a new set on the first delivery, and an id that left the delivery leaves the set. Nothing to re-arm, nothing to prune. | + +Refs deleted: `loadedParentsByIdRef`, `loadedChildsByIdRef`, `expandedIdsRef`, `round1DoneRef`, `round2DoneRef`. Refs remaining: `initializedRef`, plus a new `lastAppliedKeyRef` (the sorted ids of the last applied set, as one string). Five pieces of history collapse into one idempotence guard. + +The rule is implemented as `deriveDesiredParentIds(treeData, deliveredIds)` in `hooks/helpers.ts` — pure, so it is unit-tested directly. It deliberately takes no `startExpanded` argument: `treeNodeState` already encodes it (D2 creates nodes `EXPANDED` under `startExpanded = Yes`), and reading the prop as well would let the two sources disagree once the user collapses something. + +**"Root" means `parentId === undefined`, not "parent absent from this delivery"** — per explicit user decision. The two candidate definitions differ only for orphans, and they differ materially: + +``` +(a) root := parentId is undefined [CHOSEN] + Finance ⇒ desiredParents = {undefined, Books, Sports, ...their children} + Tablets/Android/Laptops are never requested ⇒ they leave the tree on the next + delivery. One settle step. + Cost: an orphan is never a requested parent, so it renders at root level with no + affordance even if it does have children. + +(b) root := parent absent from this delivery ("pseudo-root") + Finance ⇒ Tablets counted as a root ⇒ its children requested ⇒ next delivery drops + Tablets itself (Electronics is not in the set) ⇒ Tablets leaves the set the round + after. Two settle steps: the row visibly flickers in and back out, and one retrieve + is spent on a node that is about to disappear. + Gain: a genuine orphan does get its affordance. +``` + +(a) is chosen: an item whose parent the app will not deliver is not really a tree root, and spending a retrieve plus a visible flicker to give it an affordance dignifies an accident of configuration. The consequence is recorded as a trade-off below, not hidden. + +**Architecture: the feedback loop becomes explicit.** Visibility lives on `treeNodeState`, which lives in `useIncrementalTreeData` — so the filter has to be derived from the built tree. Today's code emulates that loop through an append-only log; D6 makes it a loop on purpose: + +``` + datasource.items ──► useIncrementalTreeData ──► treeData (nodes + EXPANDED flags) + ▲ │ + │ ▼ + setFilter ◄──── deriveDesiredParents(treeData, startExpanded) + │ + guard: skip while items === undefined + guard: skip when the set equals lastAppliedParentIds +``` + +`TreeNodeV2` consequently calls `useIncrementalTreeData(props.datasource.items, treeConfig)` directly and passes the resulting `treeData` into the preload hook, instead of threading `items` out of it — `useInfiniteTreeNodes` returns `datasource.items` verbatim today, so that indirection buys nothing once the hook no longer owns the item flow. The file keeps its name so the diff stays legible against D3/D5. + +**Why the init block survives.** The one-time init (`initializedRef`) still applies a root-only filter when "Start expanded" is No, and still applies _no_ filter when it is Yes. That asymmetry is load-bearing: for a non-microflow datasource with "Start expanded" = Yes, the unfiltered first delivery returns the whole table in a single retrieve, and the derived set computed from it is immediately stable (everything delivered is expanded, so everything is already a desired parent) — one confirming `setFilter`, then silence. Deriving from an empty tree instead would start at `{undefined}` and walk the tree down one retrieve per level, trading one round trip for depth-many. + +**Convergence.** Membership in `desiredParents` depends on a node's _ancestry_ only, never on its descendants, so the set cannot feed itself: requesting P's children can add P's children to the set, but never changes P's own membership. Combined with the equality guard, that bounds the loop: + +- Normal datasource: the set grows only as the user expands. Each delivery recomputes to the same set ⇒ one `setFilter`, then silence. +- Microflow datasource (WC-3564's villain, ignores `setFilter` and redelivers everything): the tree is full, the set derived from it is stable, the one `setFilter` is ignored, the next delivery is identical ⇒ no further calls. No loop and no stuck state — strictly better than D3's `addedAny` termination, which depended on deliveries differing. +- Shrinking (a department switch): the set shrinks once and settles, per the ancestry-only argument above. + +**Stale `ObjectItem`s.** `setFilter` needs real `ObjectItem`s for `literal()`, and `useIncrementalTreeData` deliberately keeps nodes the current delivery did not mention (D4a). The derived set is therefore built only from ids present in the current delivery, so a node holding an `ObjectItem` from an older delivery never reaches `literal()`. + +**Expansion stays a mutation.** The click handler mutates `node.treeNodeState` and calls `forceRender`, so an effect keyed on `treeData` does not re-fire on expand; the handler calls the recompute imperatively instead. `appendItems(newItem, children)` therefore becomes an argument-less `syncPreloadFilter()` — the expansion state it used to be told about is now already on the node. Lifting expansion into real React state would let the effect fire on its own, but it rewrites D2/D4 territory for no behavioural gain, so it stays out. + +## Risks / Trade-offs + +- **[Trade-off]** If a real (non-microflow) datasource's `status` never meaningfully transitions to `Loading` for some fetch (e.g. resolves synchronously from cache), the spinner simply won't show for that fetch — same as the original pre-`LOADING`-commit behavior (brief icon pop-in instead of a spinner). Cosmetic only, not a functional regression. +- **[Risk]** None identified that could reproduce either original bug — see D2's "structurally immune" reasoning above. Covered by unit tests asserting spinner-shows-while-loading, spinner-clears-on-settle (whether or not children arrived), and one node's resolution never affecting a sibling's spinner/children/state. +- **[Risk]** D3's bootstrap round-2 preload fetches one extra level for every currently-known item, regardless of whether the user has looked at it — a bounded, one-time eager-fetch (proportional to tree _width_ at that level, not depth) → **Mitigation**: capped at exactly 2 rounds via content-based flags, verified live and by a unit test asserting a 3rd datasource change does not trigger a 3rd round. This matches the cost the widget already pays for round 1 (unconditionally preloading roots' children regardless of collapse state) — round 2 is the same category of cost, one level deeper, not a new category of risk. +- **[Risk — realized and fixed, kept as a lesson]** A first attempt at this exact fix (a fire-count-based cap) passed every unit test against a mocked datasource but broke a real repro project live, because mocked datasources don't reproduce the transient "still loading, items temporarily empty" window that real ones do. Mitigation going forward: any change to `useInfiniteTreeNode.ts` that touches `setFilter` call timing/count must be live-verified against a real datasource before being trusted, not just unit-tested against a mock. +- **[Risk]** D4's `statesByIdRef` grows for the lifetime of the widget instance and is never pruned — an id whose item is deleted keeps its remembered state. Bounded by the number of distinct ids the datasource has ever delivered to this instance, holding one enum value each, and reset on unmount. → **Mitigation**: none taken deliberately; pruning on removal would break the legitimate case of an id disappearing and returning within the same session (a filter change, or an item re-delivered after a microflow round-trip), which is the case the map exists to serve. +- **[Risk]** D4's per-update sort runs over `rootsRef` plus every node's `children` array on every datasource delivery — O(n log n) across the tree rather than the previous O(1)-per-known-id. → **Mitigation**: the `children.length > 1` guard skips single-child and leaf nodes, which is most of a typical tree; the work is proportional to what the datasource just delivered, which the hook already iterates twice. +- **[Risk]** D5 changes `setFilter` call _timing and count_ in `useInfiniteTreeNode.ts` — precisely the category the lesson above says must not be trusted on mocked unit tests alone. The round1 early return is removed and the three mechanisms now share one call per pass, which is a behavioral change to the exact code path that broke live before. → **Mitigation**: mandatory live re-verification of all four scenarios (both WC-3564 bugs, the "Expanded bug" 4-tier cascade, and the "Collapsed bug" `startExpanded = false` path) against `~/Documents/_tickets/WC-3564-2` before this change is considered done — not just the new tests passing. Tracked as its own task group. +- **[Risk]** D4's early return on `items === undefined` means the widget renders stale nodes for the duration of a load, where it previously rendered an empty message. If a load never completes, the user sees old data with no indication rather than an empty tree. → **Mitigation**: accepted, and this is the intended behavior — D2's spinner is what indicates the in-flight load, and showing an empty tree mid-refresh was the reported bug. + +- **[Risk]** D6 rewrites `setFilter` timing and count a third time — the exact category task 7.1 proved cannot be trusted on mocked unit tests alone, and the category D5's risk entry already flagged. → **Mitigation**: the same mandatory live re-verification, extended with the department-switch scenario that found D6 in the first place (`treenodev2_advanced`, both `startExpanded` modes, at least two consecutive switches). A derivation is easier to reason about than three interacting flags, but that is an argument for reviewability, not a substitute for live proof. +- **[Trade-off]** Under D6's chosen root definition, an orphan (an item whose `parentId` is set but whose parent the datasource does not deliver) is never a requested parent, so it renders at root level with no expand affordance even if it has children. → **Accepted**: the alternative (definition (b)) costs a visible flicker plus a wasted retrieve on every result-set change, to serve a case that is usually a configuration accident. Recorded as a Non-Goal above. +- **[Trade-off]** Immediately after a result-set replacement, rows left over from the previous set (children of the previous set's parents, e.g. `Tablets` under department Finance) render once, inert, and disappear on the following delivery once the filter stops asking for their parents. A one-delivery transient, self-cleaning, and strictly better than today's behaviour where those rows persist for the widget's lifetime. +- **[Risk]** The derived set is recomputed by walking the tree on every delivery, plus a set comparison — O(n) where the previous mechanism was O(1) per already-known id. → **Mitigation**: the same order of work D4a's per-update sort already introduced in the sibling hook, over data the hook already iterates; and it replaces up to three separate `datasource.items` passes (round1, round2, the sweep) with one. +- **[Risk]** Making the tree → filter → tree loop explicit invites an infinite `setFilter` loop if the derivation is ever made to depend on a node's descendants. → **Mitigation**: `desiredParents` membership is defined over ancestry only, which is what makes the loop provably terminating (see D6); a unit test asserts that a repeated identical delivery triggers zero further `setFilter` calls, and the `lastAppliedParentIdsRef` equality guard is the backstop. + +## Migration Plan + +No data or config migration. `hasChildren` is no longer read by v2 at all, so existing v2 configurations (where it was always hidden/unset anyway) are unaffected; v1 is untouched. Standard widget version bump + changelog entry per repo convention; no feature flag needed since this is a bug fix restoring intended behavior. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/proposal.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/proposal.md new file mode 100644 index 0000000000..9a492e652f --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/proposal.md @@ -0,0 +1,49 @@ +## Why + +Tree Node v2 stores `LOADING` as a per-node state and resolves it via a broken heuristic: "this node's id reappeared in some later datasource delivery." A microflow datasource — which always redelivers its full flattened result and ignores `setFilter` — breaks that heuristic in two ways (WC-3564): a permanently stuck loading spinner when "Start expanded" is Yes, and a 3rd-tier node silently and permanently losing its expand icon when a sibling is expanded. Both share the same root cause and are fixed by the same change, hence one proposal covering both. + +Manual verification of that fix surfaced two further, separate, pre-existing bugs in the same "preload one level ahead of what's expanded" mechanism (`useInfiniteTreeNode.ts`) — unrelated to `LOADING`/`hasChildren`, but bundled into this same change since they were found, understood, and fixed during the same verification pass: a node's first-ever expand didn't preload its own children's children (needed a collapse+re-expand of that same node to reveal a deeper tier), and the automatic root-expansion path for "Start expanded" = Yes had the same gap, recurring at every level. A first attempt at the second fix (a fixed 2-round cap) broke a live repro project and was reverted before being corrected; a second attempt fixed that but, against a deeper (4-tier) dataset, turned out to still be too shallow — every level defaults to expanded under "Start expanded" = Yes, not just roots, so a fixed round count can't be right at all. The final fix replaces the round cap with an unbounded, self-terminating cascade, scoped specifically to "Start expanded" = Yes. See `design.md` D3 for the full story, including the lesson that a fix here needs live verification against a real datasource, not just mocked unit tests. + +Separately, a parallel branch (`tmp/treenode-fix1`) fixed two more pre-existing v2 bugs in `useIncrementalTreeData.ts`, both rooted in the same design as the bugs above — the hook builds its node map incrementally and reuses nodes across datasource updates: (a) a node is appended to `rootsRef`/`parent.children` only the first time its id is seen, so a new datasource sort order is never applied until the widget remounts; (b) a refresh throws the tree away and rebuilds it, and rebuilt nodes start collapsed, so every node collapses on refresh (and the tree briefly showed "no data available" while loading, because an undefined `items` was read as "all items removed"). It also extended the `appendItems` preload to cover children that arrive _after_ an expand. That branch is folded into this change rather than merged separately, because it rewrites the exact two functions this change already rewrites — reconciling them is a design decision, not a textual merge. See `design.md` D4 and D5. + +Live verification of _that_ fold-in then surfaced the deepest instance of the same pattern (`design.md` D6). When an app-level constraint replaces the datasource's entire result set — a gallery filtering the tree by department, a changed page parameter — every node in a "Start expanded" = No tree renders permanently inert: no expand icon, no `aria-expanded`, no clickable header, no keyboard expand. The preload filter is frozen at whatever it was on the initial load, because the round flags are one-shot for the widget's lifetime and the parent-id maps are never pruned; stale parent ids also drag rows from the previous result set into the new one, where they render at root level next to genuine roots. Rather than add a fourth delivery-history heuristic ("was the result set replaced?") on top of the three this change already has, the preload filter is replaced with a single derived rule — the parent set is computed from the current tree on every delivery, and D3's rounds, D5's sweep, and the "Start expanded" = Yes cascade all fall out of it as consequences rather than mechanisms. + +## What Changes + +- `LOADING` is no longer stored on a node at all. Nodes are created already resolved (`EXPANDED`/`COLLAPSED_WITH_JS` per `startExpanded`); the click-to-expand handler goes back to unconditionally setting `EXPANDED`. +- The spinner becomes a pure render-time decision: shown when a node has no known children yet (`node.children.length === 0`) **and** Mendix's own `datasource.status === ValueStatus.Loading` — a real, first-party "is this actually fetching right now" signal, not per-node bookkeeping. +- Expand-icon visibility for v2 stays derived from `node.children.length > 0` (unchanged from before this ticket) — **not** from the `hasChildren` widget property. `hasChildren` is hidden by Studio Pro and unset at runtime whenever `parentAssociation` is configured, which is every v2 instance; reading it crashes the widget (confirmed live during this change's implementation). +- **`useInfiniteTreeNode.ts`'s preload filter is now derived, not accumulated.** On every delivery the widget computes the set of parents it needs from the current tree — `{undefined}` plus every visible node plus every child of a visible node (visible = all ancestors expanded) — and calls `setFilter` only when that set differs from the one last applied. `loadedParentsByIdRef`, `loadedChildsByIdRef`, `expandedIdsRef`, `round1DoneRef` and `round2DoneRef` are all deleted; `appendItems(item, children)` becomes an argument-less `syncPreloadFilter()`. See `design.md` D6. + - This supersedes the two round-based fixes below and the late-arrival sweep further down — each of them is now a consequence of the rule rather than a mechanism of its own. Kept as decision records because the reasoning behind them is what produced the rule. + - ~~`appendItems`: removed a gate that skipped preloading a node's grandchildren-existence on its first-ever expand.~~ Superseded: expanding a node makes its children visible, so their children enter the derived set automatically. + - ~~The bootstrap effect cascades level-by-level under "Start expanded" = Yes, and keeps the capped round1+round2 behavior under No.~~ Superseded: under Yes every node is expanded, so every delivered node is visible and the cascade falls out of the same rule; under No nothing is expanded, so the rule yields exactly roots + one level of lookahead, and grows only as the user expands. +- A result-set replacement (an app-level constraint such as a gallery filtering by department) now re-derives the filter like any other delivery, so the new roots' children are fetched and their expand affordance is correct. Previously the filter was frozen at the initial load's value, leaving every node inert with no way to open it. +- `useIncrementalTreeData.ts` re-applies the datasource order to `rootsRef` and to every node's `children` on every update, instead of only capturing order at first-sight of an id. +- `useIncrementalTreeData.ts` returns early when `items` is undefined, keeping the tree it already built while the datasource reloads — instead of reading undefined as an empty list, concluding every item was removed, and rebuilding from scratch behind the empty message. +- `useIncrementalTreeData.ts` remembers each node's expanded/collapsed state by id and restores it when a node is re-created during a rebuild, so the rebuilds this change cannot avoid (config-reference churn, item removal) no longer collapse the tree. The remembered state wins over the `startExpanded` default in both directions. +- Children that arrive after their parent was expanded — still in flight at expand time, or created later by a microflow — still restore that parent's expand affordance. ~~Implemented as a tracked-expanded-ids sweep plus a restructure of the round1/round2 gates so they no longer return early (`design.md` D5).~~ Superseded by the derived rule above: the set is recomputed from the current delivery, so the delivery that carries the late child is the one that requests its children. The behaviour and its tests stay; the mechanism is gone. +- Stale parent ids no longer drag rows from a previous result set into the current one. Because the set is derived from the current tree rather than accumulated, ids that leave the delivery leave the filter. +- Update `TreeNodeV2.spec.tsx`, `useIncrementalTreeData.spec.ts`, and `useInfiniteTreeNode.spec.ts` to cover the corrected behavior. +- Add `@mendix/widget-plugin-test-utils` as an explicit devDependency (already imported by `useInfiniteTreeNode.spec.ts`, previously resolved only via hoisting). + +## Capabilities + +### New Capabilities + +- `tree-node-expand-state`: governs how a v2 tree node decides (a) whether it shows an expand affordance at all, (b) whether it shows a spinner in place of that affordance, and (c) whether it is expanded or collapsed — all independent of datasource re-delivery timing or datasource type (microflow vs. non-microflow). +- `tree-node-data-refresh`: governs how the v2 incremental tree map reacts to a datasource update — sibling and root ordering following the current datasource order, the tree surviving a reload rather than being torn down and rebuilt, and a replaced result set not leaving rows from the previous one behind. + +### Modified Capabilities + +(none — no existing `openspec/specs/` in this package prior to this change) + +## Impact + +- `src/components/v2/TreeNode.tsx` — icon-render condition now spinner-vs-chevron based on `datasource.status`; click handler simplified (no `LOADING` entry). +- `src/components/v2/hooks/useIncrementalTreeData.ts` — nodes created pre-resolved; no `LOADING` assignment anywhere in this file; new-node state falls back to a remembered per-id state before the `startExpanded` default; early return while `items` is undefined; datasource order re-applied to roots and to every node's children on every update. +- `src/components/v2/hooks/useInfiniteTreeNode.ts` — rewritten around the derived parent set: five history refs replaced by one `lastAppliedParentIdsRef` equality guard; the set is computed from `treeData` (visible nodes plus one level of lookahead) using only ids present in the current delivery; `appendItems(item, children)` becomes `syncPreloadFilter()`. The one-time init stays as-is (root-only filter when `startExpanded` is No, no filter when Yes). +- `src/components/v2/TreeNode.tsx` — `useIncrementalTreeData` is now fed `props.datasource.items` directly and its `treeData` is passed into the preload hook, instead of threading `items` out of that hook; the click handler calls `syncPreloadFilter()` after mutating `treeNodeState`. +- `typings/TreeNodeProps.d.ts` — no change. +- `package.json` / `pnpm-lock.yaml` — `@mendix/widget-plugin-test-utils` added as an explicit devDependency. +- `src/components/v2/__tests__/TreeNodeV2.spec.tsx`, `src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts`, `src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts` — updated/added regression tests. +- No XML property changes. `hasChildren` remains untouched for v1. The v1 code path is unaffected throughout: it rebuilds from `datasource.items` on every update, so it never exhibited the ordering or expansion-state bugs either. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md new file mode 100644 index 0000000000..b519288fb6 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/specs/tree-node-data-refresh/spec.md @@ -0,0 +1,58 @@ +## ADDED Requirements + +### Requirement: Sibling and root order always follows the current datasource order + +The v2 Tree Node widget SHALL order root nodes, and each node's children, according to the order in which the datasource currently delivers those items — re-applied on every datasource update, not captured once when a node's id is first seen. A node whose incremental tree map already contains an id MUST still be re-positioned among its siblings when the datasource's order for that id changes. + +#### Scenario: Root nodes are re-ordered when the datasource returns them in a new order + +- **WHEN** the datasource re-delivers the same root items in a different order (for example after a microflow changed a sequence attribute that the datasource sorts on) +- **THEN** the widget renders the root nodes in the new datasource order, without requiring the page to be reopened or the widget to remount + +#### Scenario: A node's children are re-ordered when the datasource returns them in a new order + +- **WHEN** the datasource re-delivers the same child items of an already-known parent in a different order +- **THEN** the widget renders that parent's children in the new datasource order + +#### Scenario: Re-ordering does not disturb node state + +- **WHEN** sibling order changes across a datasource update +- **THEN** each node keeps its own expanded/collapsed state, its title, and its already-placed children — only its position among its siblings changes + +#### Scenario: A node absent from the current delivery keeps a stable position + +- **WHEN** a node is present in the tree but its id is not in the current datasource delivery (so the datasource expresses no order for it) +- **THEN** the widget keeps that node ordered after every node the current delivery does order, preserving the absent nodes' relative order among themselves rather than reordering them arbitrarily + +### Requirement: The rendered tree is preserved while the datasource is reloading + +The v2 Tree Node widget SHALL treat an undefined `datasource.items` as "not yet delivered" and keep the tree it has already built, rather than interpreting it as an empty result. A reload MUST NOT clear the tree, discard the node map, or surface the empty-message state for the duration of the load. + +#### Scenario: A reload does not empty the tree + +- **WHEN** the datasource starts reloading and `items` becomes undefined +- **THEN** the widget keeps rendering the nodes it had before the reload, with their existing expanded/collapsed state, and does not show the "no data available" empty message + +#### Scenario: An undefined delivery is not mistaken for removed items + +- **WHEN** `items` is undefined +- **THEN** the widget does not conclude that every previously-known item was removed, and therefore does not trigger a rebuild of the node map for that reason + +#### Scenario: The tree updates once the reload settles + +- **WHEN** the reload completes and the datasource delivers items again +- **THEN** the widget applies the new items — including any additions, removals, and the current datasource order — to the tree it preserved + +### Requirement: A replaced result set does not leave rows from the previous one behind + +The v2 Tree Node widget SHALL NOT keep requesting children of parents that belong to a superseded result set. When an app-level constraint replaces the datasource's result set, rows that are only present because a previous set's parents are still being requested MUST NOT persist in the tree. + +#### Scenario: Leftover rows from the previous result set disappear + +- **WHEN** an app-level constraint replaces the result set, and the first delivery after that switch still contains items retrieved on account of the previous set's parents +- **THEN** those items are gone from the tree by the time the widget has settled, rather than remaining alongside the new set's genuine roots for the rest of the session + +#### Scenario: Retrieval does not grow across result-set changes + +- **WHEN** the result set is replaced repeatedly during a session +- **THEN** the number of parents the widget asks for reflects only the current tree, and does not accumulate the parents of every set seen so far diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md new file mode 100644 index 0000000000..ac7ad1604d --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/specs/tree-node-expand-state/spec.md @@ -0,0 +1,158 @@ +## ADDED Requirements + +### Requirement: Expand affordance visibility is driven by known children, not by a load-timing-sensitive stored state + +The v2 Tree Node widget SHALL determine whether a node's expand affordance (chevron/icon) is shown based on whether that node currently has any children placed under it (`node.children.length > 0`), computed fresh on every render — never from the `hasChildren` widget property, which is unavailable in v2's configuration mode (`parentAssociation` set), and never from a stored per-node flag that a later, unrelated datasource delivery could incorrectly mutate. + +#### Scenario: A node with children shows an expand affordance + +- **WHEN** a node has at least one child currently placed under it +- **THEN** the widget renders an expand affordance for that node + +#### Scenario: A node with no known children shows no expand affordance (absent a spinner) + +- **WHEN** a node has no children currently placed under it and the datasource is not currently loading +- **THEN** the widget renders no expand affordance for that node + +#### Scenario: Expanding one node does not affect a sibling's expand affordance + +- **WHEN** a user expands node A, causing a datasource redelivery that includes node B's id (node B was never expanded and has no relation to node A) +- **THEN** node B's expand affordance and underlying children are unchanged from before node A was expanded + +### Requirement: A loading spinner is shown only while the datasource is genuinely fetching, never as stored per-node state + +The v2 Tree Node widget SHALL show a loading spinner in place of the expand affordance for a node that has no known children yet, exactly while `datasource.status === ValueStatus.Loading`. This is a render-time computation only — no per-node "is loading" flag is stored, so the spinner cannot become stuck and cannot be affected by an unrelated node's resolution. + +#### Scenario: Spinner shown while the datasource is loading and children are unknown + +- **WHEN** a node has no children placed under it yet and the datasource's `status` is `Loading` +- **THEN** the widget shows a spinner in place of the expand affordance for that node + +#### Scenario: Spinner clears once the datasource settles, regardless of outcome + +- **WHEN** the datasource's `status` transitions away from `Loading` +- **THEN** every node's spinner clears immediately — showing an expand affordance if children arrived, or no affordance at all if they didn't + +#### Scenario: Spinner never shows for a node that already has children + +- **WHEN** a node already has at least one child placed under it +- **THEN** the widget never shows a spinner for that node, regardless of `datasource.status` + +#### Scenario: A stalled or filter-ignoring datasource never produces a stuck spinner + +- **WHEN** a microflow datasource ignores `setFilter` and its `status` never transitions to `Loading` for a given fetch attempt +- **THEN** no node is left showing a spinner indefinitely as a result of that fetch attempt + +### Requirement: A manually expanded node's own children's expand affordance is known without requiring a collapse-and-reopen + +The v2 Tree Node widget SHALL preload one level past a node's children when that node is expanded by a user click, so each child's own expand affordance is already correct the first time its parent is expanded — never requiring the user to collapse and re-expand that same node to reveal it. This preload is bounded to exactly one level past what's already known for this path; it does not eagerly walk the full tree beyond the node that was actually clicked. + +#### Scenario: A node's own children's children are known on its first expand + +- **WHEN** a user expands a node for the first time (its children are already known, but whether those children themselves have children is not) +- **THEN** each of that node's children already shows its correct expand affordance immediately, without requiring that child to be separately collapsed and re-expanded + +### Requirement: Under "Start expanded" = Yes, every auto-expanded level's own expand affordance is known automatically, all the way to the tree's real depth + +Because every node defaults to expanded (not just roots) when "Start expanded" is Yes, the v2 Tree Node widget SHALL keep preloading one level further for as long as new descendants keep appearing — not a fixed number of levels — so that every already-visible node's expand affordance is correct without any manual collapse-and-reopen, regardless of how deep the actual tree data goes. This cascade is self-terminating: it stops automatically once a level introduces no previously-unseen items, bounded by the tree's real depth rather than an arbitrary count or recursing indefinitely. + +#### Scenario: A 3rd (or deeper) tier's own expand affordance is known automatically + +- **WHEN** "Start expanded" is Yes and the underlying data has 3 or more tiers +- **THEN** every tier's nodes show their correct expand affordance immediately on load, with no tier requiring a manual collapse-and-reopen to reveal the next tier down + +#### Scenario: The cascade stops once the real data is exhausted + +- **WHEN** a subsequent datasource delivery introduces no items beyond what's already known +- **THEN** no further automatic preload round is triggered — the cascade does not continue indefinitely or re-fetch unchanged data + +#### Scenario: A transient empty datasource delivery during initial load does not disable the cascade + +- **WHEN** the datasource is still loading and delivers an empty item set one or more times before the real data arrives +- **THEN** the cascade does not lock itself out on that empty delivery — it only advances once it actually finds real, previously-unseen items, and keeps retrying harmlessly until it does + +### Requirement: The auto-cascade does not apply when "Start expanded" is No + +The v2 Tree Node widget SHALL NOT preload beyond visible nodes plus one level of lookahead when "Start expanded" is No, since deeper tiers remain collapsed by default and already resolve correctly via a single real click. Preloading further in this mode would only eagerly fetch descendants of branches the user has not opened. With nothing expanded, "visible nodes plus one level" resolves to exactly the root nodes and their children; it grows only as the user actually expands. + +#### Scenario: A 3rd-tier arrival does not trigger a further automatic round when collapsed by default + +- **WHEN** "Start expanded" is No and a 3rd-tier item arrives as a result of the existing 2-round preload +- **THEN** no further automatic preload round is triggered for it — expanding it further still requires a real click + +### Requirement: A child that arrives after its parent was expanded still restores that parent's expand affordance + +The v2 Tree Node widget SHALL keep preloading one level ahead for nodes the user has already expanded, so a child that becomes known only after the expand — because it was still in flight at expand time, or because it was created later — is still preloaded, and the affected node's expand affordance is still correct. This applies whether or not the node's children were already known when `appendItems` ran for it. + +#### Scenario: Children still in flight at expand time are preloaded once they arrive + +- **WHEN** a user expands a node whose children have not been delivered yet, and those children arrive in a later datasource delivery +- **THEN** the widget preloads those children's own children, so each arriving child shows its correct expand affordance without the user collapsing and re-expanding the parent + +#### Scenario: A child created after the expand is preloaded + +- **WHEN** a node is already expanded with known children, and a microflow adds a further child to that node +- **THEN** the widget preloads the newly-added child's own children, so the new child shows its correct expand affordance as soon as it is rendered + +#### Scenario: Repeated deliveries do not re-request the same parent + +- **WHEN** a datasource delivery leaves the set of parents the widget needs unchanged from the set it last requested +- **THEN** no further preload request is issued for that delivery, and no parent id appears more than once in the preload filter + +### Requirement: A node's expanded or collapsed state survives a tree rebuild + +The v2 Tree Node widget SHALL remember each node's expanded/collapsed state by item id and restore it when that node is re-created during a rebuild of the incremental node map, so a datasource refresh never silently collapses the tree the user had opened. A remembered state MUST take precedence over the `startExpanded` default, in both directions. + +#### Scenario: Expansion survives new prop instances on refresh + +- **WHEN** the Mendix client hands the widget new prop instances on a refresh (which the widget compares by reference and therefore treats as a configuration change, rebuilding the node map) +- **THEN** every re-created node comes back with the expanded/collapsed state it had before the rebuild, not with the `startExpanded` default + +#### Scenario: Expansion of remaining nodes survives an item removal + +- **WHEN** a single item is deleted from the datasource, triggering a rebuild of the node map +- **THEN** the remaining nodes come back with the expanded/collapsed state they had before the removal + +#### Scenario: A user-collapsed node stays collapsed even when "Start expanded" is Yes + +- **WHEN** "Start expanded" is Yes, the user collapses a node, and a later refresh rebuilds the node map +- **THEN** that node comes back collapsed — the remembered state wins over the `startExpanded` default + +#### Scenario: A node never seen before still follows the configured default + +- **WHEN** an item id appears that has no remembered state (a genuinely new node) +- **THEN** that node is created directly in the state `startExpanded` dictates (`EXPANDED` or `COLLAPSED_WITH_JS`) — never in a stored `LOADING` state + +### Requirement: The set of parents to preload is derived from the current tree, never accumulated from delivery history + +The v2 Tree Node widget SHALL determine which parents to request on every datasource delivery by deriving them from the tree as it currently stands — the roots, plus every node all of whose ancestors have their subtree rendered, plus every child of such a node — and SHALL request exactly that set. A node's subtree counts as rendered when the node is expanded, and also when it was expanded and then collapsed again (its body remains in the DOM, hidden), but not when it has never been expanded. It MUST NOT maintain a record of what has already been fetched, a one-shot "preload done" flag, or any other delivery-history state as the basis for that decision. A node counts as a root for this purpose only when it has no parent at all, not merely when its parent is absent from the current delivery. + +#### Scenario: A replaced result set still gets its expand affordance + +- **WHEN** an app-level constraint replaces the datasource's entire result set (for example the user picks a different department in a gallery that filters the tree), producing a set of roots none of which the widget has seen before +- **THEN** the widget requests the new roots' children, and every new root that has children shows its expand affordance — it is not left inert with no icon, no `aria-expanded`, no clickable header and no keyboard expand + +#### Scenario: Consecutive result-set replacements each behave identically + +- **WHEN** the result set is replaced a second and third time in the same session, without the widget remounting +- **THEN** each replacement is treated exactly like the first — there is no round, flag, or budget that a previous replacement can have used up + +#### Scenario: Parents from a previous result set are no longer requested + +- **WHEN** a delivery no longer contains an item that was previously a requested parent +- **THEN** that item's id is absent from the next filter the widget applies, so its children are no longer retrieved + +#### Scenario: Collapsing a node does not drop what was already fetched below it + +- **WHEN** the user collapses a node whose descendants have already been retrieved +- **THEN** the widget requests the same set of parents as before the collapse, and re-expanding that node shows its children with their expand affordances intact + +#### Scenario: An item whose parent is not delivered is not treated as a root + +- **WHEN** a delivered item has a parent association pointing at an object the datasource does not deliver +- **THEN** the widget does not request that item's children on the grounds that it renders at root level, and it therefore shows no expand affordance — the item leaves the tree on the following delivery, once the filter stops asking for its parent + +#### Scenario: Only items in the current delivery are used to build the filter + +- **WHEN** the tree still holds nodes whose ids were not in the current delivery (retained per the data-refresh behaviour) +- **THEN** the filter is built only from items the current delivery provided, so no stale object reference is used to request children diff --git a/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/tasks.md b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/tasks.md new file mode 100644 index 0000000000..9649ef5904 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/changes/archive/2026-09-22-fix-tree-node-loading-state/tasks.md @@ -0,0 +1,163 @@ +## 1. Resolve-at-creation, drop stored `LOADING` (D2) + +- [x] 1.1 In `useIncrementalTreeData.ts`'s node-creation branch, create new nodes directly as `config.startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS` instead of `TreeNodeState.LOADING`. +- [x] 1.2 Remove the "existing node in `LOADING` resolves on reappearance" branch entirely — no node is ever assigned `LOADING` in this file anymore, so there is nothing left to resolve. +- [x] 1.3 Revert `TreeNode.tsx`'s click handler to unconditionally set `EXPANDED` on expand-click (matches pre-`LOADING`-commit behavior) — no `LOADING` entry via click, no guard needed. +- [x] 1.4 Compute the spinner at render time in `TreeNode.tsx`: `showSpinner = node.children.length === 0 && props.datasource.status === ValueStatus.Loading`. Pass `showSpinner ? TreeNodeState.LOADING : node.treeNodeState` into `renderHeaderIcon`. No per-node `LOADING` is stored anywhere. + +## 2. `hasChildren` derivation — reverted after live-testing crash + +- [x] 2.1 **Correction, found via live Studio Pro testing (not caught by unit tests):** the initial plan was to read `props.hasChildren` in v2. This crashed the widget — `TreeNode.editorConfig.ts:38-39` hides `hasChildren` from Studio Pro whenever `parentAssociation` is configured (every v2 instance, including both of this ticket's repro projects), and it has no XML default, so `props.hasChildren` is `undefined` at runtime for v2. Reverted `TreeNode.tsx:20`'s `hasChildren` derivation back to `node.children.length > 0` — the same signal it used before this ticket. Removed the `hasChildrenExpr` parameter from `renderRecursiveNode` entirely. +- [x] 2.2 Confirmed `aria-expanded`, icon clickability, and the icon-render condition all still read the single `hasChildren` local (now `node.children.length > 0`) unchanged. +- [x] 2.3 Confirmed the icon-render condition is `(hasChildren || showSpinner) && iconPlacement !== "no"` — a node with children never spins; a childless node spins only while `datasource.status === Loading`. + +## 3. Regression tests + +- [x] 3.1 Rewrote `TreeNodeV2.spec.tsx`'s stale comment (previously claimed the datasource setup existed to satisfy a `hasChildren` prop check — no longer applicable since `hasChildren` isn't read at all). Hoisted shared test helpers to module scope so a new describe block could reuse them. +- [x] 3.2 Added a unit test (`TreeNodeV2.spec.tsx` + `useIncrementalTreeData.spec.ts`): a node is created directly in `EXPANDED`/`COLLAPSED_WITH_JS` per `startExpanded` and never passes through `LOADING`, even when the datasource keeps redelivering the same full item set (simulating a microflow ignoring `setFilter`) — Bug 1 regression. +- [x] 3.3 Added a unit test: a node's own children arriving does not change an unrelated sibling's `treeNodeState`, children, or spinner — Bug 2 regression. Covered in both spec files. +- [x] 3.4 Added unit tests for the spinner itself: shows while `datasource.status === Loading` and no children are known; clears once `status` settles regardless of whether children arrived; never shows for a node that already has children. + +## 4. Manual verification + +- [x] 4.1 Rebuilt against `~/Documents/_tickets/WC-3564-2` and drove it with Playwright. First rebuild (with the `props.hasChildren`-based design) crashed the widget live — see task 2.1. After the correction, rebuilt again and confirmed: expanding "Top level 2" leaves "Second level 1a"/"Second level 1b"'s expand icons intact (icon count unchanged before/after, screenshot-confirmed) — Bug 2 fixed. +- [x] 4.2 Confirmed bug 1's repro (microflow datasource, "Start expanded" = Yes) live: tree renders fully expanded immediately, zero `.widget-tree-node-loading-spinner` elements, no console/page errors. + +## 5. Changelog + +- [x] 5.1 Added a `CHANGELOG.md` entry under `[Unreleased]` describing the user-visible fix (both bugs), no implementation details, per repo changelog conventions. + +## 6. Third finding: `appendItems` off-by-one-click preload gap (found during manual verification) + +Pre-existing on `main`, unrelated to `useIncrementalTreeData.ts`/`TreeNode.tsx` (untouched by groups 1-2). A node's own click-to-expand never preloaded its _own_ children's children — required a collapse+re-expand of that same node before a deeper tier's expand icon appeared. + +- [x] 6.1 In `useInfiniteTreeNode.ts`'s `appendItems`, removed the outer `if (loadedParentsByIdRef.current.has(parentId))` gate around the grandchildren-preload `children.forEach(...)` step — it was skipping that step on a node's _first_ expand (the only time it matters), only running it from the second expand onward. +- [x] 6.2 Added a unit test in `useInfiniteTreeNode.spec.ts` — not needed as a new test; existing "first expansion" describe block continues to cover this since the gate removal doesn't change its assertions, but confirmed no existing test asserted the buggy gated behavior. +- [x] 6.3 Verified live: rebuilt against `~/Documents/_tickets/WC-3564-2`, single click on "Second level 1a" (previously required collapse+re-expand) now immediately reveals "Fourth level 1" under "Third level 1a1" — confirmed via Playwright polling (no click-twice needed). +- [x] 6.4 Re-ran the full rigorous Bug 1 / Bug 2 regression suite live after this change — no regressions. + +## 7. Fourth finding: bootstrap preload capped at one round, never reaches a second (found during manual verification) + +Pre-existing on `main`, in `useInfiniteTreeNode.ts`'s second `useEffect` block — separate from group 6 (that gate was in `appendItems`, click-driven; this one is in the automatic bootstrap path that runs regardless of clicks). For `startExpanded = Yes` specifically, root nodes auto-expand via this bootstrap path rather than via `appendItems`, so group 6's fix doesn't reach them — closing and reopening a root node was required to reveal a 3rd tier. + +- [x] 7.1 **First attempt (reverted):** replaced the one-shot `if (loadedParentsByIdRef.current.size === 0)` gate with a `bootstrapRoundRef` counter that advanced unconditionally on every effect firing, capped at 2. Passed unit tests against a mocked datasource. **Broke live**: rebuilt against `~/Documents/_tickets/WC-3564-2`, the "Expanded bug" tab permanently stopped rendering anything past the root level. Root-caused via temporary debug instrumentation (tagged per-widget-instance to disentangle the 3 tree widgets that mount simultaneously on that page): both rounds fired — and locked themselves in — while `datasource.items` was still transiently empty (still loading), _before_ the real root items ever arrived. The counter had no way to tell "fired" apart from "fired with real data," so it burned both of its capped rounds on nothing. +- [x] 7.2 **Second attempt (this one, kept):** replaced the counter with two content-based booleans (`round1DoneRef`, `round2DoneRef`) that only flip once real, previously-unseen items are actually found — mirroring the original round-1 gate's own self-correcting semantics (`loadedParentsByIdRef.current.size === 0`, checked _after_ attempting to populate, so it harmlessly retries on empty deliveries instead of locking in early). Round 2 only locks in once it finds at least one item that isn't already a known parent or child. +- [x] 7.3 Removed all debug instrumentation added for root-causing 7.1 (tagged `[DEBUG-t3564b]`, per-widget-instance) — confirmed zero references remain. +- [x] 7.4 Added a unit test in `useInfiniteTreeNode.spec.ts` that explicitly exercises the failure mode from 7.1: several transient empty-item rerenders before round 1 locks in, another empty/unchanged rerender before round 2 locks in, then confirms round 2 only advances once real new items appear, and a further identical rerender does not trigger a round 3. +- [x] 7.5 Verified live: rebuilt against `~/Documents/_tickets/WC-3564-2`. "Expanded bug" tab now shows all 3 tiers automatically on load — no manual toggle needed. Re-ran the full rigorous Bug 1 / Bug 2 / group-6 (`appendItems`) regression suite live — all still pass, no regressions. +- [x] 7.6 **Follow-up finding, same session:** the 2-round cap turned out insufficient — with a 4-tier dataset, the 3rd tier appeared as content but without its own expand icon (needed a real click on its own ancestor to reveal, one level deeper than the original repro). Root cause: under `startExpanded = Yes`, _every_ level defaults to `EXPANDED` (not just roots), so every level needs the same automatic preload treatment, not just a fixed 2 rounds. Fixed by replacing the 2-round cap with an unbounded, self-terminating cascade **gated specifically on `startExpanded === true`**: keep treating newly-arrived items as loaded-parents and fetching their children for as long as new descendants keep appearing, stopping naturally once a round finds nothing new (bounded by the tree's real depth, not an arbitrary count). Explicitly scoped to `startExpanded = true` only, per user decision — `startExpanded = false` keeps the original capped round1+round2 behavior unchanged (deeper tiers there already resolve correctly via a single real click, per group 6's fix; auto-cascading for still-collapsed branches would just be wasted eager-fetching of content the user hasn't opened). +- [x] 7.7 Updated the unit test from 7.4 to match: renamed/rewritten as an unbounded-cascade test asserting 3+ sequential levels each trigger exactly one more `setFilter` call as they arrive, and a repeated/empty delivery triggers none. Added a second test confirming `startExpanded = false` still caps at exactly 2 rounds (3rd-tier arrival triggers no further automatic call). +- [x] 7.8 Verified live again: rebuilt against `~/Documents/_tickets/WC-3564-2`. "Expanded bug" tab (4 levels of data) now shows all 4 tiers automatically on load, with `Third level 1a1` already showing its own expand icon with zero manual interaction — matching the exact screenshot the user originally showed as the expected/desired result. Re-ran the full rigorous Bug 1 / Bug 2 / group-6 / "Collapsed bug" (startExpanded=false, 2-round-cap) regression suite live — all still pass, no regressions. + +## 8. Planning: fold in `tmp/treenode-fix1` (scope extension) + +The parallel branch `tmp/treenode-fix1` (commits `76bfb5b94`, `362bbd396`) fixed two further v2 bugs plus a preload gap, rewriting the same two functions groups 1-7 rewrote. Folded into this change rather than merged separately — see `design.md` D4/D5 for why, and for the four collision points. + +- [x] 8.1 Analysed the overlap between this change and `tmp/treenode-fix1`; identified four collisions (stored `LOADING` vs. restored state, the `items === undefined` guard, sibling ordering, and the `appendItems`/bootstrap-effect rewrites) and confirmed only the first and last need a design decision. +- [x] 8.2 Extended `proposal.md` (Why / What Changes / Capabilities / Impact) to cover the folded-in scope. +- [x] 8.3 Added `design.md` D4 (ordering + expansion-state survival, and why `resolveRestoredState` is dropped) and D5 (bootstrap effect restructure, `appendItems` reconciliation, known root-sweep gap), plus four new Risks entries. +- [x] 8.4 Added the `tree-node-data-refresh` capability spec (ordering, reload survival) and two requirements to `tree-node-expand-state` (late-arriving children restore the affordance; expansion state survives a rebuild). +- [x] 8.5 Removed the stale `Auto-expanded root nodes (startExpanded = Yes) — NOT YET IMPLEMENTED` requirement from `specs/tree-node-expand-state/spec.md`. It described the reverted first attempt from task 7.1 and directly contradicted the unbounded-cascade requirement three blocks above it, which task 7.6 delivered. +- [x] 8.6 Base the implementation branch on this change's branch (not on `tmp/treenode-fix1`) and re-apply the parallel branch's three behaviours as fresh commits — `git rebase` would conflict on both hooks, and collisions 1 and 4 are genuine rewrites rather than textual conflicts, so a rebase resolution would silently pick a shape neither design intended. Keep `tmp/treenode-fix1` intact as the reference. + +## 9. Fifth finding: datasource sort order never re-applied (D4a) + +- [x] 9.1 In `useIncrementalTreeData.ts`, build an `orderById` map from the current delivery (`getItemId(item) -> index`) alongside the existing `incomingIds` set. +- [x] 9.2 At the end of the update, sort `rootsRef.current` and every node's `children` by that order. Nodes absent from the current delivery sort to the end via `?? sourceItems.length`, keeping their relative order among themselves. +- [x] 9.3 Guard the per-node sort on `children.length > 1` so leaves and single-child nodes are skipped. +- [x] 9.4 Confirm sorting happens after all placement (including the out-of-order child-before-parent path) and before `setTreeData`, so a node placed late in the same pass is still ordered correctly. + +## 10. Sixth finding: every node collapses on a datasource refresh (D4b) + +- [x] 10.1 Return early from the effect when `items === undefined`, keeping the existing tree — instead of `items ?? []`, which made every known id look removed and tripped a rebuild behind the empty message. Verify the mid-load "no data available" flash is gone. +- [x] 10.2 Add a `statesByIdRef: Map` and snapshot every node's `treeNodeState` into it immediately before the `configChanged || removedIdsDetected` rebuild clears the maps. +- [x] 10.3 On the node-creation branch, prefer the remembered state over the `startExpanded` default: `statesByIdRef.current.get(nodeId) ?? (config.startExpanded ? EXPANDED : COLLAPSED_WITH_JS)`. +- [x] 10.4 **Do not** port the parallel branch's `resolveRestoredState` helper. Its "nothing remembered" arm returns `TreeNodeState.LOADING`, which reintroduces WC-3564 Bug 1's exact mechanism (see task 1.1/1.2), and its "remembered is `LOADING`" arm is dead code after group 1. See `design.md` D4. +- [x] 10.5 Confirm the remembered state wins over `startExpanded` in both directions — a node collapsed by the user under "Start expanded" = Yes must come back collapsed. +- [x] 10.6 Leave `isConfigChanged`'s reference comparison as-is; rebuilding on prop-instance churn is correct once the rebuild is non-destructive, and `ListExpressionValue`/`ListReferenceValue` have no meaningful structural equality to compare instead. + +## 11. Seventh finding: children arriving after an expand are never preloaded (D5) + +- [x] 11.1 Add an `expandedIdsRef: Set` to `useInfiniteTreeNode.ts`, populated in `appendItems` with the expanded node's id, and reset it in the `initializedRef` init block alongside `round1DoneRef`/`round2DoneRef`. +- [x] 11.2 Add the late-arrival sweep to the post-init effect: for each `datasource.items` entry not tracked in either map, if `getParentId(item, parentAssociation)` is in `expandedIdsRef`, add it to `loadedChildsByIdRef` and mark a refilter as needed. +- [x] 11.3 **Restructure the `startExpanded = false` path so round1 and round2 no longer `return`.** Replace their individual `setFilter` calls with a single `shouldRefilter` flag and one `setFilter` at the end of the pass; keep round1/round2 mutually exclusive (`else if`); run the sweep unconditionally after them. Without this, round1's early return swallows the sweep entirely — see `design.md` D5 for the concrete trace. +- [x] 11.4 Leave D3's `startExpanded = true` cascade untouched, including its early return — it already treats every newly-arrived item as a loaded parent, which subsumes the sweep for that mode. +- [x] 11.5 Add `parentAssociation` to the effect's dependency array (now read via `getParentId`) and import `getParentId` from `./helpers`. +- [x] 11.6 Keep group 6's `appendItems` shape, but add the parallel branch's `if (!loadedParentsByIdRef.current.has(childId))` guard to the child loop — without it a previously-expanded-then-collapsed child lands in both maps and duplicates a parent id in the preload filter. +- [x] 11.7 Note in review that roots are still never added to `expandedIdsRef` under `startExpanded = false`, so a child added to a _root_ after round2 locks in is not swept. Pre-existing in the parallel branch, deliberately out of scope — see `design.md` D5. + +## 12. Test reconciliation + +Both branches edited `useIncrementalTreeData.spec.ts` and `useInfiniteTreeNode.spec.ts`. The merged model changes what several tests can assert, so these are ported deliberately rather than concatenated. + +- [x] 12.1 Port the parallel branch's three ordering tests (roots reordered, children reordered, expansion state preserved across a reorder). The third asserted `EXPANDED` only after a second render because nodes used to start in `LOADING`; under group 1 they are `EXPANDED` from the first render, so the extra `rerender` is now redundant rather than required. +- [x] 12.2 Port the parallel branch's four expansion-survival tests (reload with `items: undefined`, config-instance churn, item removal, user-collapsed node under `startExpanded: true`). +- [x] 12.3 Keep group 3's rewritten `LOADING` tests as the authority on node-creation state — do **not** restore the parallel branch's copies, which still assert `LOADING` on first render. +- [x] 12.4 Port the parallel branch's `requestedParentIds(setFilter)` helper and its `appendItems` preload tests, including the "does not ask for the same parent twice" test that asserts the filter's parent ids are a set (this is what task 11.6's guard protects). +- [x] 12.5 Port the two late-arrival tests. Confirm the first one — "pre-loads children that were not known when the node was expanded" — passes only after task 11.3's restructure; it is the test that fails on a naive merge, and it is the regression test for that specific collision. +- [x] 12.6 Re-run group 7.7's two cascade tests unchanged and confirm both still pass: `startExpanded = true` unbounded cascade, and `startExpanded = false` capped at exactly 2 rounds. The latter is the one at risk from task 11.3 — verify the sweep is a genuine no-op there (no `appendItems`, so `expandedIdsRef` is empty) rather than merely appearing to pass. +- [x] 12.7 Add `@mendix/widget-plugin-test-utils` to `devDependencies` and refresh `pnpm-lock.yaml`. `useInfiniteTreeNode.spec.ts` already imports it (and now needs `dynamic` as well as `listReference`); it resolved via hoisting only. +- [x] 12.8 Run the full package suite (`pnpm run test`) and confirm it is green, then `pnpm run lint`. + +## 13. Live re-verification (mandatory — D5 changes `setFilter` timing) + +Task 11.3 changes `setFilter` call timing and count in `useInfiniteTreeNode.ts`, which is exactly the category task 7.1 proved cannot be trusted on mocked unit tests alone. All four scenarios must be re-confirmed live against `~/Documents/_tickets/WC-3564-2` before this change is done. + +- [ ] 13.1 Rebuild against the repro project and re-confirm WC-3564 Bug 1 (microflow datasource, "Start expanded" = Yes): fully expanded on load, zero `.widget-tree-node-loading-spinner` elements, no console errors. +- [ ] 13.2 Re-confirm WC-3564 Bug 2: expanding "Top level 2" leaves the sibling nodes' expand icons intact. +- [ ] 13.3 Re-confirm the "Expanded bug" tab's 4-tier cascade still resolves fully on load with no manual toggling. +- [ ] 13.4 Re-confirm the "Collapsed bug" tab (`startExpanded = false`): single-click expand still reveals a deeper tier's icon, and the restructured round1/round2/sweep pass does not over-fetch. +- [ ] 13.5 Verify the two newly-fixed bugs live: change a sequence attribute via a microflow and confirm the tree reorders without reopening the page; expand several nodes, trigger a refresh, and confirm they stay expanded and the tree does not flash the empty message. +- [x] 13.6 Verify D6 live on `treenodev2_advanced` (`http://localhost:8080/p/treenodev2_advanced`): select department Finance, and confirm `.mx-name-treeNode1` (`startExpanded = No`) shows `Sports` with a working expand icon revealing `Fitness Equipment`, and that `Tablets`/`Android` (children of the previous set's parents) are gone. Then switch to IT and back at least once more, confirming each switch behaves identically. + + **Confirmed.** Fresh load (HR): `treeNode1` shows `Electronics` with icon, clickable header and `aria-expanded="false"`, and `Phones` — one level past the visible root — already carries its own icon. After selecting Finance, `treeNode1` is exactly `Books` (no icon, genuinely childless — `treeNode2` under `startExpanded = Yes` shows none either) and `Sports` with `aria-expanded="false"`, an icon and a clickable header; clicking it expands to `Fitness Equipment`. No node from the previous set (`Electronics`/`Phones`/`Tablets`) remains. Switched Finance → HR → Finance → IT; every switch behaved identically. Zero spinners left on the page, no console errors or warnings. + +- [x] 13.7 In the same session, confirm the retrieve does not accumulate: after several department switches and expands, the parent ids in the outgoing filter reflect only the current tree, not every set seen so far. + + **Confirmed** by hooking `window.fetch` and reading the `extraXpath` of every `runtimeOperation` across three consecutive switches. Per widget per switch the sequence is: one retrieve still carrying the _previous_ department's parent list (the app-level constraint changes before the widget can re-derive — unavoidable and harmless), then `[not(MyFirstModule.Category_Parent/MyFirstModule.Category)]` alone, then roots + one level (2 parent ids), then 3. The parent list resets to root-only on every switch instead of growing — the tree rebuild empties `treeData`, so the derived set falls back to `{undefined}` and re-derives from the new delivery. Four retrieves per widget per switch, steady across switches, bounded by the tree's depth rather than by history. + +## 14. Changelog and domain notes + +- [x] 14.1 Merge the parallel branch's three `CHANGELOG.md` entries into this change's existing `[Unreleased] / Fixed` block — user-visible behaviour only, no implementation details. +- [x] 14.2 Update `CONTEXT.md`: the `LOADING` section should state that expansion state is now remembered by id across rebuilds, and the preload section should describe the third mechanism (the late-arrival sweep) alongside `appendItems` and the bootstrap rounds, including why the round gates must not return early. +- [x] 14.3 Add a decisions-log entry to `CONTEXT.md` recording that `tmp/treenode-fix1` was folded into this change rather than merged separately, and that `resolveRestoredState` was deliberately dropped. + +## 15. Eighth finding: the preload filter is frozen across a result-set replacement (D6) + +Found live on `treenodev2_advanced` while verifying group 13: after a gallery department switch, every node in the `startExpanded = No` tree is permanently inert. Root cause is the mechanism groups 7 and 11 built — one-shot round flags plus never-pruned parent maps. Replaces that mechanism with a derived parent set rather than patching it with a "was the result set replaced?" detector. See `design.md` D6. + +- [x] 15.1 Add a pure `deriveDesiredParentIds(treeData, deliveredIds, startExpanded)` helper to `hooks/helpers.ts`: walk from the roots, treat a node as visible when every ancestor is `EXPANDED` (roots always visible; under `startExpanded = true` every node qualifies), and collect `{undefined}` plus every visible node plus every child of a visible node. Intersect the result with `deliveredIds` so no node retained from an older delivery contributes. + + **Two deviations, both deliberate.** (a) The `startExpanded` parameter was dropped — the signature is `deriveDesiredParentIds(treeData, deliveredIds)`. `treeNodeState` already encodes it (nodes are created `EXPANDED` under `startExpanded = Yes`), so reading the prop again would double-source the same truth and let the two disagree after a user collapse. (b) Recursion continues through `COLLAPSED_WITH_CSS` as well as `EXPANDED`, not `EXPANDED` alone. Found while wiring it up: that state means the node was opened and then closed, so its subtree is still rendered (hidden by CSS). Recursing only through `EXPANDED` would shrink the derived set on a collapse, drop already-fetched grandchildren from the result set, trip the removed-ids rebuild, and leave the re-expanded node's children iconless — a new bug of exactly the reported kind. Walk order is BFS so the emitted ids read roots-first. + +- [x] 15.2 Rewire `TreeNodeV2`: call `useIncrementalTreeData(props.datasource.items, treeConfig)` directly and pass the resulting `treeData` into the preload hook. The `items` passthrough out of `useInfiniteTreeNodes` (it returned `datasource.items` verbatim) goes away. +- [x] 15.3 Rewrite `useInfiniteTreeNode.ts` around the derived set: delete `loadedParentsByIdRef`, `loadedChildsByIdRef`, `expandedIdsRef`, `round1DoneRef` and `round2DoneRef`; add `lastAppliedParentIdsRef`; apply `setFilter` only when the derived set differs from it. Guards: skip entirely while `datasource.items` is `undefined`. (Landed as `lastAppliedKeyRef` — a sorted-id string key rather than a set, so the comparison is one `===`.) +- [x] 15.4 Keep the one-time `initializedRef` block exactly as-is — root-only filter when `startExpanded` is No, no filter at all when Yes. The asymmetry is load-bearing: the unfiltered first delivery returns a whole non-microflow table in one retrieve, and the set derived from it is immediately stable. Deriving from an empty tree instead would cost one retrieve per level. +- [x] 15.5 Replace `appendItems(newItem, children)` with an argument-less `syncPreloadFilter()`; update the click handler in `TreeNode.tsx` to call it after mutating `node.treeNodeState`. The expansion state it used to be passed is already on the node. (The collapse arm deliberately does not call it: `COLLAPSED_WITH_CSS` keeps the subtree rendered, so the derived set is unchanged.) +- [x] 15.6 Keep `getDatasourceFilter`'s single-vs-`or` shape, and keep `undefined` in the set unconditionally so true roots are always retrieved. +- [x] 15.7 Confirm by reading, not just by test, that the three mechanisms this deletes are genuinely subsumed: round1 = nothing expanded; round2 = the children-of-visible term; the late-arrival sweep = recomputation on the delivery that carries the late child. Any one of them not falling out of the rule means the rule is wrong, not that the flag should come back. **Confirmed**, and the `startExpanded = Yes` cascade too: every level is created `EXPANDED`, so each arriving level widens the visible set and the next derivation asks one level further — the same self-terminating walk, without a flag. Roots are covered by `{undefined}` plus the children-of-visible term, which also closes the sweep's documented "a child added to a root is never swept" gap, since roots no longer need to be registered as expanded for the lookahead to reach them. +- [x] 15.8 Remove the round1/round2/sweep comments from the hook; that reasoning now lives in `design.md` D3/D5/D6 as decision records, not in the code. +- [x] 15.9 Confirm no `LOADING` is reintroduced anywhere, and that `src/components/v1/` is untouched. +- [x] 15.10 `pnpm run test` and `pnpm run lint` in the package. (86/86 green; `tsc --noEmit` clean; auto-lint hook reported nothing.) + +## 16. Test reconciliation for D6 + +- [x] 16.1 Unit-test `deriveDesiredParentIds` directly (it is pure): nothing expanded ⇒ roots + their children; one node expanded ⇒ plus that node's grandchildren; everything expanded ⇒ the whole delivered tree; an item whose parent is not delivered is _not_ counted as a root; ids absent from the delivery are excluded. (New file `hooks/__tests__/helpers.spec.ts`, 9 cases — the listed five plus `COLLAPSED_WITH_CSS` recursion, the `COLLAPSED_WITH_JS` stop, deduplication, and the empty tree.) +- [x] 16.2 Rewrite group 7.7's and 12.6's round-cap tests as derivation tests. The "`startExpanded = false` caps at exactly 2 rounds" test becomes "asks for roots plus one level and nothing further until a real expand" — same observable behaviour, no round counting. +- [x] 16.3 Keep the `startExpanded = true` unbounded-cascade test asserting the same observable outcome (each newly-arriving level triggers exactly one more `setFilter`, a repeated or empty delivery triggers none). Call counts (0, 0, 0, 1, 1, 2, 3, 3) are unchanged from the pre-D6 cascade test. +- [x] 16.4 Re-run groups 12.4 and 12.5 unchanged — the `appendItems` preload tests and both late-arrival tests. They must pass without modification beyond the `syncPreloadFilter` rename; they are the regression guarantee that D6 did not quietly drop D5's fix. + + **The "without modification" premise did not hold, and that was predictable rather than a warning sign.** Every _assertion_ in those tests survives verbatim (`[undefined, "root", "child"]`, `[undefined, "root", "child", "grandchild"]`, `[undefined, "leaf"]`, and both late-arrival tests' expectations), but their _setup_ had to change: the hook's input went from `props` alone to `(props, treeData)`, so a test that used to hand `appendItems` a node and its children now builds the tree that node would be in. The whole point of D6 is that the input is the tree, so a test cannot drive it without one. What matters for the guarantee — that the same observable filters come out — is intact. + + Two tests were dropped rather than ported, both because they asserted the old mechanism rather than the behaviour: "returns datasource items" (the hook no longer passes `items` through, task 15.2) and "does not add duplicate entries when same parent expanded twice", which asserted the `setFilter` call count _increases_ on a re-expand. Under D6 the key guard suppresses that call, which is the improvement 16.6 asserts; the deduplication it meant to check is covered directly in 16.1 and by "does not ask for the same parent twice". + +- [x] 16.5 New test: deliver result set A, then a disjoint result set B, and assert the widget requests B's roots as parents. This is the reported bug's unit-level regression test, and it fails on the pre-D6 code. (Two tests: the single switch, plus three consecutive replacements behaving identically.) +- [x] 16.6 New test: a repeated identical delivery triggers zero further `setFilter` calls (the `lastAppliedParentIdsRef` guard, and the backstop against the explicit tree → filter → tree loop). +- [x] 16.7 New test: an id that leaves the delivery leaves the applied filter, and a node retained from an older delivery never contributes its (stale) object reference to the filter. +- [x] 16.8 Full package suite green, then lint. (95/95 across 6 suites; `tsc --noEmit` clean.) + +## 17. Domain notes for D6 + +- [x] 17.1 Rewrite `CONTEXT.md`'s preload section (`useInfiniteTreeNode.ts`'s one-level-lookahead preload — content-gated, not fire-count-gated): the mechanism it describes no longer exists. State the derived rule, the root definition, and why deriving replaced logging. (Also corrected the `hasChildren` section's reference to the now-deleted `loadedChildsByIdRef`.) +- [x] 17.2 Add a decisions-log entry to `CONTEXT.md`: four bugs in this mechanism all traced to delivery-history state standing in for a derived fact, so the filter is now derived from the current tree. Note the rejected alternative (a result-set-replacement detector) and why. (Two entries: the replacement itself, and the dropped `startExpanded` parameter.) +- [x] 17.3 Check whether the existing `[Unreleased] / Fixed` changelog entries already cover the user-visible effect ("expand icons are correct after the tree's data is filtered or replaced"); add one if not. Behaviour only, no mechanism. **They did not** — the closest entry is about a child arriving late, which is a different cause. Added one about nodes having no expand icon after the data source is filtered elsewhere on the page. Also removed three entries that the branch fold had duplicated verbatim. diff --git a/packages/pluggableWidgets/tree-node-web/openspec/specs/tree-node-data-refresh/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/specs/tree-node-data-refresh/spec.md new file mode 100644 index 0000000000..e397ea1158 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/specs/tree-node-data-refresh/spec.md @@ -0,0 +1,64 @@ +# tree-node-data-refresh Specification + +## Purpose + +Governs how the v2 incremental tree map reacts to a datasource update — sibling and root ordering following the current datasource order, the tree surviving a reload rather than being torn down and rebuilt, and a replaced result set not leaving rows from the previous one behind. + +## Requirements + +### Requirement: Sibling and root order always follows the current datasource order + +The v2 Tree Node widget SHALL order root nodes, and each node's children, according to the order in which the datasource currently delivers those items — re-applied on every datasource update, not captured once when a node's id is first seen. A node whose incremental tree map already contains an id MUST still be re-positioned among its siblings when the datasource's order for that id changes. + +#### Scenario: Root nodes are re-ordered when the datasource returns them in a new order + +- **WHEN** the datasource re-delivers the same root items in a different order (for example after a microflow changed a sequence attribute that the datasource sorts on) +- **THEN** the widget renders the root nodes in the new datasource order, without requiring the page to be reopened or the widget to remount + +#### Scenario: A node's children are re-ordered when the datasource returns them in a new order + +- **WHEN** the datasource re-delivers the same child items of an already-known parent in a different order +- **THEN** the widget renders that parent's children in the new datasource order + +#### Scenario: Re-ordering does not disturb node state + +- **WHEN** sibling order changes across a datasource update +- **THEN** each node keeps its own expanded/collapsed state, its title, and its already-placed children — only its position among its siblings changes + +#### Scenario: A node absent from the current delivery keeps a stable position + +- **WHEN** a node is present in the tree but its id is not in the current datasource delivery (so the datasource expresses no order for it) +- **THEN** the widget keeps that node ordered after every node the current delivery does order, preserving the absent nodes' relative order among themselves rather than reordering them arbitrarily + +### Requirement: The rendered tree is preserved while the datasource is reloading + +The v2 Tree Node widget SHALL treat an undefined `datasource.items` as "not yet delivered" and keep the tree it has already built, rather than interpreting it as an empty result. A reload MUST NOT clear the tree, discard the node map, or surface the empty-message state for the duration of the load. + +#### Scenario: A reload does not empty the tree + +- **WHEN** the datasource starts reloading and `items` becomes undefined +- **THEN** the widget keeps rendering the nodes it had before the reload, with their existing expanded/collapsed state, and does not show the "no data available" empty message + +#### Scenario: An undefined delivery is not mistaken for removed items + +- **WHEN** `items` is undefined +- **THEN** the widget does not conclude that every previously-known item was removed, and therefore does not trigger a rebuild of the node map for that reason + +#### Scenario: The tree updates once the reload settles + +- **WHEN** the reload completes and the datasource delivers items again +- **THEN** the widget applies the new items — including any additions, removals, and the current datasource order — to the tree it preserved + +### Requirement: A replaced result set does not leave rows from the previous one behind + +The v2 Tree Node widget SHALL NOT keep requesting children of parents that belong to a superseded result set. When an app-level constraint replaces the datasource's result set, rows that are only present because a previous set's parents are still being requested MUST NOT persist in the tree. + +#### Scenario: Leftover rows from the previous result set disappear + +- **WHEN** an app-level constraint replaces the result set, and the first delivery after that switch still contains items retrieved on account of the previous set's parents +- **THEN** those items are gone from the tree by the time the widget has settled, rather than remaining alongside the new set's genuine roots for the rest of the session + +#### Scenario: Retrieval does not grow across result-set changes + +- **WHEN** the result set is replaced repeatedly during a session +- **THEN** the number of parents the widget asks for reflects only the current tree, and does not accumulate the parents of every set seen so far diff --git a/packages/pluggableWidgets/tree-node-web/openspec/specs/tree-node-expand-state/spec.md b/packages/pluggableWidgets/tree-node-web/openspec/specs/tree-node-expand-state/spec.md new file mode 100644 index 0000000000..236c3283f7 --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/openspec/specs/tree-node-expand-state/spec.md @@ -0,0 +1,164 @@ +# tree-node-expand-state Specification + +## Purpose + +Governs how a v2 tree node decides (a) whether it shows an expand affordance at all, (b) whether it shows a spinner in place of that affordance, and (c) whether it is expanded or collapsed — all independent of datasource re-delivery timing or datasource type (microflow vs. non-microflow). It also governs which parents the widget preloads children for, so that every visible node's expand affordance is correct without a manual collapse-and-reopen. + +## Requirements + +### Requirement: Expand affordance visibility is driven by known children, not by a load-timing-sensitive stored state + +The v2 Tree Node widget SHALL determine whether a node's expand affordance (chevron/icon) is shown based on whether that node currently has any children placed under it (`node.children.length > 0`), computed fresh on every render — never from the `hasChildren` widget property, which is unavailable in v2's configuration mode (`parentAssociation` set), and never from a stored per-node flag that a later, unrelated datasource delivery could incorrectly mutate. + +#### Scenario: A node with children shows an expand affordance + +- **WHEN** a node has at least one child currently placed under it +- **THEN** the widget renders an expand affordance for that node + +#### Scenario: A node with no known children shows no expand affordance (absent a spinner) + +- **WHEN** a node has no children currently placed under it and the datasource is not currently loading +- **THEN** the widget renders no expand affordance for that node + +#### Scenario: Expanding one node does not affect a sibling's expand affordance + +- **WHEN** a user expands node A, causing a datasource redelivery that includes node B's id (node B was never expanded and has no relation to node A) +- **THEN** node B's expand affordance and underlying children are unchanged from before node A was expanded + +### Requirement: A loading spinner is shown only while the datasource is genuinely fetching, never as stored per-node state + +The v2 Tree Node widget SHALL show a loading spinner in place of the expand affordance for a node that has no known children yet, exactly while `datasource.status === ValueStatus.Loading`. This is a render-time computation only — no per-node "is loading" flag is stored, so the spinner cannot become stuck and cannot be affected by an unrelated node's resolution. + +#### Scenario: Spinner shown while the datasource is loading and children are unknown + +- **WHEN** a node has no children placed under it yet and the datasource's `status` is `Loading` +- **THEN** the widget shows a spinner in place of the expand affordance for that node + +#### Scenario: Spinner clears once the datasource settles, regardless of outcome + +- **WHEN** the datasource's `status` transitions away from `Loading` +- **THEN** every node's spinner clears immediately — showing an expand affordance if children arrived, or no affordance at all if they didn't + +#### Scenario: Spinner never shows for a node that already has children + +- **WHEN** a node already has at least one child placed under it +- **THEN** the widget never shows a spinner for that node, regardless of `datasource.status` + +#### Scenario: A stalled or filter-ignoring datasource never produces a stuck spinner + +- **WHEN** a microflow datasource ignores `setFilter` and its `status` never transitions to `Loading` for a given fetch attempt +- **THEN** no node is left showing a spinner indefinitely as a result of that fetch attempt + +### Requirement: A manually expanded node's own children's expand affordance is known without requiring a collapse-and-reopen + +The v2 Tree Node widget SHALL preload one level past a node's children when that node is expanded by a user click, so each child's own expand affordance is already correct the first time its parent is expanded — never requiring the user to collapse and re-expand that same node to reveal it. This preload is bounded to exactly one level past what's already known for this path; it does not eagerly walk the full tree beyond the node that was actually clicked. + +#### Scenario: A node's own children's children are known on its first expand + +- **WHEN** a user expands a node for the first time (its children are already known, but whether those children themselves have children is not) +- **THEN** each of that node's children already shows its correct expand affordance immediately, without requiring that child to be separately collapsed and re-expanded + +### Requirement: Under "Start expanded" = Yes, every auto-expanded level's own expand affordance is known automatically, all the way to the tree's real depth + +Because every node defaults to expanded (not just roots) when "Start expanded" is Yes, the v2 Tree Node widget SHALL keep preloading one level further for as long as new descendants keep appearing — not a fixed number of levels — so that every already-visible node's expand affordance is correct without any manual collapse-and-reopen, regardless of how deep the actual tree data goes. This cascade is self-terminating: it stops automatically once a level introduces no previously-unseen items, bounded by the tree's real depth rather than an arbitrary count or recursing indefinitely. + +#### Scenario: A 3rd (or deeper) tier's own expand affordance is known automatically + +- **WHEN** "Start expanded" is Yes and the underlying data has 3 or more tiers +- **THEN** every tier's nodes show their correct expand affordance immediately on load, with no tier requiring a manual collapse-and-reopen to reveal the next tier down + +#### Scenario: The cascade stops once the real data is exhausted + +- **WHEN** a subsequent datasource delivery introduces no items beyond what's already known +- **THEN** no further automatic preload round is triggered — the cascade does not continue indefinitely or re-fetch unchanged data + +#### Scenario: A transient empty datasource delivery during initial load does not disable the cascade + +- **WHEN** the datasource is still loading and delivers an empty item set one or more times before the real data arrives +- **THEN** the cascade does not lock itself out on that empty delivery — it only advances once it actually finds real, previously-unseen items, and keeps retrying harmlessly until it does + +### Requirement: The auto-cascade does not apply when "Start expanded" is No + +The v2 Tree Node widget SHALL NOT preload beyond visible nodes plus one level of lookahead when "Start expanded" is No, since deeper tiers remain collapsed by default and already resolve correctly via a single real click. Preloading further in this mode would only eagerly fetch descendants of branches the user has not opened. With nothing expanded, "visible nodes plus one level" resolves to exactly the root nodes and their children; it grows only as the user actually expands. + +#### Scenario: A 3rd-tier arrival does not trigger a further automatic round when collapsed by default + +- **WHEN** "Start expanded" is No and a 3rd-tier item arrives as a result of the existing 2-round preload +- **THEN** no further automatic preload round is triggered for it — expanding it further still requires a real click + +### Requirement: A child that arrives after its parent was expanded still restores that parent's expand affordance + +The v2 Tree Node widget SHALL keep preloading one level ahead for nodes the user has already expanded, so a child that becomes known only after the expand — because it was still in flight at expand time, or because it was created later — is still preloaded, and the affected node's expand affordance is still correct. This applies whether or not the node's children were already known when `appendItems` ran for it. + +#### Scenario: Children still in flight at expand time are preloaded once they arrive + +- **WHEN** a user expands a node whose children have not been delivered yet, and those children arrive in a later datasource delivery +- **THEN** the widget preloads those children's own children, so each arriving child shows its correct expand affordance without the user collapsing and re-expanding the parent + +#### Scenario: A child created after the expand is preloaded + +- **WHEN** a node is already expanded with known children, and a microflow adds a further child to that node +- **THEN** the widget preloads the newly-added child's own children, so the new child shows its correct expand affordance as soon as it is rendered + +#### Scenario: Repeated deliveries do not re-request the same parent + +- **WHEN** a datasource delivery leaves the set of parents the widget needs unchanged from the set it last requested +- **THEN** no further preload request is issued for that delivery, and no parent id appears more than once in the preload filter + +### Requirement: A node's expanded or collapsed state survives a tree rebuild + +The v2 Tree Node widget SHALL remember each node's expanded/collapsed state by item id and restore it when that node is re-created during a rebuild of the incremental node map, so a datasource refresh never silently collapses the tree the user had opened. A remembered state MUST take precedence over the `startExpanded` default, in both directions. + +#### Scenario: Expansion survives new prop instances on refresh + +- **WHEN** the Mendix client hands the widget new prop instances on a refresh (which the widget compares by reference and therefore treats as a configuration change, rebuilding the node map) +- **THEN** every re-created node comes back with the expanded/collapsed state it had before the rebuild, not with the `startExpanded` default + +#### Scenario: Expansion of remaining nodes survives an item removal + +- **WHEN** a single item is deleted from the datasource, triggering a rebuild of the node map +- **THEN** the remaining nodes come back with the expanded/collapsed state they had before the removal + +#### Scenario: A user-collapsed node stays collapsed even when "Start expanded" is Yes + +- **WHEN** "Start expanded" is Yes, the user collapses a node, and a later refresh rebuilds the node map +- **THEN** that node comes back collapsed — the remembered state wins over the `startExpanded` default + +#### Scenario: A node never seen before still follows the configured default + +- **WHEN** an item id appears that has no remembered state (a genuinely new node) +- **THEN** that node is created directly in the state `startExpanded` dictates (`EXPANDED` or `COLLAPSED_WITH_JS`) — never in a stored `LOADING` state + +### Requirement: The set of parents to preload is derived from the current tree, never accumulated from delivery history + +The v2 Tree Node widget SHALL determine which parents to request on every datasource delivery by deriving them from the tree as it currently stands — the roots, plus every node all of whose ancestors have their subtree rendered, plus every child of such a node — and SHALL request exactly that set. A node's subtree counts as rendered when the node is expanded, and also when it was expanded and then collapsed again (its body remains in the DOM, hidden), but not when it has never been expanded. It MUST NOT maintain a record of what has already been fetched, a one-shot "preload done" flag, or any other delivery-history state as the basis for that decision. A node counts as a root for this purpose only when it has no parent at all, not merely when its parent is absent from the current delivery. + +#### Scenario: A replaced result set still gets its expand affordance + +- **WHEN** an app-level constraint replaces the datasource's entire result set (for example the user picks a different department in a gallery that filters the tree), producing a set of roots none of which the widget has seen before +- **THEN** the widget requests the new roots' children, and every new root that has children shows its expand affordance — it is not left inert with no icon, no `aria-expanded`, no clickable header and no keyboard expand + +#### Scenario: Consecutive result-set replacements each behave identically + +- **WHEN** the result set is replaced a second and third time in the same session, without the widget remounting +- **THEN** each replacement is treated exactly like the first — there is no round, flag, or budget that a previous replacement can have used up + +#### Scenario: Parents from a previous result set are no longer requested + +- **WHEN** a delivery no longer contains an item that was previously a requested parent +- **THEN** that item's id is absent from the next filter the widget applies, so its children are no longer retrieved + +#### Scenario: Collapsing a node does not drop what was already fetched below it + +- **WHEN** the user collapses a node whose descendants have already been retrieved +- **THEN** the widget requests the same set of parents as before the collapse, and re-expanding that node shows its children with their expand affordances intact + +#### Scenario: An item whose parent is not delivered is not treated as a root + +- **WHEN** a delivered item has a parent association pointing at an object the datasource does not deliver +- **THEN** the widget does not request that item's children on the grounds that it renders at root level, and it therefore shows no expand affordance — the item leaves the tree on the following delivery, once the filter stops asking for its parent + +#### Scenario: Only items in the current delivery are used to build the filter + +- **WHEN** the tree still holds nodes whose ids were not in the current delivery (retained per the data-refresh behaviour) +- **THEN** the filter is built only from items the current delivery provided, so no stale object reference is used to request children diff --git a/packages/pluggableWidgets/tree-node-web/package.json b/packages/pluggableWidgets/tree-node-web/package.json index c4ee4dc679..98c569c94f 100644 --- a/packages/pluggableWidgets/tree-node-web/package.json +++ b/packages/pluggableWidgets/tree-node-web/package.json @@ -23,15 +23,15 @@ }, "testProject": { "githubUrl": "https://github.com/mendix/testProjects", - "branchName": "tree-node-web/tree-node-v2" + "branchName": "tree-node-web/v2" }, "scripts": { "build": "pluggable-widgets-tools build:web", "create-translation": "rui-create-translation", "dev": "pluggable-widgets-tools start:web", - "e2e": "MENDIX_VERSION=11.9.1 run-e2e ci", + "e2e": "MENDIX_VERSION=11.12.0 run-e2e ci", "e2e-update-project": "pnpm --filter @mendix/data-widgets run build:include-deps", - "e2edev": "MENDIX_VERSION=11.9.1 run-e2e dev --with-preps", + "e2edev": "MENDIX_VERSION=11.12.0 run-e2e dev --with-preps", "format": "prettier --ignore-path ./node_modules/@mendix/prettier-config-web-widgets/global-prettierignore --write .", "lint": "eslint src/ package.json", "release": "pluggable-widgets-tools release:web", diff --git a/packages/pluggableWidgets/tree-node-web/src/TreeNode.editorPreview.tsx b/packages/pluggableWidgets/tree-node-web/src/TreeNode.editorPreview.tsx index 44a4c5f312..f8f8c4242f 100644 --- a/packages/pluggableWidgets/tree-node-web/src/TreeNode.editorPreview.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/TreeNode.editorPreview.tsx @@ -29,11 +29,13 @@ export function preview(props: TreeNodePreviewProps): ReactElement | null { ), bodyContent: ( - +
), - isUserDefinedLeafNode: !props.hasChildren + isUserDefinedLeafNode: props.parentAssociation ? false : !props.hasChildren } ]} startExpanded diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v1/TreeNodeBranch.tsx b/packages/pluggableWidgets/tree-node-web/src/components/v1/TreeNodeBranch.tsx index c2eaeb5ecc..89c7875afd 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v1/TreeNodeBranch.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/components/v1/TreeNodeBranch.tsx @@ -136,7 +136,7 @@ export function TreeNodeBranch({ useLayoutEffect(() => { if (animateTreeNodeContentProp && treeNodeState !== TreeNodeState.LOADING) { - const animationCleanup = animateTreeNodeContent(); + const animationCleanup = animateTreeNodeContent(treeNodeState === TreeNodeState.EXPANDED); if (animationCleanup) { return animationCleanup; } @@ -198,6 +198,12 @@ export function TreeNodeBranch({ id={treeNodeBranchUtils.getBodyId(id)} aria-hidden={treeNodeState !== TreeNodeState.EXPANDED} ref={treeNodeBranchBody} + // Keep children out of view while loading so they don't flash before the expand animation + style={ + animateTreeNodeContentProp && treeNodeState === TreeNodeState.LOADING + ? { height: 0 } + : undefined + } onTransitionEnd={cleanupAnimation} > {children} diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v1/hooks/useAnimatedHeight.tsx b/packages/pluggableWidgets/tree-node-web/src/components/v1/hooks/useAnimatedHeight.tsx index 749bc3150e..45c8bd9455 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v1/hooks/useAnimatedHeight.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/components/v1/hooks/useAnimatedHeight.tsx @@ -1,12 +1,12 @@ -import { RefObject, useCallback, useRef, useState } from "react"; +import { RefObject, TransitionEvent, useCallback, useLayoutEffect, useRef, useState } from "react"; export const useAnimatedTreeNodeContentHeight = ( treeNodeBranchBody: RefObject ): { isAnimating: boolean; captureElementHeight: () => void; - animateTreeNodeContent: () => (() => void) | undefined; - cleanupAnimation: () => void; + animateTreeNodeContent: (isExpanding: boolean) => (() => void) | undefined; + cleanupAnimation: (event?: TransitionEvent) => void; } => { const currentElementHeight = useRef(undefined); const [isAnimating, setIsAnimating] = useState(false); @@ -15,29 +15,50 @@ export const useAnimatedTreeNodeContentHeight = ( currentElementHeight.current = treeNodeBranchBody.current?.getBoundingClientRect().height ?? 0; }, []); - const animateTreeNodeContent = useCallback(() => { - if ( - treeNodeBranchBody.current && - currentElementHeight.current !== undefined && - !Number.isNaN(currentElementHeight.current) - ) { - const newElementHeight = treeNodeBranchBody.current.getBoundingClientRect().height; - if (newElementHeight - currentElementHeight.current !== 0) { - setIsAnimating(true); - treeNodeBranchBody.current.style.height = `${currentElementHeight.current}px`; - const timeout = setTimeout(() => { - treeNodeBranchBody.current!.style.height = `${newElementHeight}px`; - currentElementHeight.current = newElementHeight; - }, 1); - return () => clearTimeout(timeout); - } + const cleanupAnimation = useCallback((event?: TransitionEvent) => { + // Ignore transitions bubbling up from nested tree node bodies + if (event && (event.target !== event.currentTarget || event.propertyName !== "height")) { + return; } - }, []); - - const cleanupAnimation = useCallback(() => { setIsAnimating(false); - treeNodeBranchBody.current?.style.removeProperty("height"); }, []); + // Remove the inline height in the same commit that applies the hidden class, to avoid a full-height frame + useLayoutEffect(() => { + if (!isAnimating) { + treeNodeBranchBody.current?.style.removeProperty("height"); + } + }, [isAnimating]); + + const animateTreeNodeContent = useCallback( + (isExpanding: boolean) => { + const element = treeNodeBranchBody.current; + const startHeight = currentElementHeight.current; + currentElementHeight.current = undefined; + if (!element || startHeight === undefined || Number.isNaN(startHeight)) { + return; + } + + // Measure the natural height; mid-animation the inline height would be read instead + element.style.removeProperty("height"); + const targetHeight = isExpanding ? element.getBoundingClientRect().height : 0; + if (targetHeight === startHeight) { + cleanupAnimation(); + return; + } + + setIsAnimating(true); + element.style.height = `${startHeight}px`; + // Wait for the re-render that un-hides a collapsing body before starting the transition + const timeout = setTimeout(() => { + // Force reflow so the start height is committed; otherwise the transition may never start + element.getBoundingClientRect(); + element.style.height = `${targetHeight}px`; + }, 1); + return () => clearTimeout(timeout); + }, + [cleanupAnimation] + ); + return { isAnimating, captureElementHeight, animateTreeNodeContent, cleanupAnimation }; }; diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx b/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx index 24ed6171ba..8ed0db22a6 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/TreeNode.tsx @@ -15,9 +15,13 @@ function renderRecursiveNode( iconPlacement: TreeNodeContainerProps["showIcon"], openNodeOn: TreeNodeContainerProps["openNodeOn"], onNodeClick: (node: TreeNodeV2DataItem) => void, + isDatasourceLoading: boolean, children?: TreeNodeContainerProps["children"] ): ReactElement { const hasChildren = node.children.length > 0; + // We don't yet know whether this node has children (nothing placed under it yet); show a + // spinner only while the datasource is actually fetching, never as a stored/stale node state. + const showSpinner = !hasChildren && isDatasourceLoading; const isExpanded = node.treeNodeState === TreeNodeState.EXPANDED; const isIconClickable = openNodeOn === "iconClick"; const isHeaderClickable = openNodeOn === "headerClick"; @@ -46,14 +50,14 @@ function renderRecursiveNode( onClick={onHeaderClick} > {node.title} - {(hasChildren || node.treeNodeState === TreeNodeState.LOADING) && iconPlacement !== "no" && ( + {(hasChildren || showSpinner) && iconPlacement !== "no" && ( - {renderHeaderIcon(node.treeNodeState, iconPlacement)} + {renderHeaderIcon(showSpinner ? TreeNodeState.LOADING : node.treeNodeState, iconPlacement)} )} @@ -75,6 +79,7 @@ function renderRecursiveNode( iconPlacement, openNodeOn, onNodeClick, + isDatasourceLoading, children )} @@ -88,7 +93,6 @@ function renderRecursiveNode( } export function TreeNodeV2(props: TreeNodeContainerProps): ReactElement { - const { items, appendItems } = useInfiniteTreeNodes(props); const [, forceRender] = useState(0); const expandedIcon = props.expandedIcon?.status === ValueStatus.Available ? props.expandedIcon.value : undefined; @@ -119,23 +123,26 @@ export function TreeNodeV2(props: TreeNodeContainerProps): ReactElement { [props.headerCaption, props.headerContent, props.headerType, props.parentAssociation, props.startExpanded] ); - const treeData = useIncrementalTreeData(items, treeConfig); + const treeData = useIncrementalTreeData(props.datasource.items, treeConfig); + // The preload filter is derived from the tree, so it has to be given the tree, and the click + // handler has to ask for a re-derivation after it mutates a node's state. + const { syncPreloadFilter } = useInfiniteTreeNodes(props, treeData); + const isDatasourceLoading = props.datasource.status === ValueStatus.Loading; const onNodeClick = useCallback( (node: TreeNodeV2DataItem) => { if (node.treeNodeState === TreeNodeState.EXPANDED) { + // Collapsing leaves the subtree rendered (hidden by CSS), so what the tree needs + // from the datasource is unchanged — no re-derivation. node.treeNodeState = TreeNodeState.COLLAPSED_WITH_CSS; forceRender(version => version + 1); return; } node.treeNodeState = TreeNodeState.EXPANDED; - appendItems( - node.item, - node.children.map(child => child.item) - ); + syncPreloadFilter(); forceRender(version => version + 1); }, - [appendItems] + [syncPreloadFilter] ); if (treeData.length === 0) { @@ -160,6 +167,7 @@ export function TreeNodeV2(props: TreeNodeContainerProps): ReactElement { iconPlacement, props.openNodeOn, onNodeClick, + isDatasourceLoading, props.children ) )} diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx b/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx index 5e378b30d7..474c09d60b 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/__tests__/TreeNodeV2.spec.tsx @@ -20,72 +20,72 @@ jest.mock("mendix/filters/builders", () => ({ or: jest.fn((...args: unknown[]) => ({ type: "or", args })) })); -describe("TreeNodeV2 - Keyboard Navigation", () => { - const makeItem = (id: string): ObjectItem => ({ id: id as GUID }); - - const makeListValue = (items: ObjectItem[]): ListValue => - ({ - status: ValueStatus.Available, - items, - limit: 100, - offset: 0, - hasMoreItems: false, - sortOrder: [], - filter: undefined, - setLimit: jest.fn(), - setOffset: jest.fn(), - setSortOrder: jest.fn(), - requestTotalCount: jest.fn(), - setFilter: jest.fn(), - reload: jest.fn(), - totalCount: undefined - }) as unknown as ListValue; - - const makeExpression = (value: string): ListExpressionValue => ({ - get: (): DynamicValue => ({ status: ValueStatus.Available, value }) - }); +const makeItem = (id: string): ObjectItem => ({ id: id as GUID }); + +const makeListValue = (items: ObjectItem[]): ListValue => + ({ + status: ValueStatus.Available, + items, + limit: 100, + offset: 0, + hasMoreItems: false, + sortOrder: [], + filter: undefined, + setLimit: jest.fn(), + setOffset: jest.fn(), + setSortOrder: jest.fn(), + requestTotalCount: jest.fn(), + setFilter: jest.fn(), + reload: jest.fn(), + totalCount: undefined + }) as unknown as ListValue; + +const makeExpression = (value: string): ListExpressionValue => ({ + get: (): DynamicValue => ({ status: ValueStatus.Available, value }) +}); - const makeBoolExpression = (value: boolean): ListExpressionValue => ({ - get: (): DynamicValue => ({ status: ValueStatus.Available, value }) - }); +const makeBoolExpression = (value: boolean): ListExpressionValue => ({ + get: (): DynamicValue => ({ status: ValueStatus.Available, value }) +}); - /** - * Creates a ListReferenceValue mock where childId → parentId, all others → undefined. - */ - const makeParentAssociation = (childId: string, parentId: string): ListReferenceValue => - ({ - id: "parentAssoc", - type: "Reference", - get: (item: ObjectItem): DynamicValue => { - if (String(item.id) === childId) { - return { status: ValueStatus.Available, value: makeItem(parentId) }; - } - return { status: ValueStatus.Available, value: undefined as unknown as ObjectItem }; +/** + * Creates a ListReferenceValue mock where childId → parentId, all others → undefined. + */ +const makeParentAssociation = (childId: string, parentId: string): ListReferenceValue => + ({ + id: "parentAssoc", + type: "Reference", + get: (item: ObjectItem): DynamicValue => { + if (String(item.id) === childId) { + return { status: ValueStatus.Available, value: makeItem(parentId) }; } - }) as unknown as ListReferenceValue; - - /** - * Default props for tests that need a node with children. - * Datasource contains parent + child; parentAssociation links child → parent. - * This makes node.children.length > 0 so aria-expanded is rendered. - */ - const makeDefaultProps = (startExpanded = false): TreeNodeContainerProps => ({ - name: "treeNode", - class: "", - tabIndex: 0, - advancedMode: false, - datasource: makeListValue([makeItem("1"), makeItem("2")]), - parentAssociation: makeParentAssociation("2", "1"), - headerType: "text", - headerCaption: makeExpression("Node"), - hasChildren: makeBoolExpression(true), - showIcon: "right", - openNodeOn: "headerClick", - animate: false, - animateIcon: false, - startExpanded - }); + return { status: ValueStatus.Available, value: undefined as unknown as ObjectItem }; + } + }) as unknown as ListReferenceValue; + +/** + * Default props for tests that need a node with children. + * `hasChildren` (not the datasource) is what drives aria-expanded; the + * datasource's parent + child items exist so the expanded body actually renders content. + */ +const makeDefaultProps = (startExpanded = false): TreeNodeContainerProps => ({ + name: "treeNode", + class: "", + tabIndex: 0, + advancedMode: false, + datasource: makeListValue([makeItem("1"), makeItem("2")]), + parentAssociation: makeParentAssociation("2", "1"), + headerType: "text", + headerCaption: makeExpression("Node"), + hasChildren: makeBoolExpression(true), + showIcon: "right", + openNodeOn: "headerClick", + animate: false, + animateIcon: false, + startExpanded +}); +describe("TreeNodeV2 - Keyboard Navigation", () => { it("expands node when Enter key is pressed", () => { render(createElement(TreeNodeV2, makeDefaultProps(false))); const treeItem = screen.getAllByRole("treeitem")[0]; @@ -204,3 +204,106 @@ describe("TreeNodeV2 - Keyboard Navigation", () => { expect(parentItem.getAttribute("aria-expanded")).toBe(initialState); }); }); + +describe("TreeNodeV2 - Loading state (WC-3564 regressions)", () => { + const spinner = (container: HTMLElement): Element | null => + container.querySelector(".widget-tree-node-loading-spinner"); + + const makeListValueWithStatus = (items: ObjectItem[], status: ValueStatus): ListValue => + ({ ...makeListValue(items), status }) as unknown as ListValue; + + it("never shows a stuck spinner, even when the datasource keeps redelivering the same full item set (Bug 1)", () => { + // "1" genuinely has a child ("2"), so its expand affordance should resolve immediately, not depend on a later delivery. + const props: TreeNodeContainerProps = { + ...makeDefaultProps(true), + datasource: makeListValue([makeItem("1"), makeItem("2")]), + parentAssociation: makeParentAssociation("2", "1") + }; + + const { container, rerender } = render(createElement(TreeNodeV2, props)); + expect(spinner(container)).toBeNull(); + expect(screen.getAllByRole("treeitem")[0]).toHaveAttribute("aria-expanded", "true"); + + // Simulate a microflow datasource ignoring setFilter and redelivering + // the exact same full result on a later render (new array reference). + rerender( + createElement(TreeNodeV2, { + ...props, + datasource: makeListValue([makeItem("1"), makeItem("2")]) + }) + ); + + expect(spinner(container)).toBeNull(); + expect(screen.getAllByRole("treeitem")[0]).toHaveAttribute("aria-expanded", "true"); + }); + + it("shows a spinner while the datasource is actually loading and no children are known yet", () => { + const noParent = makeParentAssociation("__none__", "__none__"); + const props: TreeNodeContainerProps = { + ...makeDefaultProps(false), + datasource: makeListValueWithStatus([makeItem("1")], ValueStatus.Loading), + parentAssociation: noParent + }; + + const { container } = render(createElement(TreeNodeV2, props)); + expect(spinner(container)).not.toBeNull(); + expect(screen.getByRole("treeitem")).not.toHaveAttribute("aria-expanded"); + }); + + it("clears the spinner once the datasource settles, even if it turns out the node has no children", () => { + const noParent = makeParentAssociation("__none__", "__none__"); + const props: TreeNodeContainerProps = { + ...makeDefaultProps(false), + datasource: makeListValueWithStatus([makeItem("1")], ValueStatus.Loading), + parentAssociation: noParent + }; + + const { container, rerender } = render(createElement(TreeNodeV2, props)); + expect(spinner(container)).not.toBeNull(); + + rerender( + createElement(TreeNodeV2, { + ...props, + datasource: makeListValueWithStatus([makeItem("1")], ValueStatus.Available) + }) + ); + + expect(spinner(container)).toBeNull(); + expect(screen.getByRole("treeitem")).not.toHaveAttribute("aria-expanded"); + }); + + it("resolving one node's children does not affect an unrelated sibling's spinner or state (Bug 2)", () => { + const parentAssociation = makeParentAssociation("C", "A"); + const props: TreeNodeContainerProps = { + ...makeDefaultProps(false), + datasource: makeListValueWithStatus([makeItem("A"), makeItem("B")], ValueStatus.Loading), + parentAssociation + }; + + const { container, rerender } = render(createElement(TreeNodeV2, props)); + const [nodeA, nodeB] = screen.getAllByRole("treeitem"); + expect(nodeA).not.toHaveAttribute("aria-expanded"); + expect(nodeB).not.toHaveAttribute("aria-expanded"); + // Both spin while nothing is known yet and the datasource is loading. + expect(container.querySelectorAll(".widget-tree-node-loading-spinner")).toHaveLength(2); + + // The datasource settles, delivering a child for A only. B was never involved. + rerender( + createElement(TreeNodeV2, { + ...props, + datasource: makeListValueWithStatus( + [makeItem("A"), makeItem("B"), makeItem("C")], + ValueStatus.Available + ) + }) + ); + + const [nodeAAfter, nodeBAfter] = screen.getAllByRole("treeitem"); + expect(nodeAAfter).toHaveAttribute("aria-expanded", "false"); + expect(nodeAAfter.querySelector(".widget-tree-node-branch-header-icon-container")).not.toBeNull(); + // B has no children and the datasource is no longer loading — no icon, no spinner, untouched by A's resolution. + expect(nodeBAfter).not.toHaveAttribute("aria-expanded"); + expect(nodeBAfter.querySelector(".widget-tree-node-branch-header-icon-container")).toBeNull(); + expect(container.querySelectorAll(".widget-tree-node-loading-spinner")).toHaveLength(0); + }); +}); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/helpers.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/helpers.spec.ts new file mode 100644 index 0000000000..7b91201a0f --- /dev/null +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/helpers.spec.ts @@ -0,0 +1,120 @@ +import { ObjectItem } from "mendix"; +import { TreeNodeState } from "../../../common/TreeNodeState"; +import { deriveDesiredParentIds } from "../helpers"; +import { TreeNodeV2DataItem } from "../useIncrementalTreeData"; + +interface NodeSpec { + id: string; + state?: TreeNodeState; + children?: NodeSpec[]; + /** Only for orphans: the datasource renders them at root level, but they are not roots. */ + orphanOf?: string; +} + +function makeTree(specs: NodeSpec[], parentId?: string): TreeNodeV2DataItem[] { + return specs.map(spec => ({ + children: makeTree(spec.children ?? [], spec.id), + id: spec.id, + item: { id: spec.id } as ObjectItem, + parentId: spec.orphanOf ?? parentId, + treeNodeState: spec.state ?? TreeNodeState.COLLAPSED_WITH_JS, + title: spec.id + })); +} + +function allIds(nodes: TreeNodeV2DataItem[]): string[] { + return nodes.flatMap(node => [node.id, ...allIds(node.children)]); +} + +/** Derives over a tree whose every node the delivery carried. */ +function derive(specs: NodeSpec[]): string[] { + const tree = makeTree(specs); + return deriveDesiredParentIds(tree, new Set(allIds(tree))); +} + +const expanded = TreeNodeState.EXPANDED; + +describe("deriveDesiredParentIds", () => { + it("asks for the roots and one level past them when nothing is expanded", () => { + // The roots are visible, so their children must be retrievable; those children need their + // own children known too, or they render without an expand icon. + expect( + derive([{ id: "root1", children: [{ id: "child1", children: [{ id: "grandchild1" }] }] }, { id: "root2" }]) + // each node is followed by the children it contributes; the filter itself is a set + ).toEqual(["root1", "child1", "root2"]); + }); + + it("reaches one level further for each node that is expanded", () => { + expect( + derive([{ id: "root", state: expanded, children: [{ id: "child", children: [{ id: "grandchild" }] }] }]) + ).toEqual(["root", "child", "grandchild"]); + }); + + it("asks for the whole tree when every node is expanded", () => { + const tree = [ + { + id: "root", + state: expanded, + children: [{ id: "child", state: expanded, children: [{ id: "grandchild", state: expanded }] }] + } + ]; + + expect(derive(tree)).toEqual(["root", "child", "grandchild"]); + }); + + it("keeps the subtree of a node that was opened and then closed", () => { + // COLLAPSED_WITH_CSS still renders its body (hidden), so dropping its descendants here would + // remove them from the tree and leave them iconless on reopen. + expect( + derive([ + { + id: "root", + state: TreeNodeState.COLLAPSED_WITH_CSS, + children: [{ id: "child", state: expanded, children: [{ id: "grandchild" }] }] + } + ]) + ).toEqual(["root", "child", "grandchild"]); + }); + + it("stops at a node whose body was never rendered", () => { + expect( + derive([ + { + id: "root", + children: [ + { id: "child", state: TreeNodeState.COLLAPSED_WITH_JS, children: [{ id: "grandchild" }] } + ] + } + ]) + ).toEqual(["root", "child"]); + }); + + it("does not treat an item whose parent was not delivered as a root", () => { + // An orphan is promoted to root level for rendering, but making it a root here would make + // the derived set depend on what was delivered, which the filter itself decides. + const result = derive([ + { id: "root", children: [{ id: "child" }] }, + { id: "orphan", orphanOf: "absent", children: [{ id: "orphanChild" }] } + ]); + + expect(result).toEqual(["root", "child"]); + }); + + it("excludes ids the current delivery did not carry", () => { + const tree = makeTree([{ id: "root", state: expanded, children: [{ id: "fresh" }, { id: "retained" }] }]); + + expect(deriveDesiredParentIds(tree, new Set(["root", "fresh"]))).toEqual(["root", "fresh"]); + }); + + it("returns each id once even when the same node is reachable twice", () => { + const result = derive([ + { id: "root", state: expanded, children: [{ id: "child", state: expanded, children: [{ id: "leaf" }] }] } + ]); + + expect(result).toEqual([...new Set(result)]); + }); + + it("returns nothing for an empty tree", () => { + expect(deriveDesiredParentIds([], new Set())).toEqual([]); + }); +}); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts index f876b758e6..1a1eb33fcb 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts @@ -70,7 +70,7 @@ describe("useIncrementalTreeData", () => { expect(result.current[0].children[0].id).toBe("child"); }); - it("assigns LOADING on first render, then COLLAPSED_WITH_JS when startExpanded is false", () => { + it("assigns COLLAPSED_WITH_JS on first render when startExpanded is false, and never enters LOADING on redelivery (WC-3564 Bug 1)", () => { const items = [makeItem("a")]; const config = makeConfig({ startExpanded: false }); const { result, rerender } = renderHook( @@ -78,13 +78,13 @@ describe("useIncrementalTreeData", () => { useIncrementalTreeData(items, config), { initialProps: { items, config } } ); - expect(result.current[0].treeNodeState).toBe(TreeNodeState.LOADING); - // Simulate Mendix re-providing items (new array reference) + expect(result.current[0].treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + // Simulate a microflow datasource redelivering the same full result (new array reference). rerender({ items: [...items], config }); expect(result.current[0].treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); }); - it("assigns LOADING on first render, then EXPANDED when startExpanded is true", () => { + it("assigns EXPANDED on first render when startExpanded is true, and stays EXPANDED on redelivery (WC-3564 Bug 1)", () => { const items = [makeItem("a")]; const config = makeConfig({ startExpanded: true }); const { result, rerender } = renderHook( @@ -92,11 +92,31 @@ describe("useIncrementalTreeData", () => { useIncrementalTreeData(items, config), { initialProps: { items, config } } ); - expect(result.current[0].treeNodeState).toBe(TreeNodeState.LOADING); - // Simulate Mendix re-providing items (new array reference) + expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); + // Simulate a microflow datasource redelivering the same full result (new array reference). rerender({ items: [...items], config }); expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); }); + + it("leaves an unrelated sibling's state untouched when a node's children arrive later (WC-3564 Bug 2)", () => { + const parent = makeItem("parent"); + const sibling = makeItem("sibling"); + const config = makeConfigWithParentMap({ child: "parent" }, { startExpanded: false }); + + const { result, rerender } = renderHook( + ({ items }: { items: ObjectItem[] }) => useIncrementalTreeData(items, config), + { initialProps: { items: [parent, sibling] } } + ); + + expect(result.current.find(n => n.id === "parent")!.treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + expect(result.current.find(n => n.id === "sibling")!.treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + + // "parent"'s child arrives later; "sibling" was never involved. + rerender({ items: [parent, sibling, makeItem("child")] }); + expect(result.current.find(n => n.id === "parent")!.children).toHaveLength(1); + expect(result.current.find(n => n.id === "sibling")!.treeNodeState).toBe(TreeNodeState.COLLAPSED_WITH_JS); + expect(result.current.find(n => n.id === "sibling")!.children).toHaveLength(0); + }); }); describe("out-of-order arrival (child before parent)", () => { @@ -232,7 +252,6 @@ describe("useIncrementalTreeData", () => { { initialProps: { items: [makeItem("a"), makeItem("b")] } } ); - rerender({ items: [makeItem("a"), makeItem("b")] }); expect(result.current.every(n => n.treeNodeState === TreeNodeState.EXPANDED)).toBe(true); rerender({ items: [makeItem("b"), makeItem("a")] }); @@ -256,7 +275,6 @@ describe("useIncrementalTreeData", () => { { initialProps: { items: [makeItem("a"), makeItem("b")] } as { items: ObjectItem[] | undefined } } ); - rerender({ items: [makeItem("a"), makeItem("b")] }); expand(result.current[0]); // datasource reloading: items is undefined until the new data arrives @@ -279,7 +297,6 @@ describe("useIncrementalTreeData", () => { { initialProps: { items, config: makeConfig() } } ); - rerender({ items: [...items], config: makeConfig() }); expand(result.current[0]); // Mendix hands over new prop instances on every refresh, which forces a rebuild @@ -298,7 +315,6 @@ describe("useIncrementalTreeData", () => { { initialProps: { items: [makeItem("a"), makeItem("b"), makeItem("c")] } } ); - rerender({ items: [makeItem("a"), makeItem("b"), makeItem("c")] }); expand(result.current[0]); rerender({ items: [makeItem("a"), makeItem("c")] }); @@ -316,7 +332,6 @@ describe("useIncrementalTreeData", () => { { initialProps: { items: [makeItem("a")], config } } ); - rerender({ items: [makeItem("a")], config }); expect(result.current[0].treeNodeState).toBe(TreeNodeState.EXPANDED); result.current[0].treeNodeState = TreeNodeState.COLLAPSED_WITH_CSS; diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts index 4c29e30983..41d3afa099 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/__tests__/useInfiniteTreeNode.spec.ts @@ -1,9 +1,11 @@ import { act, renderHook } from "@testing-library/react"; import { ObjectItem } from "mendix"; import * as FilterBuilders from "mendix/filters/builders"; -import { dynamic, listReference } from "@mendix/widget-plugin-test-utils"; +import { listReference } from "@mendix/widget-plugin-test-utils"; import { TreeNodeContainerProps } from "../../../../../typings/TreeNodeProps"; +import { TreeNodeState } from "../../../common/TreeNodeState"; import { useInfiniteTreeNodes } from "../useInfiniteTreeNode"; +import { TreeNodeV2DataItem } from "../useIncrementalTreeData"; jest.mock("mendix/filters/builders", () => ({ association: jest.fn(() => "assocExpr"), @@ -32,6 +34,34 @@ function requestedParentIds(setFilter: unknown): Array { return read(calls[calls.length - 1][0]); } +interface NodeSpec { + id: string; + state?: TreeNodeState; + children?: NodeSpec[]; + /** + * Only for orphans: a node the datasource placed at root level because its parent was not + * delivered. It renders as a root but is not one. + */ + orphanOf?: string; +} + +/** Builds the shape `useIncrementalTreeData` would have produced for these items. */ +function makeTree(specs: NodeSpec[], parentId?: string): TreeNodeV2DataItem[] { + return specs.map(spec => ({ + children: makeTree(spec.children ?? [], spec.id), + id: spec.id, + item: makeItem(spec.id), + parentId: spec.orphanOf ?? parentId, + treeNodeState: spec.state ?? TreeNodeState.COLLAPSED_WITH_JS, + title: spec.id + })); +} + +/** Every item a delivery would carry for the given tree. */ +function deliveryFor(nodes: TreeNodeV2DataItem[]): ObjectItem[] { + return nodes.flatMap(node => [node.item, ...deliveryFor(node.children)]); +} + function makeProps(overrides: Partial = {}): TreeNodeContainerProps { const setFilter = makeSetFilter(); return { @@ -70,11 +100,31 @@ function makeProps(overrides: Partial = {}): TreeNodeCon } as unknown as TreeNodeContainerProps; } +/** Renders the hook over a tree and its matching delivery, and syncs once as a click would. */ +function renderWithTree( + specs: NodeSpec[], + overrides: Partial = {} +): { props: TreeNodeContainerProps; sync: () => void } { + const tree = makeTree(specs); + const base = makeProps(overrides); + const props = { + ...base, + datasource: { ...base.datasource, items: deliveryFor(tree) } as any + } as TreeNodeContainerProps; + + const { result } = renderHook(() => useInfiniteTreeNodes(props, tree)); + + return { + props, + sync: () => act(() => result.current.syncPreloadFilter()) + }; +} + describe("useInfiniteTreeNodes", () => { describe("initialization", () => { it("sets filter to root-only (parent = undefined) on first render when startExpanded is false", () => { const props = makeProps({ startExpanded: false }); - renderHook(() => useInfiniteTreeNodes(props)); + renderHook(() => useInfiniteTreeNodes(props, [])); expect(props.datasource.setFilter).toHaveBeenCalledTimes(1); // The filter call should use literal(undefined) for root-only query expect(FilterBuilders.literal).toHaveBeenCalledWith(undefined); @@ -82,100 +132,47 @@ describe("useInfiniteTreeNodes", () => { it("does not filter on first render when startExpanded is true", () => { const props = makeProps({ startExpanded: true }); - renderHook(() => useInfiniteTreeNodes(props)); + renderHook(() => useInfiniteTreeNodes(props, [])); expect(props.datasource.setFilter).not.toHaveBeenCalled(); }); - it("returns datasource items", () => { - const items = [makeItem("a"), makeItem("b")]; - const props = makeProps({ datasource: { ...makeProps().datasource, items } as any }); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); - expect(result.current.items).toBe(items); - }); - }); - - describe("appendItems — first expansion", () => { - it("adds the expanded parent to the filter", () => { - const parentItem = makeItem("parent"); - const childItem = makeItem("child"); - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); + it("does not filter while the datasource is still loading", () => { + const base = makeProps(); + const props = { + ...base, + datasource: { ...base.datasource, items: undefined } as any + } as TreeNodeContainerProps; - act(() => { - result.current.appendItems(parentItem, [childItem]); - }); - - // setFilter called at least twice: init + after expand - expect(props.datasource.setFilter).toHaveBeenCalledTimes(2); - }); - - it("pre-loads children of the expanded node", () => { - const parentItem = makeItem("parent"); - const child1 = makeItem("child1"); - const child2 = makeItem("child2"); - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); - - act(() => { - result.current.appendItems(parentItem, [child1, child2]); - }); - - // second setFilter call should include parent + children - expect(props.datasource.setFilter).toHaveBeenCalledTimes(2); - // or() called for multi-item filter - expect(FilterBuilders.or).toHaveBeenCalled(); - }); - }); + const { result, rerender } = renderHook(() => useInfiniteTreeNodes(props, [])); + act(() => result.current.syncPreloadFilter()); + rerender(); - describe("appendItems — expanding a pre-loaded child", () => { - it("moves pre-loaded child from loadedChildren to loadedParents when it gets expanded", () => { - const rootItem = makeItem("root"); - const childItem = makeItem("child"); - const grandchildItem = makeItem("grandchild"); - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); - - // Expand root — child is pre-loaded - act(() => { - result.current.appendItems(rootItem, [childItem]); - }); - - const callCountAfterFirstExpand = (props.datasource.setFilter as jest.Mock).mock.calls.length; - - // Now expand child (which was pre-loaded, not yet in loadedParents) - act(() => { - result.current.appendItems(childItem, [grandchildItem]); - }); - - // Another setFilter call should have been made - expect((props.datasource.setFilter as jest.Mock).mock.calls.length).toBeGreaterThan( - callCountAfterFirstExpand - ); + // only the initial root-only filter + expect(props.datasource.setFilter).toHaveBeenCalledTimes(1); }); }); - describe("appendItems — pre-loading one level ahead", () => { + describe("deriving the parent set — one level past what is rendered", () => { it("asks for the children of the expanded node and of its children", () => { - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); + const { props, sync } = renderWithTree([ + { id: "root", state: TreeNodeState.EXPANDED, children: [{ id: "child" }] } + ]); - act(() => { - result.current.appendItems(makeItem("root"), [makeItem("child")]); - }); + sync(); expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "root", "child"]); }); it("keeps pre-loading when a node that was itself a pre-loaded child gets expanded", () => { - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); + const { props, sync } = renderWithTree([ + { + id: "root", + state: TreeNodeState.EXPANDED, + children: [{ id: "child", state: TreeNodeState.EXPANDED, children: [{ id: "grandchild" }] }] + } + ]); - act(() => { - result.current.appendItems(makeItem("root"), [makeItem("child")]); - }); - act(() => { - result.current.appendItems(makeItem("child"), [makeItem("grandchild")]); - }); + sync(); // without the grandchild in the filter, "child"'s children can never report // whether they have children of their own @@ -183,146 +180,355 @@ describe("useInfiniteTreeNodes", () => { }); it("does not ask for the same parent twice", () => { - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); - - act(() => { - result.current.appendItems(makeItem("root"), [makeItem("child")]); - }); - act(() => { - result.current.appendItems(makeItem("child"), [makeItem("grandchild")]); - }); - act(() => { - result.current.appendItems(makeItem("child"), [makeItem("grandchild")]); - }); + const { props, sync } = renderWithTree([ + { + id: "root", + state: TreeNodeState.EXPANDED, + children: [{ id: "child", state: TreeNodeState.EXPANDED, children: [{ id: "grandchild" }] }] + } + ]); + + sync(); + sync(); + sync(); const requested = requestedParentIds(props.datasource.setFilter); expect(requested).toEqual([...new Set(requested)]); }); it("asks for the children of a node expanded while it has none yet", () => { - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); + const { props, sync } = renderWithTree([{ id: "leaf", state: TreeNodeState.EXPANDED }]); - act(() => { - result.current.appendItems(makeItem("leaf"), []); - }); + sync(); expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "leaf"]); }); + + it("does not look past a node that was never opened", () => { + // "second" is one level past the visible root, so it is requested; "third" sits under a + // node whose body was never rendered, so it is not. + const { props, sync } = renderWithTree([ + { id: "root", children: [{ id: "second", children: [{ id: "third" }] }] } + ]); + + sync(); + + expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "root", "second"]); + }); + + it("issues no further request when the same set is derived again", () => { + const { props, sync } = renderWithTree([ + { id: "root", state: TreeNodeState.EXPANDED, children: [{ id: "child" }] } + ]); + + sync(); + const callCount = (props.datasource.setFilter as jest.Mock).mock.calls.length; + sync(); + sync(); + + expect(props.datasource.setFilter).toHaveBeenCalledTimes(callCount); + }); }); - describe("children arriving after the expansion", () => { - function makePropsWithParents( - parentMap: Record, - items: ObjectItem[] - ): TreeNodeContainerProps { - const props = makeProps({ - parentAssociation: listReference(b => - b - .withId("assoc_1") - .withGet((item: ObjectItem) => { - const parentId = parentMap[String(item.id)]; - return parentId ? dynamic.available(makeItem(parentId)) : dynamic.unavailable(); - }) - .build() - ) - }); - return { ...props, datasource: { ...props.datasource, items } as any } as TreeNodeContainerProps; - } + describe("what the derived set deliberately excludes", () => { + it("does not treat an item whose parent was not delivered as a root", () => { + // "orphan" renders at root level because its parent is missing from the delivery, but + // it is not a root, so its children are not requested — see design.md D6. + const { props, sync } = renderWithTree([ + { id: "root", children: [{ id: "child" }] }, + { id: "orphan", orphanOf: "missing-parent", children: [{ id: "orphanChild" }] } + ]); - it("pre-loads children that were not known when the node was expanded", () => { - // node expanded while its children are still in flight, so appendItems gets none - let props = makePropsWithParents({ parent: undefined }, []); - const setFilter = props.datasource.setFilter; + sync(); - const { result, rerender } = renderHook(({ p }: { p: TreeNodeContainerProps }) => useInfiniteTreeNodes(p), { - initialProps: { p: props } - }); + const requested = requestedParentIds(props.datasource.setFilter); + expect(requested).toEqual([undefined, "root", "child"]); + expect(requested).not.toContain("orphan"); + }); - act(() => { - result.current.appendItems(makeItem("parent")); - }); - expect(requestedParentIds(setFilter)).toEqual([undefined, "parent"]); + it("never puts a node the current delivery did not mention into the filter", () => { + // The tree keeps nodes a delivery did not carry (data-refresh behaviour); their object + // reference is from an older delivery and must not be used to request children. + const tree = makeTree([ + { id: "root", state: TreeNodeState.EXPANDED, children: [{ id: "child" }, { id: "retained" }] } + ]); + const base = makeProps(); + const props = { + ...base, + datasource: { ...base.datasource, items: [makeItem("root"), makeItem("child")] } as any + } as TreeNodeContainerProps; - // the children arrive in a later datasource update - props = makePropsWithParents({ parent: undefined, child: "parent" }, [ - makeItem("parent"), - makeItem("child") + const { result } = renderHook(() => useInfiniteTreeNodes(props, tree)); + act(() => result.current.syncPreloadFilter()); + + expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "root", "child"]); + }); + }); + + describe("collapsing", () => { + it("keeps asking for a collapsed node's subtree, because it is still rendered", () => { + // COLLAPSED_WITH_CSS means the node was opened and then closed: its body is still in the + // DOM, hidden. Dropping its descendants from the filter would remove them from the tree + // and leave them iconless when it is reopened. + const { props, sync } = renderWithTree([ + { + id: "root", + state: TreeNodeState.COLLAPSED_WITH_CSS, + children: [{ id: "child", state: TreeNodeState.EXPANDED, children: [{ id: "grandchild" }] }] + } ]); - (props.datasource as any).setFilter = setFilter; - rerender({ p: props }); - expect(requestedParentIds(setFilter)).toEqual([undefined, "parent", "child"]); + sync(); + + expect(requestedParentIds(props.datasource.setFilter)).toEqual([undefined, "root", "child", "grandchild"]); }); - it("pre-loads a child added to an already expanded node", () => { - let props = makePropsWithParents({ parent: undefined, child: "parent" }, [ - makeItem("parent"), - makeItem("child") + it("does not re-request anything when a collapsed node is expanded again", () => { + const { props, sync } = renderWithTree([ + { id: "root", state: TreeNodeState.COLLAPSED_WITH_CSS, children: [{ id: "child" }] } ]); - const setFilter = props.datasource.setFilter; - const { result, rerender } = renderHook(({ p }: { p: TreeNodeContainerProps }) => useInfiniteTreeNodes(p), { - initialProps: { p: props } - }); + sync(); + const callCount = (props.datasource.setFilter as jest.Mock).mock.calls.length; + + // re-expanding restores EXPANDED, which the derivation already treated as rendered + sync(); + + expect(props.datasource.setFilter).toHaveBeenCalledTimes(callCount); + }); + }); + + describe("children arriving after the expansion", () => { + it("pre-loads children that were not known when the node was expanded", () => { + // node expanded while its children are still in flight, so the tree has none yet + let tree = makeTree([{ id: "parent", state: TreeNodeState.EXPANDED }]); + const base = makeProps(); + const setFilter = base.datasource.setFilter; + const propsFor = (t: TreeNodeV2DataItem[]): TreeNodeContainerProps => + ({ + ...base, + datasource: { ...base.datasource, items: deliveryFor(t), setFilter } as any + }) as TreeNodeContainerProps; + + const { result, rerender } = renderHook( + ({ t }: { t: TreeNodeV2DataItem[] }) => useInfiniteTreeNodes(propsFor(t), t), + { initialProps: { t: tree } } + ); + + act(() => result.current.syncPreloadFilter()); + expect(requestedParentIds(setFilter)).toEqual([undefined, "parent"]); - act(() => { - result.current.appendItems(makeItem("parent"), [makeItem("child")]); - }); + // the children arrive in a later datasource update + tree = makeTree([{ id: "parent", state: TreeNodeState.EXPANDED, children: [{ id: "child" }] }]); + rerender({ t: tree }); + + expect(requestedParentIds(setFilter)).toEqual([undefined, "parent", "child"]); + }); + + it("pre-loads a child added to an already expanded node", () => { + let tree = makeTree([{ id: "parent", state: TreeNodeState.EXPANDED, children: [{ id: "child" }] }]); + const base = makeProps(); + const setFilter = base.datasource.setFilter; + const propsFor = (t: TreeNodeV2DataItem[]): TreeNodeContainerProps => + ({ + ...base, + datasource: { ...base.datasource, items: deliveryFor(t), setFilter } as any + }) as TreeNodeContainerProps; + + const { rerender } = renderHook( + ({ t }: { t: TreeNodeV2DataItem[] }) => useInfiniteTreeNodes(propsFor(t), t), + { initialProps: { t: tree } } + ); // a microflow adds a second child later - props = makePropsWithParents({ parent: undefined, child: "parent", added: "parent" }, [ - makeItem("parent"), - makeItem("child"), - makeItem("added") + tree = makeTree([ + { id: "parent", state: TreeNodeState.EXPANDED, children: [{ id: "child" }, { id: "added" }] } ]); - (props.datasource as any).setFilter = setFilter; - rerender({ p: props }); + rerender({ t: tree }); expect(requestedParentIds(setFilter)).toContain("added"); }); }); - describe("appendItems — re-expanding already-expanded node", () => { - it("does not add duplicate entries when same parent expanded twice", () => { - const parentItem = makeItem("parent"); - const childItem = makeItem("child"); - const props = makeProps(); - const { result } = renderHook(() => useInfiniteTreeNodes(props)); + describe("a replaced result set (WC-3564 follow-up)", () => { + it("asks for the new roots' children after an app-level constraint replaces the result set", () => { + // A gallery filtering the tree by department swaps the whole result set for a disjoint + // one. Before this was derived, the filter stayed frozen at the first set's parents and + // every new node was left with no expand affordance at all. + let tree = makeTree([{ id: "oldRoot", children: [{ id: "oldChild" }] }]); + const base = makeProps(); + const setFilter = base.datasource.setFilter; + const propsFor = (t: TreeNodeV2DataItem[]): TreeNodeContainerProps => + ({ + ...base, + datasource: { ...base.datasource, items: deliveryFor(t), setFilter } as any + }) as TreeNodeContainerProps; + + const { rerender } = renderHook( + ({ t }: { t: TreeNodeV2DataItem[] }) => useInfiniteTreeNodes(propsFor(t), t), + { initialProps: { t: tree } } + ); + rerender({ t: tree }); + expect(requestedParentIds(setFilter)).toEqual([undefined, "oldRoot", "oldChild"]); - act(() => { - result.current.appendItems(parentItem, [childItem]); - }); + // department switched: an entirely different set of roots, none seen before + tree = makeTree([{ id: "newRoot", children: [{ id: "newChild" }] }]); + rerender({ t: tree }); - const callCount = (props.datasource.setFilter as jest.Mock).mock.calls.length; + const requested = requestedParentIds(setFilter); + expect(requested).toEqual([undefined, "newRoot", "newChild"]); + expect(requested).not.toContain("oldRoot"); + }); - // expand same parent again with no children (collapsed → re-expanded, children already known) - act(() => { - result.current.appendItems(parentItem); - }); + it("treats a second and third replacement exactly like the first", () => { + const base = makeProps(); + const setFilter = base.datasource.setFilter; + const propsFor = (t: TreeNodeV2DataItem[]): TreeNodeContainerProps => + ({ + ...base, + datasource: { ...base.datasource, items: deliveryFor(t), setFilter } as any + }) as TreeNodeContainerProps; + + const { rerender } = renderHook( + ({ t }: { t: TreeNodeV2DataItem[] }) => useInfiniteTreeNodes(propsFor(t), t), + { initialProps: { t: makeTree([{ id: "setA", children: [{ id: "childA" }] }]) } } + ); - // setFilter still called (re-expansion triggers filter update) - expect((props.datasource.setFilter as jest.Mock).mock.calls.length).toBeGreaterThan(callCount); + for (const name of ["setB", "setC", "setD"]) { + const tree = makeTree([{ id: name, children: [{ id: `child-${name}` }] }]); + rerender({ t: tree }); + expect(requestedParentIds(setFilter)).toEqual([undefined, name, `child-${name}`]); + } }); }); describe("second render (pre-loading roots' children)", () => { it("pre-loads children of root nodes on second datasource render", () => { - const rootItems = [makeItem("root1"), makeItem("root2")]; - let items: ObjectItem[] = []; - const props = makeProps(); - - const { rerender } = renderHook(() => - useInfiniteTreeNodes({ ...props, datasource: { ...props.datasource, items } as any }) + let tree: TreeNodeV2DataItem[] = []; + const base = makeProps(); + const setFilter = base.datasource.setFilter; + const propsFor = (t: TreeNodeV2DataItem[]): TreeNodeContainerProps => + ({ + ...base, + datasource: { ...base.datasource, items: deliveryFor(t), setFilter } as any + }) as TreeNodeContainerProps; + + const { rerender } = renderHook( + ({ t }: { t: TreeNodeV2DataItem[] }) => useInfiniteTreeNodes(propsFor(t), t), + { initialProps: { t: tree } } ); // Simulate datasource delivering root items - items = rootItems; - rerender(); + tree = makeTree([{ id: "root1" }, { id: "root2" }]); + rerender({ t: tree }); // setFilter should have been called again to load children of roots - expect(props.datasource.setFilter).toHaveBeenCalledTimes(2); + expect(setFilter).toHaveBeenCalledTimes(2); + }); + }); + + describe("cascading down the tree (WC-3564)", () => { + function makeCascadeHarness(startExpanded: boolean): { + setFilter: jest.Mock; + deliver: (specs: NodeSpec[]) => void; + } { + const base = makeProps({ startExpanded }); + const setFilter = base.datasource.setFilter as jest.Mock; + let tree: TreeNodeV2DataItem[] = []; + const propsFor = (t: TreeNodeV2DataItem[]): TreeNodeContainerProps => + ({ + ...base, + datasource: { ...base.datasource, items: deliveryFor(t), setFilter } as any + }) as TreeNodeContainerProps; + + const { rerender } = renderHook( + ({ t }: { t: TreeNodeV2DataItem[] }) => useInfiniteTreeNodes(propsFor(t), t), + { initialProps: { t: tree } } + ); + + return { + setFilter, + deliver: specs => { + tree = makeTree(specs); + rerender({ t: tree }); + } + }; + } + + const expanded = TreeNodeState.EXPANDED; + + it("keeps cascading level by level under startExpanded, and stops once nothing new arrives — never locking in on a transient empty delivery", () => { + const { setFilter, deliver } = makeCascadeHarness(true); + expect(setFilter).toHaveBeenCalledTimes(0); // startExpanded skips the initial root-only filter + + // Datasource stays empty across a few transient renders (still loading) — must not + // call setFilter on empty data (this is exactly what broke live testing with a naive + // fire-count-based cap instead of a content-based one). + deliver([]); + deliver([]); + expect(setFilter).toHaveBeenCalledTimes(0); + + // Real root items arrive — cascades to fetch their children. + deliver([ + { id: "root1", state: expanded }, + { id: "root2", state: expanded } + ]); + expect(setFilter).toHaveBeenCalledTimes(1); + + // An unchanged redelivery of the same roots must not trigger another call. + deliver([ + { id: "root1", state: expanded }, + { id: "root2", state: expanded } + ]); + expect(setFilter).toHaveBeenCalledTimes(1); + + // Second tier arrives — cascades one level further automatically (no click involved). + deliver([ + { id: "root1", state: expanded, children: [{ id: "second1", state: expanded }] }, + { id: "root2", state: expanded } + ]); + expect(setFilter).toHaveBeenCalledTimes(2); + + // Third tier arrives — keeps cascading (this is the exact scenario that was broken: + // every level defaults to EXPANDED under startExpanded=true, so every level needs its + // own affordance pre-checked, not just roots + one bonus level). + deliver([ + { + id: "root1", + state: expanded, + children: [{ id: "second1", state: expanded, children: [{ id: "third1", state: expanded }] }] + }, + { id: "root2", state: expanded } + ]); + expect(setFilter).toHaveBeenCalledTimes(3); + + // Nothing new this time (same set redelivered) — stops here, no further call. + deliver([ + { + id: "root1", + state: expanded, + children: [{ id: "second1", state: expanded, children: [{ id: "third1", state: expanded }] }] + }, + { id: "root2", state: expanded } + ]); + expect(setFilter).toHaveBeenCalledTimes(3); + }); + + it("stops one level past the roots when startExpanded is false — deeper tiers resolve via a real click", () => { + const { setFilter, deliver } = makeCascadeHarness(false); + expect(setFilter).toHaveBeenCalledTimes(1); // initial root-only filter + + deliver([{ id: "root1" }]); + expect(setFilter).toHaveBeenCalledTimes(2); + + // one level past the visible roots + deliver([{ id: "root1", children: [{ id: "second1" }] }]); + expect(setFilter).toHaveBeenCalledTimes(3); + + // Third tier arriving must NOT trigger a further automatic round — unlike + // startExpanded=true, deeper tiers here only resolve via a real click. + deliver([{ id: "root1", children: [{ id: "second1", children: [{ id: "third1" }] }] }]); + expect(setFilter).toHaveBeenCalledTimes(3); }); }); }); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/helpers.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/helpers.ts index 39d4702a2e..4c083a3775 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/helpers.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/helpers.ts @@ -1,6 +1,7 @@ import { ObjectItem } from "mendix"; import { ReactNode, KeyboardEvent } from "react"; -import { TreeConfigRef } from "./useIncrementalTreeData"; +import { TreeConfigRef, TreeNodeV2DataItem } from "./useIncrementalTreeData"; +import { TreeNodeState } from "../../common/TreeNodeState"; import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps"; export function getItemId(item: ObjectItem): string { @@ -35,6 +36,60 @@ export function isConfigChanged(previous: TreeConfigRef | null, next: TreeConfig ); } +/** + * The parent ids whose children the datasource has to deliver for the tree to be renderable and + * correct: every node the widget currently renders, plus one level past it so a node's own expand + * affordance is already known before the expand that reveals it lands. + * + * Derived from the tree as it stands — deliberately not from a record of what has been fetched + * before. A node only starts a chain here when it has no parent at all; an orphan (its parent + * exists but the datasource does not deliver it) is promoted to root level for rendering, but is + * not a root for retrieval. See `design.md` D6. + * + * Recursion continues through `COLLAPSED_WITH_CSS` as well as `EXPANDED`: that state means the + * node was opened and then closed, so its subtree is still rendered (hidden by CSS) and still has + * to be kept correct. `COLLAPSED_WITH_JS` stops it — that body was never rendered. + */ +export function deriveDesiredParentIds(treeData: TreeNodeV2DataItem[], deliveredIds: Set): string[] { + const desiredIds: string[] = []; + const seen = new Set(); + + const add = (node: TreeNodeV2DataItem): void => { + if (seen.has(node.id) || !deliveredIds.has(node.id)) { + return; + } + seen.add(node.id); + desiredIds.push(node.id); + }; + + const isSubtreeRendered = (node: TreeNodeV2DataItem): boolean => + node.treeNodeState === TreeNodeState.EXPANDED || node.treeNodeState === TreeNodeState.COLLAPSED_WITH_CSS; + + // Breadth-first over the rendered part of the tree. Order is incidental — the caller turns this + // into an `or` of equalities, which is a set. + let level = treeData.filter(node => node.parentId === undefined); + + while (level.length > 0) { + const nextLevel: TreeNodeV2DataItem[] = []; + + for (const node of level) { + add(node); + + for (const child of node.children) { + add(child); + + if (isSubtreeRendered(node)) { + nextLevel.push(child); + } + } + } + + level = nextLevel; + } + + return desiredIds; +} + export function onKeyDownHandler( event: KeyboardEvent, hasChildren: boolean, diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts index cebebdb6e7..a5c46120a5 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useIncrementalTreeData.ts @@ -23,19 +23,6 @@ export interface TreeNodeV2DataItem { title: ReactNode; } -function resolveRestoredState(remembered: TreeNodeState | undefined, startExpanded: boolean): TreeNodeState { - if (remembered === undefined) { - return TreeNodeState.LOADING; - } - - // A remembered LOADING means the node was already seen in an earlier batch, so it resolves now. - if (remembered === TreeNodeState.LOADING) { - return startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS; - } - - return remembered; -} - export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: TreeConfigRef): TreeNodeV2DataItem[] { const [treeData, setTreeData] = useState([]); @@ -44,18 +31,23 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: const placementByIdRef = useRef>(new Map()); const previousIdsRef = useRef>(new Set()); const previousConfigRef = useRef(null); - // Expansion state per item id, kept across rebuilds so a data refresh does not collapse the tree. + // Expanded/collapsed state per item id, kept across rebuilds so a data refresh does not + // collapse the tree. A rebuild is unavoidable for a config-reference change or a removed + // item, but it does not have to be destructive. const statesByIdRef = useRef>(new Map()); useEffect(() => { if (items === undefined) { - // Datasource is (re)loading. Keep the current tree instead of treating it as an empty list. + // Datasource is (re)loading. Keep the tree we already built instead of reading + // undefined as an empty list, which would look like every item was removed. return; } const sourceItems = items; const incomingIds = new Set(sourceItems.map(getItemId)); - // The datasource order (e.g. a sort on a sequence attribute) is the source of truth for sibling order. + // The datasource order (e.g. a sort on a sequence attribute) is the source of truth for + // root and sibling order, and has to be re-applied on every update, not only when an id + // is first seen. const orderById = new Map(sourceItems.map((item, index) => [getItemId(item), index])); const removedIdsDetected = @@ -136,12 +128,6 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: placeNode(existingNode); } - if (existingNode.treeNodeState === TreeNodeState.LOADING) { - existingNode.treeNodeState = config.startExpanded - ? TreeNodeState.EXPANDED - : TreeNodeState.COLLAPSED_WITH_JS; - nodesByIdRef.current.set(nodeId, existingNode); - } continue; } @@ -150,7 +136,11 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: id: nodeId, item, parentId: nextParentId, - treeNodeState: resolveRestoredState(statesByIdRef.current.get(nodeId), config.startExpanded), + // A remembered state wins over the configured default in both directions: a node + // the user collapsed under "Start expanded" = Yes must come back collapsed. + treeNodeState: + statesByIdRef.current.get(nodeId) ?? + (config.startExpanded ? TreeNodeState.EXPANDED : TreeNodeState.COLLAPSED_WITH_JS), title: nextTitle }; nodesByIdRef.current.set(nodeId, newNode); @@ -164,8 +154,10 @@ export function useIncrementalTreeData(items: ObjectItem[] | undefined, config: } } - // Re-apply the datasource order, existing nodes keep their insertion order otherwise. - // Nodes missing from the current batch keep their relative order at the end of the list. + // Re-apply the datasource order once all placement is done — placement itself is + // order-independent by design (a child can arrive before its parent), so this is the only + // point where the full delivery order is known. A node the current delivery does not + // mention sorts after every node it does, keeping the unmentioned nodes' relative order. const orderOf = (node: TreeNodeV2DataItem): number => orderById.get(node.id) ?? sourceItems.length; const compareByDatasourceOrder = (a: TreeNodeV2DataItem, b: TreeNodeV2DataItem): number => orderOf(a) - orderOf(b); diff --git a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts index 88e9f6fd3f..385c38e566 100644 --- a/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts +++ b/packages/pluggableWidgets/tree-node-web/src/components/v2/hooks/useInfiniteTreeNode.ts @@ -1,23 +1,28 @@ import { ObjectItem, Option } from "mendix"; import { association, equals, literal, or } from "mendix/filters/builders"; import { useCallback, useEffect, useRef } from "react"; -import { getItemId, getParentId } from "./helpers"; +import { deriveDesiredParentIds, getItemId } from "./helpers"; +import { TreeNodeV2DataItem } from "./useIncrementalTreeData"; import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps"; export type ItemType = Array>; -export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { - items: ObjectItem[] | undefined; - appendItems: (newItem: ObjectItem, children?: ObjectItem[]) => void; -} { +/** + * Keeps the datasource filter in sync with what the tree currently needs: the children of every + * rendered node, plus one level past it. The set is *derived* from `treeData` on every delivery — + * this hook keeps no record of what it has already fetched, which is what let a replaced result + * set (a gallery filtering the tree by department) leave every node without an expand affordance. + * See `design.md` D6. + */ +export function useInfiniteTreeNodes( + props: TreeNodeContainerProps, + treeData: TreeNodeV2DataItem[] +): { syncPreloadFilter: () => void } { const { datasource, parentAssociation, startExpanded } = props; - // loadedParents : track the nodes that are expanded - const loadedParentsByIdRef = useRef>(new Map()); - // loadedChilds : track the pre-loaded nodes of expanded nodes. - const loadedChildsByIdRef = useRef>(new Map()); - // expandedIds : nodes the user opened, their children need to be pre-loaded as they arrive. - const expandedIdsRef = useRef>(new Set()); const initializedRef = useRef(false); + // The only state this hook holds: the parent ids the last setFilter call asked for, as a + // comparison key, so re-deriving the same set is a no-op instead of another retrieve. + const lastAppliedKeyRef = useRef(null); const getDatasourceFilter = useCallback( (items?: ItemType) => { @@ -31,83 +36,58 @@ export function useInfiniteTreeNodes(props: TreeNodeContainerProps): { [parentAssociation] ); - const getExpandedFilterItems = useCallback( - (): ItemType => [undefined, ...loadedParentsByIdRef.current.values(), ...loadedChildsByIdRef.current.values()], - [] - ); - - const appendItems = useCallback( - (newItem: ObjectItem, children?: ObjectItem[]) => { - const parentId = getItemId(newItem); - expandedIdsRef.current.add(parentId); + const applyFilter = useCallback( + (items: ItemType) => { + const key = items + .map(item => (item === undefined ? "" : getItemId(item))) + .sort() + .join("\u0000"); - // The expanded node is a loaded parent now, it is no longer a pre-loaded child. - loadedParentsByIdRef.current.set(parentId, newItem); - loadedChildsByIdRef.current.delete(parentId); - - children?.forEach(child => { - const childId = getItemId(child); - // pre-load the children of the expanded node, - // this is needed to be able to know if a node has further level children before expanding it. - if (!loadedParentsByIdRef.current.has(childId)) { - loadedChildsByIdRef.current.set(childId, child); - } - }); + if (key === lastAppliedKeyRef.current) { + return; + } - datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); + lastAppliedKeyRef.current = key; + datasource.setFilter(getDatasourceFilter(items)); }, - [datasource, getDatasourceFilter, getExpandedFilterItems] + [datasource, getDatasourceFilter] ); - useEffect(() => { - if (initializedRef.current) { - // after the first load of the datasource, - // we want to pre-load the child nodes of roots - if (loadedParentsByIdRef.current.size === 0) { - datasource.items?.forEach(item => { - const parentId = getItemId(item); - loadedParentsByIdRef.current.set(parentId, item); - }); - datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); - return; - } + const syncPreloadFilter = useCallback(() => { + const items = datasource.items; - // children of an expanded node can arrive after the expansion, or be added later on. - // pre-load them here as well, so every visible node knows whether it has children. - let hasNewChilds = false; - datasource.items?.forEach(item => { - const itemId = getItemId(item); + // Nothing to derive from: the datasource is still loading, or delivered nothing at all. + // `undefined` (the root level) is always part of the filter, so a delivery can only be + // empty when there is genuinely nothing to show. + if (items === undefined || items.length === 0) { + return; + } - if (loadedParentsByIdRef.current.has(itemId) || loadedChildsByIdRef.current.has(itemId)) { - return; - } + // Only the current delivery's objects may go into the filter. The tree deliberately keeps + // nodes a delivery did not mention, and their `item` reference would be a stale one. + const deliveredById = new Map(items.map(item => [getItemId(item), item])); + const desiredIds = deriveDesiredParentIds(treeData, new Set(deliveredById.keys())); - const parentId = getParentId(item, parentAssociation); - if (parentId && expandedIdsRef.current.has(parentId)) { - loadedChildsByIdRef.current.set(itemId, item); - hasNewChilds = true; - } - }); - - if (hasNewChilds) { - datasource.setFilter(getDatasourceFilter(getExpandedFilterItems())); - } + applyFilter([undefined, ...desiredIds.map(id => deliveredById.get(id)!)]); + }, [datasource, treeData, applyFilter]); + useEffect(() => { + if (initializedRef.current) { + syncPreloadFilter(); return; } initializedRef.current = true; - loadedParentsByIdRef.current.clear(); - // when datasource is loaded for the first time, we want to load only the root nodes (nodes without parent) - // if startExpanded is false, otherwise we want to load all nodes + // On the very first pass there is no tree to derive from yet. When only roots are expanded, + // ask for the root level and let the derivation take over from the first real delivery. + // When "Start expanded" is Yes, apply no filter at all: a non-microflow datasource then + // returns the whole table in one retrieve, and the set derived from it is already stable — + // starting from the root level instead would cost one retrieve per level of depth. if (!startExpanded) { - datasource.setFilter(getDatasourceFilter([undefined])); + applyFilter([undefined]); } - }, [datasource, getDatasourceFilter, getExpandedFilterItems, parentAssociation, startExpanded]); + }, [syncPreloadFilter, applyFilter, startExpanded]); - return { - items: datasource.items, - appendItems - }; + return { syncPreloadFilter }; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9a1c6cf095..6e0aa96d4f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2689,7 +2689,7 @@ importers: version: link:../../shared/eslint-config-web-widgets '@mendix/pluggable-widgets-tools': specifier: 11.13.0 - version: 11.13.0(patch_hash=1879adf9f5f058d67d2e08e79916d5adce82416899758529f812f4207c064fed)(@jest/transform@30.3.0)(@jest/types@30.4.1)(@types/babel__core@7.20.5)(@types/node@24.12.4)(canvas@3.2.3)(eslint@9.39.5(jiti@2.6.1))(jest-util@30.4.1)(prettier@3.9.6)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(tslib@2.8.1) + version: 11.13.0(patch_hash=1879adf9f5f058d67d2e08e79916d5adce82416899758529f812f4207c064fed)(@jest/transform@30.3.0)(@jest/types@30.4.1)(@types/babel__core@7.20.5)(@types/node@24.12.4)(canvas@3.2.3)(eslint@9.39.5(jiti@2.6.1))(jest-util@30.4.1)(picomatch@4.0.5)(prettier@3.9.6)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(tslib@2.8.1) '@mendix/prettier-config-web-widgets': specifier: workspace:* version: link:../../shared/prettier-config-web-widgets