fix: galleries keep the tiles that did not change, so toggling a reference no longer rebuilds the bin, reloads its thumbnails or resets its scroll

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-13 05:38:27 +03:00
parent d80b0aebef
commit cd9959cb19
3 changed files with 109 additions and 13 deletions
@@ -46,25 +46,41 @@ function thumbOf(item, cls = "cx-cell-thumb", marks = true) {
 function collection({ items = [], empty = EMPTY, selection = [], onActivate, onContext, drag } = {},
                    host, cellFor) {
  const chosen = new Set(selection);
+  let kept = new Map();               // id -> { cell, sig }: a tile is built once and kept while what it shows is unchanged
+
+  const mark = (cell, on) => { cell.classList.toggle("cx-on", on); cell.setAttribute("aria-selected", String(on)); };

  function draw() {
-    host.replaceChildren();
    if (!items.length) {
-      host.append(el("p", { cls: "cx-list-empty", text: empty }));
+      kept = new Map();
+      host.replaceChildren(el("p", { cls: "cx-list-empty", text: empty }));
      return;
    }
-    for (const item of items) {
-      const cell = cellFor(item, chosen.has(item.id));
-      cell.addEventListener("click", () => { if (onActivate) onActivate(item); });
-      if (onContext) {
-        cell.addEventListener("contextmenu", (e) => { e.preventDefault(); onContext(item, e); });
+    // Only a tile whose item changed is rebuilt. Rebuilding them all re-created every thumbnail and reset the
+    // scroll position, and a selection change alone is just a class and an attribute.
+    const next = new Map();
+    const cells = items.map((item) => {
+      const sig = JSON.stringify(item), old = kept.get(item.id);
+      let entry = old && old.sig === sig ? old : null;
+      if (!entry) {
+        const cell = cellFor(item, chosen.has(item.id));
+        cell.addEventListener("click", () => { if (onActivate) onActivate(item); });
+        if (onContext) {
+          cell.addEventListener("contextmenu", (e) => { e.preventDefault(); onContext(item, e); });
+        }
+        if (drag) {       // { type, data(item) }: a tile that can be dropped elsewhere
+          cell.draggable = true;
+          cell.addEventListener("dragstart", (e) => { e.dataTransfer.setData(drag.type, JSON.stringify(drag.data(item))); e.dataTransfer.effectAllowed = "copy"; });
+        }
+        entry = { cell, sig };
      }
-      if (drag) {       // { type, data(item) }: a tile that can be dropped elsewhere
-        cell.draggable = true;
-        cell.addEventListener("dragstart", (e) => { e.dataTransfer.setData(drag.type, JSON.stringify(drag.data(item))); e.dataTransfer.effectAllowed = "copy"; });
-      }
-      host.append(cell);
-    }
+      mark(entry.cell, chosen.has(item.id));
+      next.set(item.id, entry);
+      return entry.cell;
+    });
+    kept = next;
+    const now = [...host.children];
+    if (now.length !== cells.length || cells.some((cell, i) => cell !== now[i])) host.replaceChildren(...cells);
  }
  draw();

@@ -0,0 +1,60 @@
+// A gallery keeps the tiles it has: only a tile whose item changed is rebuilt, and a selection change rebuilds nothing.
+import test from "node:test";
+import assert from "node:assert/strict";
+import { setupDom, teardownDom } from "./_dom.js";
+
+let composer;
+test.before(async () => { setupDom(); ({ composer } = await import("../composer.js")); });
+test.after(() => teardownDom());
+
+const item = (id, extra = {}) => ({ id, label: id, thumb: `/t/${id}`, badge: "image", ...extra });
+const cells = (g) => [...g.node.querySelectorAll(".cx-cell")];
+
+for (const view of ["adaptive", "icons", "list"]) {
+  test(`${view}: setting the same items again keeps every tile, and a selection change rebuilds none`, () => {
+    const g = composer.gallery[view]({ items: [item("a"), item("b"), item("c")] });
+    document.body.append(g.node);
+    const first = [...g.node.querySelectorAll("button")];
+    g.setItems([item("a"), item("b"), item("c")]);
+    g.setValue(["b"]);
+    const again = [...g.node.querySelectorAll("button")];
+    assert.equal(again.length, 3);
+    again.forEach((n, i) => assert.equal(n, first[i], "the very same tile, so its picture is not fetched again"));
+    assert.deepEqual(again.map((n) => n.getAttribute("aria-selected")), ["false", "true", "false"]);
+    assert.ok(again[1].classList.contains("cx-on") && !again[0].classList.contains("cx-on"));
+    g.setValue([]);
+    assert.deepEqual([...g.node.querySelectorAll("button")].map((n) => n.getAttribute("aria-selected")), ["false", "false", "false"]);
+    g.node.remove();
+  });
+}
+
+test("a changed item rebuilds only its own tile, and a new order moves tiles instead of making new ones", () => {
+  const g = composer.gallery.adaptive({ items: [item("a"), item("b"), item("c")] });
+  document.body.append(g.node);
+  const [a, b, c] = cells(g);
+  g.setItems([item("a"), item("b", { flags: [{ text: "R2", title: "slot 2" }] }), item("c")]);
+  const [a2, b2, c2] = cells(g);
+  assert.equal(a2, a); assert.equal(c2, c);
+  assert.notEqual(b2, b, "the one whose flags changed");
+  assert.equal(b2.querySelector(".cx-cell-flag").textContent, "R2");
+  g.setItems([item("c"), item("a"), item("b", { flags: [{ text: "R2", title: "slot 2" }] })]);
+  assert.deepEqual(cells(g), [c, a, b2], "reordered, not rebuilt");
+  g.setItems([item("a")]);
+  assert.deepEqual(cells(g), [a], "a tile that is gone is gone");
+  g.setItems([]);
+  assert.equal(g.node.querySelectorAll(".cx-cell").length, 0);
+  assert.ok(g.node.querySelector(".cx-list-empty"));
+  g.node.remove();
+});
+
+test("a click still reaches the item after tiles were kept across redraws", () => {
+  const seen = [];
+  const g = composer.gallery.adaptive({ items: [item("a"), item("b")], onActivate: (it) => seen.push(it.id) });
+  document.body.append(g.node);
+  g.setItems([item("a"), item("b")]);
+  g.setValue(["a"]);
+  cells(g)[1].click();
+  cells(g)[0].click();
+  assert.deepEqual(seen, ["b", "a"]);
+  g.node.remove();
+});
@@ -86,3 +86,23 @@ test("a reference's tile menu pins it at the slot number it holds, for every sce
  [...document.querySelectorAll(".cx-menu-item")].find((b) => b.textContent === "Unpin reference from slot R2").click();
  assert.deepEqual(doc.fixed_references, {});
 });
+
+test("toggling a reference leaves the other tiles untouched, so the bin does not jump or reload its thumbnails", async () => {
+  const scene = { id: "s1", references: [] };
+  const project = { selected: scene, project: { fixed_references: {} }, setScene: () => {} };
+  const api = { media: async () => ({ media: [{ id: "m1", name: "cat.png", kind: "image" }, { id: "m2", name: "dog.png", kind: "image" }, { id: "m3", name: "owl.png", kind: "image" }] }) };
+  const host = document.createElement("div");
+  let fire;
+  hub.setup({ host, app: { project, api, mediaPeek: {}, say: () => {}, on: (cb) => { fire = cb; return () => {}; } } });
+  await new Promise((r) => setTimeout(r, 10));
+  const tiles = () => [...host.querySelectorAll(".cx-cell")];
+  const before = tiles();
+  scene.references = ["m2"];
+  fire("change");
+  const after = tiles();
+  assert.equal(after.length, 3);
+  assert.equal(after[0], before[0], "cat.png: same tile");
+  assert.equal(after[2], before[2], "owl.png: same tile");
+  assert.notEqual(after[1], before[1], "dog.png is the one that changed: it gained its R1 mark");
+  assert.equal(after[1].querySelector(".cx-cell-flag").textContent, "R1");
+});