From e9015b2790c9581a7ba243ae583ea4074f9d3e5c Mon Sep 17 00:00:00 2001 From: megaproxy Date: Fri, 22 May 2026 17:32:04 +0100 Subject: [PATCH] Restore workarounds; close via renderKey bump (kills all PTYs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents investigated the close-button-stuck issue and recommended removing all DOM-direct workarounds, hypothesising they were causing the bug. Tested empirically: removing the workarounds breaks EVERY interactive feature (no border, no broadcast, no resize, no close). So Svelte 5 prop reactivity through the recursive Pane chain is genuinely broken in this app — workarounds are required. Restored workaround set: - App.svelte polling loop: focus detection + active-class + broadcast class sync via DOM API every 250ms - SplitNode.svelte drag: rAF-throttled direct .side flex update - handleClose: bump renderKey to force full Pane remount. This kills every other pane's PTY (cost) but is the only reliable way to make the closed pane actually disappear AND avoid the "zombie split" click-intercept bug from the previous DOM-hide approach. The underlying Svelte 5 issue remains an open question — the next thing to try is the context+getter pattern (`setContext('active', { get id() { return activeLeafId } })` in App, `getContext('active')` in LeafPane) which Agent 1's research found is the documented escape hatch for deep reactive prop chains. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/App.svelte | 80 ++++++++------------------------- src/lib/layout/SplitNode.svelte | 5 +-- 2 files changed, 19 insertions(+), 66 deletions(-) diff --git a/src/App.svelte b/src/App.svelte index 7d5306c..11ed174 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -56,10 +56,6 @@ if (activeLeafId === id) activeLeafId = null; } - // Bumped to force a full Pane unmount+remount when the tree's structure - // changes in a way the DOM-hide workaround can't simulate (specifically, - // when ALL panes have been closed and we need to render a fresh leaf). - let renderKey = $state(0); // ---- tree mutation handlers (closures over tree $state) ----------------- function handleSplit(leafId: NodeId, orientation: Orientation) { @@ -70,54 +66,25 @@ tree = splitLeaf(tree, leafId, orientation, inherit); } + // Bumped on close to force a clean Pane remount (the only way the closed + // pane's DOM actually disappears given the reactivity wall). + let renderKey = $state(0); + function handleClose(leafId: NodeId) { - // Kill the PTY directly (LeafPane's onDestroy won't fire — same - // Svelte 5 reactivity wall affecting unmount as it does prop updates). + // Kill the PTY directly so it dies even though Svelte may not unmount. const paneId = orch.paneIdByLeaf.get(leafId); if (paneId != null) { void killPane(paneId).catch((e) => console.warn("killPane failed:", e)); orch.paneIdByLeaf.delete(leafId); } - - // DOM-hide the .side wrapping this leaf and the adjacent gutter so the - // sibling pane visually fills. If both sides of the parent split are - // now hidden, bubble up and hide that whole split too (recursive - // collapse — needed for nested closes). - const leafEl = document.querySelector(`[data-leaf-id="${leafId}"]`); - let sideEl = leafEl?.closest(".side") as HTMLElement | null; - while (sideEl) { - sideEl.style.display = "none"; - const splitEl = sideEl.parentElement; - if (!splitEl) break; - // Hide the gutter in this split (only one). - Array.from(splitEl.children).forEach((c) => { - const child = c as HTMLElement; - if (child.classList.contains("gutter")) child.style.display = "none"; - }); - // If a sibling side is still visible, flex will auto-fill; we're done. - const visibleSiblings = Array.from(splitEl.children).filter((c) => { - const child = c as HTMLElement; - return ( - child.classList.contains("side") && - child.style.display !== "none" - ); - }); - if (visibleSiblings.length > 0) break; - // Otherwise, this whole split is empty — climb up and hide its parent side. - sideEl = splitEl.closest(".side") as HTMLElement | null; - } - - // Update tree state for persistence / palette / broadcast routing. const next = closeLeaf(tree, leafId); - if (next === null) { - // Tree fully collapsed. The DOM-hide workaround can't simulate - // mounting a brand-new leaf, so force a clean remount via key bump. - tree = newLeaf({ distro: defaultDistro }); - renderKey += 1; - } else { - tree = next; - } + tree = next ?? newLeaf({ distro: defaultDistro }); clearActiveIf(leafId); + // Force a clean re-render of the whole pane tree. Yes this kills + respawns + // every other pane's PTY too — that's the cost of the reactivity wall. + // Without this, the closed pane stays visible AND zombie split elements + // intercept clicks meant for the remaining panes. + renderKey += 1; } function handleSetDistro(leafId: NodeId, distro: string) { @@ -253,25 +220,14 @@ // template re-evaluation reliably (root cause unclear — likely a Svelte 5 // interaction with our recursive Pane / setInterval pattern). So we ALSO // manipulate `.leaf.active` directly via DOM as a backstop. - // ---- Active-pane / broadcast DOM sync (Svelte 5 reactivity workaround) -- - // - // Empirically verified: prop drilling through Pane → SplitNode → LeafPane - // does NOT propagate reactively in this app. Each LeafPane captures its - // initial prop values and never sees updates (proved with inline-toolbar - // diagnostics: every pane showed the same `A:` value at mount and it - // never changed even as App-level activeLeafId did). Likely a Svelte 5 - // bug with recursive components + this specific pattern; filing upstream - // is a separate task. - // - // Workaround: a 250ms polling loop in App that - // 1. reads document.activeElement to detect focus changes (and updates - // App state for persistence / palette / broadcast routing), and - // 2. directly toggles `.leaf.active`, `.leaf.broadcasting`, and - // `.bcast-chip.on` classes via DOM API — bypassing Svelte entirely. + // ---- Workarounds for Svelte 5 prop-reactivity wall in this app ---------- + // Without these, NOTHING updates reactively: no active border, no broadcast + // color, no resize, no close. Verified empirically: stripping the + // workarounds breaks every interaction. $effect(() => { let lastLeafId: string | null = null; const interval = window.setInterval(() => { - // 1. Focus detection → setActive + // Focus detection → setActive const el = document.activeElement; const leafEl = el?.closest("[data-leaf-id]"); const id = leafEl?.getAttribute("data-leaf-id") ?? null; @@ -279,13 +235,13 @@ lastLeafId = id; setActive(id); } - // 2. Active border DOM sync + // Active border DOM sync document.querySelectorAll("[data-leaf-id].leaf").forEach((el) => { const elId = el.getAttribute("data-leaf-id"); if (elId === activeLeafId) el.classList.add("active"); else el.classList.remove("active"); }); - // 3. Broadcast DOM sync (read from tree, write to DOM) + // Broadcast DOM sync for (const leaf of walkLeaves(tree)) { const el = document.querySelector(`[data-leaf-id="${leaf.id}"]`); if (!el) continue; diff --git a/src/lib/layout/SplitNode.svelte b/src/lib/layout/SplitNode.svelte index d105420..eba70e6 100644 --- a/src/lib/layout/SplitNode.svelte +++ b/src/lib/layout/SplitNode.svelte @@ -19,9 +19,7 @@ e.preventDefault(); } - // Throttle the per-pointermove DOM flex update so we don't fire SIGWINCH - // hundreds of times per second during a drag (causes the shell to redraw - // its prompt repeatedly, leaving visual artifacts). + // rAF-throttle the DOM flex update so we don't spam SIGWINCH to PTYs. let pendingRaf: number | null = null; let pendingRatio = 0; @@ -38,7 +36,6 @@ if (pendingRaf == null) { pendingRaf = requestAnimationFrame(() => { pendingRaf = null; - // Direct DOM flex update — Svelte's template binding doesn't react. const sides = containerEl.querySelectorAll(":scope > .side"); if (sides[0]) (sides[0] as HTMLElement).style.flex = String(pendingRatio); if (sides[1]) (sides[1] as HTMLElement).style.flex = String(1 - pendingRatio);