From e6d004002106c2014ff34c72b21c4181d5a246ee Mon Sep 17 00:00:00 2001 From: megaproxy Date: Thu, 28 May 2026 20:24:09 +0100 Subject: [PATCH] Fix workspace accumulation, tab-close popover, scrollbars, drag ghost - window_state.rs: persist only the main window's workspaces. The aggregator flattened every window's tabs into the saved file; main then adopted the whole blob on launch, so detached windows' ephemeral tabs (and Pane N drag-out artifacts) accumulated without bound. - TabStrip: portal the close-confirm popover to with fixed, viewport-clamped positioning so the horizontally-scrolling strip can't clip it and it never runs off a window edge. - styles.css: make themed ::-webkit-scrollbar global, not just xterm viewport. - LeafPane: B1 drag-out ghost chip (portal, edge-pinned, orange detach state). - App.tsx: moveToNewWindow waits briefly for pane registration instead of failing instantly on an in-flight spawn/adopt. - gitignore cargo-test.lo*. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 2 +- memory.md | 13 ++++- src-tauri/src/window_state.rs | 33 ++++------- src/App.tsx | 15 ++++- src/components/TabStrip.css | 23 ++++---- src/components/TabStrip.tsx | 103 ++++++++++++++++++++++------------ src/lib/layout/LeafPane.css | 37 ++++++++++++ src/lib/layout/LeafPane.tsx | 76 +++++++++++++++++++++---- src/styles.css | 17 +++--- 9 files changed, 224 insertions(+), 95 deletions(-) diff --git a/.gitignore b/.gitignore index 81ba218..fa15af7 100644 --- a/.gitignore +++ b/.gitignore @@ -29,4 +29,4 @@ src-tauri/gen/ /shot*.png /tiletopia-window.png /tilescript.ps1 -/cargo-test.log +/cargo-test.lo* diff --git a/memory.md b/memory.md index 5761c25..94ec740 100644 --- a/memory.md +++ b/memory.md @@ -115,8 +115,17 @@ Smoke test on Windows revealed bugs specific to detached (non-main) windows. Mai - **B2–B5 (blank/dead detached windows) = the capability hypothesis, confirmed.** `src-tauri/capabilities/default.json` had `"windows": ["main"]`; detached labels are `pane-window-` (commands.rs:122) → matched nothing → every `invoke`/`listen` silently denied. Fix: `"windows": ["main", "pane-window-*"]`. Tauri 2 glob pattern works; one line cleared all four. (App-defined commands aren't individually permission-gated — they're available to any window the capability is *applied* to, i.e. listed in `windows`.) - **Session-loss-on-adopt (surfaced after B2–B5 cleared) = destructive read × StrictMode.** Once IPC worked, drag-out still spawned a FRESH pty (new id, tab named "Default", status `alive` not `adopted`) instead of adopting. Cause: `take_pending_window_init` is a **destructive** backend read (`by_label.remove`); React StrictMode runs the mount effect twice in dev — pass 1 consumed the payload then bailed on the `cancelled` flag, pass 2 got `null` → fell back to `singletonEnvelope` (fresh "Default" + fresh spawn). The `cancelled`-flag pattern guards against *using* stale async results but cannot un-consume a destructive backend call. Fix: module-level memoized `consumePendingWindowInit()` in App.tsx so the take fires **exactly once per window** and both StrictMode passes share the payload. Dev-only symptom (prod StrictMode doesn't double-invoke effects) but fixed for robustness. **Lesson: any destructive/once-only backend read called from a mount effect must be memoized at module scope, not just guarded by `cancelled`.** -- **Verified:** user confirmed adopt works (scrollback intact, same pane id, live input). `tsc -b` clean. B1 (drag ghost image) still deferred — cosmetic. -- Committed together with the carried-over `use tauri::Manager;` lib.rs import. +- **Verified:** user confirmed adopt works (scrollback intact, same pane id, live input). `tsc -b` clean. +- Committed (`bea6cf2`) together with the carried-over `use tauri::Manager;` lib.rs import. + +**Follow-on fixes same session (commit after `bea6cf2`):** + +- **B1 drag ghost (done).** Cursor-following chip via `createPortal` in LeafPane, `pointer-events:none` so it doesn't disturb the `elementFromPoint` drop-target hit-test. Turns orange "↗ New window" past the 60px edge margin. A webview **can't paint outside its own OS window**, so the chip is clamped to the viewport edge and flips to the cursor's inner side near right/bottom rather than vanishing — that's the best achievable; a ghost floating over the desktop is impossible. Hoisted `PANE_DRAG_OUT_MARGIN` + `isFarOutsideViewport()` to module scope so move-handler (preview) and up-handler (release) can't drift. +- **Drag-out "PTY not ready" (mitigated).** `moveToNewWindow` now `await waitForPaneRegistration(leafId, 5000)` instead of failing instantly when the id isn't registered yet — covers the race where a just-spawned/just-adopted pane is dragged before its async spawn round-trip registers. Resolves instantly if already registered. +- **Tab accumulation (root-caused + fixed).** The cross-window save aggregator (`window_state.rs::build_envelope`) concatenated EVERY window's workspaces into the saved file; on launch main loaded the whole blob and adopted it as its own tabs, then re-saved under "main" → unbounded growth (hit 14 tabs incl. `Pane 28`/`Pane 38` drag-out artifacts + piles of `Default` from pre-fix detached boots). Fix: `build_envelope` persists **only `MAIN_WINDOW_LABEL`'s** workspaces — detached windows are ephemeral by design (discarded on close), so they're now structurally unable to pollute the file. **Reset the corrupted `workspace.json`** (backed up to `workspace.json.corrupt-backup` in app config dir, then deleted; main reboots a clean single Default). Detached windows still `push_window_workspaces` (harmless; backend just ignores non-main for persistence). +- **Can't close tabs (fixed).** Tab strip is `overflow-x:auto`, which per spec coerces `overflow-y` to auto too → the in-strip absolutely-positioned close-confirm popover got clipped once enough tabs forced horizontal scroll. Fix: `createPortal` the confirm to ``, `position:fixed`, fixed `width:300px` (matches `CONFIRM_POPOVER_WIDTH` const in TabStrip.tsx), right-aligned to the × button then **clamped into the viewport** so a left-side tab doesn't run off the left edge. +- **Native scrollbars (fixed).** `::-webkit-scrollbar` theming was scoped to `.xterm-viewport` only; made it global (`*::-webkit-scrollbar` + `* { scrollbar-width/color }`) so the tab strip / panels / menus match the dark theme. +- **Capability fix recap:** `default.json` `"windows": ["main", "pane-window-*"]` — the load-bearing fix for the whole detached-window feature (B2–B5). Confirmed: app-defined Tauri commands aren't individually permission-gated; they're available to any window the capability is *applied* to (listed in `windows`). 2. `pnpm tauri dev` — smoke test: - Existing workspace loads as one tab named "Default" ✓ migrate diff --git a/src-tauri/src/window_state.rs b/src-tauri/src/window_state.rs index 0529464..a6ba575 100644 --- a/src-tauri/src/window_state.rs +++ b/src-tauri/src/window_state.rs @@ -66,29 +66,20 @@ impl WindowsState { } } - /// Build the on-disk envelope by concatenating every window's - /// workspaces in stable label order (main first when present, then - /// the rest sorted alphabetically by label — deterministic so the - /// file diff stays stable across no-op saves). + /// Build the on-disk envelope from ONLY the main window's workspaces. + /// + /// Detached windows are ephemeral — their tabs are discarded on close + /// (Chrome-style), and only the main window's tabs are meant to survive + /// a restart. Persisting every window's workspaces (the original design) + /// let detached windows' tabs — and the `Pane N` adopt-targets from + /// drag-out — leak into the saved file; on the next launch the main + /// window loaded the whole blob and adopted them all, so they + /// accumulated without bound. Keying the persisted set to the main label + /// makes detached state structurally unable to pollute it. fn build_envelope(&self) -> Value { let map = self.per_window.lock(); - let mut keys: Vec<&String> = map.keys().collect(); - keys.sort_by(|a, b| { - // main first, then alpha - match (a.as_str(), b.as_str()) { - (MAIN_WINDOW_LABEL, _) => std::cmp::Ordering::Less, - (_, MAIN_WINDOW_LABEL) => std::cmp::Ordering::Greater, - (x, y) => x.cmp(y), - } - }); - let mut workspaces: Vec = Vec::new(); - for k in keys { - if let Some(list) = map.get(k) { - for w in list { - workspaces.push(w.clone()); - } - } - } + let workspaces: Vec = + map.get(MAIN_WINDOW_LABEL).cloned().unwrap_or_default(); serde_json::json!({ "version": 2, "workspaces": workspaces, diff --git a/src/App.tsx b/src/App.tsx index 484c02a..07fefe2 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1005,10 +1005,19 @@ export default function App() { notify("Cannot move — pane not found"); return; } - const paneId = paneIdByLeafRef.current.get(leafId); + // The pane's id is registered only after its XtermPane finishes the + // async spawn/adopt round-trip. If the user drags out a pane that's + // still completing that (e.g. just after a shell-swap, or a pane in a + // freshly-detached window), wait for registration instead of failing + // outright. Resolves immediately if already registered. + let paneId = paneIdByLeafRef.current.get(leafId); if (paneId == null) { - notify("Cannot move — PTY not ready yet"); - return; + try { + paneId = await waitForPaneRegistration(leafId, 5000); + } catch { + notify("Cannot move — PTY not ready yet"); + return; + } } try { diff --git a/src/components/TabStrip.css b/src/components/TabStrip.css index 4a3c17b..212d024 100644 --- a/src/components/TabStrip.css +++ b/src/components/TabStrip.css @@ -10,14 +10,11 @@ color: #aaa; user-select: none; overflow-x: auto; - overflow-y: visible; min-height: 28px; box-sizing: border-box; white-space: nowrap; - /* Allow the inline confirm popover to spill over the bottom edge instead - of being clipped by overflow:hidden. overflow-y:visible alongside - overflow-x:auto only works because confirm popovers position themselves - absolutely. */ + /* The confirm popover is portalled to (see TabStrip.tsx), so it is + not clipped by this strip's overflow. */ } .tab-strip-item { @@ -113,15 +110,15 @@ border-color: #2a5a8c; } -/* Inline confirm popover anchored to the close button — spills below the - strip. Plain matte panel; reuses the existing app palette. */ +/* Confirm popover anchored to the close button. Portalled to and + positioned `fixed` (top/right set inline) so the horizontally-scrolling + tab strip — overflow-x:auto forces overflow-y:auto, which would clip an + in-strip popover — can't hide it. Plain matte panel; app palette. */ .tab-strip-confirm { - position: absolute; - top: calc(100% + 4px); - right: 0; - z-index: 50; - min-width: 260px; - max-width: 360px; + position: fixed; + z-index: 1000; + /* width must match CONFIRM_POPOVER_WIDTH in TabStrip.tsx (clamp math). */ + width: 300px; background: #1a1a1a; color: #e6e6e6; border: 1px solid #c98a1f; diff --git a/src/components/TabStrip.tsx b/src/components/TabStrip.tsx index cb06e0c..8a64071 100644 --- a/src/components/TabStrip.tsx +++ b/src/components/TabStrip.tsx @@ -7,9 +7,14 @@ import { type KeyboardEvent as ReactKeyboardEvent, type MouseEvent as ReactMouseEvent, } from "react"; +import { createPortal } from "react-dom"; import { walkLeaves, leafCount, type Workspace, type NodeId } from "../lib/layout/tree"; import "./TabStrip.css"; +/** Fixed width of the close-confirm popover — must match the `width` in + * TabStrip.css so the viewport-clamp math positions it accurately. */ +const CONFIRM_POPOVER_WIDTH = 300; + interface TabStripProps { workspaces: Workspace[]; currentWorkspaceId: NodeId | null; @@ -37,6 +42,14 @@ export default function TabStrip({ const [draft, setDraft] = useState(""); const editInputRef = useRef(null); const [confirmingId, setConfirmingId] = useState(null); + // Anchor rect (the close button's) for the confirm popover. The popover is + // portalled to with position:fixed because the tab strip scrolls + // horizontally (overflow-x:auto, which forces overflow-y to auto too), + // so an in-strip absolutely-positioned popover would be clipped. + const [confirmAnchor, setConfirmAnchor] = useState<{ + top: number; + left: number; + } | null>(null); const startEdit = useCallback( (id: NodeId, current: string, e: ReactMouseEvent) => { @@ -106,6 +119,19 @@ export default function TabStrip({ onClose(id); return; } + const rect = (e.currentTarget as HTMLElement).getBoundingClientRect(); + // Right-align the popover to the close button by default, then clamp + // both edges into the viewport so a left-side tab doesn't push it off + // the left edge (or a right-side tab off the right). + const pad = 8; + const left = Math.max( + pad, + Math.min( + rect.right - CONFIRM_POPOVER_WIDTH, + window.innerWidth - CONFIRM_POPOVER_WIDTH - pad, + ), + ); + setConfirmAnchor({ top: rect.bottom + 4, left }); setConfirmingId(id); }, [workspaces, onClose], @@ -127,7 +153,6 @@ export default function TabStrip({ {workspaces.map((w) => { const isActive = w.id === currentWorkspaceId; const isEditing = editingId === w.id; - const isConfirming = confirmingId === w.id; return (
× - {isConfirming && ( -
e.stopPropagation()} - > -
- Close "{confirmingWorkspace?.name}"? -
-
- This will kill {confirmingPaneLabels.length} pane - {confirmingPaneLabels.length === 1 ? "" : "s"}: -
- {confirmingPaneLabels.join(", ")} -
-
-
- - -
-
- )}
); })} @@ -207,6 +196,46 @@ export default function TabStrip({ > + + {confirmingId != null && + confirmAnchor && + createPortal( +
e.stopPropagation()} + > +
+ Close "{confirmingWorkspace?.name}"? +
+
+ This will kill {confirmingPaneLabels.length} pane + {confirmingPaneLabels.length === 1 ? "" : "s"}: +
+ {confirmingPaneLabels.join(", ")} +
+
+
+ + +
+
, + document.body, + )} ); } diff --git a/src/lib/layout/LeafPane.css b/src/lib/layout/LeafPane.css index f5ff85f..785e5f2 100644 --- a/src/lib/layout/LeafPane.css +++ b/src/lib/layout/LeafPane.css @@ -302,3 +302,40 @@ background: #2a5a8c; color: #fff; } + +/* Cursor-following ghost shown while dragging a pane toolbar (B1). Rendered + into document.body via a portal, offset from the cursor, and pointer-events + none so it never disturbs the elementFromPoint hit-test that drives the + drop-target highlight. */ +.pane-drag-ghost { + position: fixed; + z-index: 1000; + /* transform set inline so the chip can flip to the cursor's inner side + near the right/bottom edges (keeps it visible while pinned to the edge). */ + pointer-events: none; + display: flex; + align-items: center; + gap: 8px; + max-width: 320px; + padding: 4px 10px; + border: 1px solid #5a8cd8; + border-radius: 4px; + background: rgba(20, 28, 40, 0.95); + box-shadow: 0 4px 14px rgba(0, 0, 0, 0.5); + font: inherit; + font-size: 12px; + color: #cfe0f5; + white-space: nowrap; +} +.pane-drag-ghost-label { + overflow: hidden; + text-overflow: ellipsis; +} +.pane-drag-ghost.detach { + border-color: #e09838; + color: #ffd9a0; +} +.pane-drag-ghost-hint { + font-weight: 600; + color: #ffb840; +} diff --git a/src/lib/layout/LeafPane.tsx b/src/lib/layout/LeafPane.tsx index 7fd9ee7..75ad84c 100644 --- a/src/lib/layout/LeafPane.tsx +++ b/src/lib/layout/LeafPane.tsx @@ -7,6 +7,7 @@ import { type MouseEvent, type PointerEvent as ReactPointerEvent, } from "react"; +import { createPortal } from "react-dom"; import { type LeafNode, resolveFontSize, type LeafShellSpec } from "./tree"; import { useOrchestration } from "./orchestration"; import XtermPane from "../../components/XtermPane"; @@ -15,6 +16,19 @@ import "./LeafPane.css"; const IDLE_THRESHOLD_MS = 5000; +/** How far past a viewport edge the cursor must travel before a release is + * treated as "drag pane out of window" instead of "drop on empty space + * inside this window". Picked so an accidental release on the OS titlebar + * (~30px tall) stays inside the threshold. */ +const PANE_DRAG_OUT_MARGIN = 60; + +/** True when a point is past any viewport edge by PANE_DRAG_OUT_MARGIN. */ +const isFarOutsideViewport = (x: number, y: number) => + x < -PANE_DRAG_OUT_MARGIN || + x > window.innerWidth + PANE_DRAG_OUT_MARGIN || + y < -PANE_DRAG_OUT_MARGIN || + y > window.innerHeight + PANE_DRAG_OUT_MARGIN; + export default function LeafPane({ leaf }: { leaf: LeafNode }) { const orch = useOrchestration(); const isActive = orch.activeLeafId === leaf.id; @@ -225,6 +239,17 @@ export default function LeafPane({ leaf }: { leaf: LeafNode }) { const dragStartRef = useRef<{ x: number; y: number; armed: boolean; dragging: boolean } | null>( null, ); + // Cursor-following ghost shown while dragging the toolbar. `detach` flips + // true once the cursor is past the viewport edge by PANE_DRAG_OUT_MARGIN, + // mirroring the release condition in onToolbarPointerUp so the ghost + // previews what a release right now would do. + const [dragGhost, setDragGhost] = useState<{ + x: number; + y: number; + detach: boolean; + flipX: boolean; + flipY: boolean; + } | null>(null); const isDragSource = orch.dragSourceId === leaf.id; const isDragTarget = orch.dragOverId === leaf.id && orch.dragSourceId !== leaf.id; @@ -264,16 +289,27 @@ export default function LeafPane({ leaf }: { leaf: LeafNode }) { const tEl = el?.closest("[data-leaf-id]"); const targetId = tEl?.getAttribute("data-leaf-id") ?? null; orch.setHeaderDragOver(targetId); + // Move the cursor-following ghost (B1). It has pointer-events:none so + // it doesn't interfere with the elementFromPoint hit-test above. + // A webview can't paint outside its own OS window, so once the cursor + // crosses the edge we clamp the chip to the viewport (and flip it to + // the cursor's inner side near right/bottom) so it stays visible and + // its `detach` styling is what previews the release. `detach` itself + // is computed from the RAW cursor position so the preview is accurate. + const GHOST_PAD = 4; + const FLIP_X_ZONE = 180; // ~max chip width + const FLIP_Y_ZONE = 48; + setDragGhost({ + x: Math.max(GHOST_PAD, Math.min(e.clientX, window.innerWidth - GHOST_PAD)), + y: Math.max(GHOST_PAD, Math.min(e.clientY, window.innerHeight - GHOST_PAD)), + detach: isFarOutsideViewport(e.clientX, e.clientY), + flipX: e.clientX > window.innerWidth - FLIP_X_ZONE, + flipY: e.clientY > window.innerHeight - FLIP_Y_ZONE, + }); }, [orch.beginHeaderDrag, orch.setHeaderDragOver, leaf.id], ); - /** How far past a viewport edge the cursor must travel before a release - * is treated as "drag pane out of window" instead of "drop on empty - * space inside this window". Picked so an accidental release on the OS - * titlebar (~30px tall) stays inside the threshold. */ - const PANE_DRAG_OUT_MARGIN = 60; - const onToolbarPointerUp = useCallback( (e: ReactPointerEvent) => { const st = dragStartRef.current; @@ -281,14 +317,11 @@ export default function LeafPane({ leaf }: { leaf: LeafNode }) { (e.currentTarget as HTMLElement).releasePointerCapture(e.pointerId); const wasDragging = st.dragging; dragStartRef.current = null; + setDragGhost(null); if (!wasDragging) return; document.body.style.cursor = ""; - const releasedFarOutside = - e.clientX < -PANE_DRAG_OUT_MARGIN || - e.clientX > window.innerWidth + PANE_DRAG_OUT_MARGIN || - e.clientY < -PANE_DRAG_OUT_MARGIN || - e.clientY > window.innerHeight + PANE_DRAG_OUT_MARGIN; + const releasedFarOutside = isFarOutsideViewport(e.clientX, e.clientY); if (releasedFarOutside) { // Cancel any in-flight swap state without committing, then pop @@ -310,6 +343,7 @@ export default function LeafPane({ leaf }: { leaf: LeafNode }) { (e.currentTarget as HTMLElement).releasePointerCapture(e.pointerId); const wasDragging = st.dragging; dragStartRef.current = null; + setDragGhost(null); if (wasDragging) { document.body.style.cursor = ""; orch.endHeaderDrag(false); @@ -579,6 +613,26 @@ export default function LeafPane({ leaf }: { leaf: LeafNode }) { )} + {dragGhost && + createPortal( + , + document.body, + )} ); } diff --git a/src/styles.css b/src/styles.css index 51390ff..78989e6 100644 --- a/src/styles.css +++ b/src/styles.css @@ -38,28 +38,31 @@ body { .xterm { height: 100%; } .xterm-viewport { background: #0c0c0c !important; } -/* Themed scrollbars — Chromium pseudo-elements (WebView2 supports these). */ -.xterm-viewport::-webkit-scrollbar { +/* Themed scrollbars — Chromium pseudo-elements (WebView2 supports these). + Applied globally so every scroll container (tab strip, panels, menus, + xterm viewport) matches the dark theme instead of falling back to the + native WebView2 scrollbar. */ +*::-webkit-scrollbar { width: 8px; height: 8px; } -.xterm-viewport::-webkit-scrollbar-track { +*::-webkit-scrollbar-track { background: transparent; } -.xterm-viewport::-webkit-scrollbar-thumb { +*::-webkit-scrollbar-thumb { background: #2a2a2a; border-radius: 4px; border: 1px solid #1a1a1a; } -.xterm-viewport::-webkit-scrollbar-thumb:hover { +*::-webkit-scrollbar-thumb:hover { background: #3a3a3a; } -.xterm-viewport::-webkit-scrollbar-corner { +*::-webkit-scrollbar-corner { background: transparent; } /* Firefox fallback (and the new spec) — not strictly needed in WebView2 but free-and-correct. */ -.xterm-viewport { +* { scrollbar-width: thin; scrollbar-color: #2a2a2a transparent; }