🐛 Fix collapsing token set groups after reordering (#12671)

* 🐛 Fix collapsing token set groups after reordering

The handler captured the first render path, and the tree is keyed by
index, so after a reorder it toggled the path of another group.

Closes #12625

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Refresh double click handler of token set groups

Add the start edition callback to the hook deps so the handler does
not keep the initial closure.

AI-assisted-by: deepseek-v4.1-flash

* ✨ Add regression test for folding reordered token set groups

Reorder a group in the sets tree, fold it, and assert that only that
group collapses. The tree is keyed by index, so the test also proves
the collapse handler does not keep a stale path after a reorder.

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Refresh double click handler of token set nodes

Add the start edition callback to the hook deps so the handler does not
keep the initial closure.

AI-assisted-by: deepseek-v4.1-flash

* ♻️ Key token set tree nodes by identity instead of index

React reuses a node by its key. Keyed by the tree index, a reordered
group moved the instance to another node and its handlers kept the
previous paths. Groups are now keyed by path and sets by id, so each
node keeps its own handlers.

The set being created has no persisted id yet, so the tree walk
generates a new one on every render. Keep one uuid for its whole
creation instead.

AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
Luis de Dios 2026-11-12 13:49:43 +01:00 committed by GitHub
parent ef725c0cc2
commit 185fef889b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 71 additions and 6 deletions

View File

@ -186,6 +186,61 @@ test.describe("Tokens: Sets Tab", () => {
await expect(darkSet).toHaveCount(0);
});
test("Folding a group after reordering collapses only that group", async ({
page,
}) => {
const { tokenThemesSetsSidebar, tokenSetGroupItems } =
await setupTokensFileRender(page);
// The reorder swaps two groups, so every tree index changes while the DOM
// nodes keep their positions.
await createSet(tokenThemesSetsSidebar, "Colors/primary");
const lightDarkGroup = tokenSetGroupItems.filter({ hasText: "LightDark" });
const colorsGroup = tokenSetGroupItems.filter({ hasText: "Colors" });
await expect(tokenSetGroupItems).toHaveCount(2);
await expect(tokenSetGroupItems.first()).toContainText("LightDark");
// Drag the Colors group above the LightDark group. The sets list is a small
// scroll container, so both groups cannot be under the pointer at once and
// the HTML5 drag events are dispatched directly.
const target = await lightDarkGroup.boundingBox();
const dataTransfer = await page.evaluateHandle(() => new DataTransfer());
await colorsGroup.dispatchEvent("dragstart", { dataTransfer });
await lightDarkGroup.dispatchEvent("dragenter", { dataTransfer });
await lightDarkGroup.dispatchEvent("dragover", {
dataTransfer,
clientX: target.x + 100,
clientY: target.y + 2,
});
await lightDarkGroup.dispatchEvent("drop", {
dataTransfer,
clientX: target.x + 100,
clientY: target.y + 2,
});
await colorsGroup.dispatchEvent("dragend", { dataTransfer });
await expect(tokenSetGroupItems.first()).toContainText("Colors");
// Folding the first group hides only its own children.
await tokenSetGroupItems
.first()
.getByTestId("tokens-set-group-collapse")
.click();
await expect(
tokenThemesSetsSidebar.getByRole("button", {
name: "primary",
exact: true,
}),
).toHaveCount(0);
await expect(
tokenThemesSetsSidebar.getByRole("button", { name: "dark", exact: true }),
).toBeVisible();
});
test("Change current theme", async ({ page }) => {
const { tokenThemesSetsSidebar, tokenSetItems } =
await setupTokensFileRender(page);

View File

@ -10,6 +10,7 @@
(:require
[app.common.data.macros :as dm]
[app.common.types.tokens-lib :as ctob]
[app.common.uuid :as uuid]
[app.main.data.workspace.tokens.library-edit :as dwtl]
[app.main.store :as st]
[app.main.ui.context :as ctx]
@ -134,13 +135,16 @@
on-collapse-click
(mf/use-fn
(mf/deps path on-toggle-collapse)
(fn [event]
(dom/prevent-default event)
(dom/stop-propagation event)
(on-toggle-collapse path)))
on-double-click
(mf/use-fn (mf/deps id) #(on-start-edition id))
(mf/use-fn
(mf/deps id on-start-edition)
#(on-start-edition id))
on-checkbox-click
(mf/use-fn
@ -240,7 +244,7 @@
on-double-click
(mf/use-fn
(mf/deps id is-new)
(mf/deps id is-new on-start-edition)
(fn []
(when-not is-new
(on-start-edition id))))
@ -358,7 +362,13 @@
(fn [path]
(swap! collapsed-paths* #(if (contains? % path)
(disj % path)
(conj % path)))))]
(conj % path)))))
;; The set being created is not persisted yet, so its id is generated
;; on every walk of the tree. Keep one for the lifetime of this
;; creation so the node can be keyed by identity like the rest.
new-node-id
(mf/use-memo (mf/deps new-path) #(dm/str (uuid/next)))]
(mf/with-effect []
(let [sub (rx/subs! (fn [paths']
@ -378,7 +388,7 @@
(cond
^boolean is-group
[:> sets-tree-set-group*
{:key index
{:key id
:id id
:label (peek path)
:is-editing (= edition-id id)
@ -400,7 +410,7 @@
^boolean is-new
[:> sets-tree-set*
{:key index
{:key new-node-id
:id id
:set token-set
:label ""
@ -420,7 +430,7 @@
:else
[:> sets-tree-set*
{:key index
{:key id
:id id
:set token-set
:label (peek path)