Restore workarounds; close via renderKey bump (kills all PTYs)
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) <noreply@anthropic.com>
This commit is contained in:
parent
40ce27251d
commit
e9015b2790
2 changed files with 19 additions and 66 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue