From 445ecaf82ea85544f1af7ae6ee6169c10c30dd9a Mon Sep 17 00:00:00 2001 From: Blomios Date: Sat, 18 Jul 2026 09:41:39 +0200 Subject: [PATCH] =?UTF-8?q?fix(frontend):=20refit=20diff=C3=A9r=C3=A9=20de?= =?UTF-8?q?s=20cellules=20terminal=20apr=C3=A8s=20mutation=20de=20layout?= =?UTF-8?q?=20(#61)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le ResizeObserver de TerminalView ne déclenche pas toujours un événement utile quand une cellule voisine apparaît/disparaît (split/merge), forçant l'utilisateur à redimensionner la fenêtre pour rafraîchir le scaling xterm. useLayout expose un layoutVersion bumpé à chaque commit d'arbre (chargement initial inclus), relayé par LayoutGrid comme refitSignal à chaque TerminalView survivant pour déclencher fit.fit() sans rouvrir le PTY. Co-Authored-By: Claude Sonnet 5 --- frontend/src/features/layout/LayoutGrid.tsx | 1 + frontend/src/features/layout/useLayout.ts | 31 +++++++-- .../features/terminals/TerminalView.test.tsx | 65 +++++++++++++++++++ .../src/features/terminals/TerminalView.tsx | 41 +++++++++++- 4 files changed, 130 insertions(+), 8 deletions(-) diff --git a/frontend/src/features/layout/LayoutGrid.tsx b/frontend/src/features/layout/LayoutGrid.tsx index f6b7e0f..64ad1df 100644 --- a/frontend/src/features/layout/LayoutGrid.tsx +++ b/frontend/src/features/layout/LayoutGrid.tsx @@ -917,6 +917,7 @@ function LeafView({ onSessionId={(sid) => void vm.setSession(id, sid)} agentMode={agentId != null} portal={agentId != null ? portal : undefined} + refitSignal={vm.layoutVersion} /> {/* Write-portal overlay (ARCHITECTURE §20.3 step b/e): while a delegation is being injected into the agent's PTY, a grey veil with a centred diff --git a/frontend/src/features/layout/useLayout.ts b/frontend/src/features/layout/useLayout.ts index 9f810b8..e50f152 100644 --- a/frontend/src/features/layout/useLayout.ts +++ b/frontend/src/features/layout/useLayout.ts @@ -23,6 +23,15 @@ import { leaves, splitOp } from "./layout"; export interface LayoutViewModel { /** The current layout tree, or `null` until loaded. */ layout: LayoutTree | null; + /** + * Bumped every time a mutation (split/merge/resize/move/…) commits a new + * tree, INCLUDING the initial load (ticket #61). `LayoutGrid` forwards it to + * every mounted `TerminalView` as `refitSignal` so surviving terminals refit + * after a structural change (split/merge) instead of waiting for a manual + * window resize — their own `ResizeObserver` doesn't always fire a useful + * event when a SIBLING cell appears or disappears. + */ + layoutVersion: number; /** Last error message, or `null`. */ error: string | null; /** Whether a request is in flight. */ @@ -65,9 +74,18 @@ export function useLayout( ): LayoutViewModel { const { layout: gateway, terminal } = useGateways(); const [layout, setLayout] = useState(null); + const [layoutVersion, setLayoutVersion] = useState(0); const [error, setError] = useState(null); const [busy, setBusy] = useState(false); + // Commits a freshly-loaded/mutated tree AND bumps the refit signal (ticket + // #61) in one place, so every caller below (initial load, `mutate`, + // `mutateChain`) stays a one-liner instead of repeating the pair. + const applyTree = useCallback((tree: LayoutTree) => { + setLayout(tree); + setLayoutVersion((v) => v + 1); + }, []); + useEffect(() => { // No project, or a DI subset without the layout gateway (e.g. a focused // test) → render nothing rather than crash the tab (mirrors TerminalView). @@ -80,7 +98,7 @@ export function useLayout( gateway .loadLayout(projectId, layoutId) .then((tree) => { - if (!cancelled) setLayout(tree); + if (!cancelled) applyTree(tree); }) .catch((e: unknown) => { if (!cancelled) setError(describe(e)); @@ -91,7 +109,7 @@ export function useLayout( return () => { cancelled = true; }; - }, [gateway, projectId, layoutId]); + }, [gateway, projectId, layoutId, applyTree]); const mutate = useCallback( async (operation: LayoutOperation) => { @@ -99,14 +117,14 @@ export function useLayout( setBusy(true); setError(null); try { - setLayout(await gateway.mutateLayout(projectId, operation, layoutId)); + applyTree(await gateway.mutateLayout(projectId, operation, layoutId)); } catch (e) { setError(describe(e)); } finally { setBusy(false); } }, - [gateway, projectId, layoutId], + [gateway, projectId, layoutId, applyTree], ); // Applies several operations in series, persisting each, but commits a single @@ -124,14 +142,14 @@ export function useLayout( for (const op of operations) { tree = await gateway.mutateLayout(projectId, op, layoutId); } - if (tree) setLayout(tree); + if (tree) applyTree(tree); } catch (e) { setError(describe(e)); } finally { setBusy(false); } }, - [gateway, projectId, layoutId], + [gateway, projectId, layoutId, applyTree], ); const split = useCallback( @@ -230,6 +248,7 @@ export function useLayout( return { layout, + layoutVersion, error, busy, split, diff --git a/frontend/src/features/terminals/TerminalView.test.tsx b/frontend/src/features/terminals/TerminalView.test.tsx index b24ab37..864af61 100644 --- a/frontend/src/features/terminals/TerminalView.test.tsx +++ b/frontend/src/features/terminals/TerminalView.test.tsx @@ -13,6 +13,8 @@ import { describe, it, expect, vi, beforeAll, afterAll } from "vitest"; import { render, screen, waitFor } from "@testing-library/react"; +import { FitAddon } from "@xterm/addon-fit"; + import type { Gateways, ReattachResult, @@ -289,4 +291,67 @@ describe("TerminalView — visible launch-failure surface (ticket #14 F3)", () = expect(screen.queryByRole("alert")).toBeNull(); expect(screen.queryByTestId("terminal-error")).toBeNull(); }); + + describe("refitSignal (ticket #61 — refit after split/merge)", () => { + it("refits WITHOUT reopening the terminal when refitSignal changes", async () => { + // Simulates LayoutGrid bumping `useLayout`'s layout version after a + // split/merge: a survivor cell must refit its xterm grid, but must NOT + // re-run open/reattach — that would tear down and relaunch its PTY. + const fitSpy = vi.spyOn(FitAddon.prototype, "fit"); + const open = vi.fn(async () => makeHandle({ sessionId: "survivor-1" })); + + const { rerender } = renderView(new MockTerminalGateway(), "/cwd", { + open, + refitSignal: 1, + }); + await waitFor(() => expect(open).toHaveBeenCalledTimes(1)); + + const fitCallsAtMount = fitSpy.mock.calls.length; + expect(fitCallsAtMount).toBeGreaterThan(0); + + // jsdom reports a zero-size layout box, which the coalesced refit + // deliberately skips (the same guard that protects the resize-observer + // path from fitting to a transient zero size). Stub a real size on the + // inner xterm container so the refit triggered below actually reaches + // `fit.fit()` instead of bailing on the zero-size guard. + const container = screen.getByTestId("terminal-view").firstElementChild as HTMLElement; + Object.defineProperty(container, "clientWidth", { value: 400, configurable: true }); + Object.defineProperty(container, "clientHeight", { value: 200, configurable: true }); + + rerender( + + + , + ); + + await waitFor(() => + expect(fitSpy.mock.calls.length).toBeGreaterThan(fitCallsAtMount), + ); + // The structural-mutation refit must never reopen the PTY. + expect(open).toHaveBeenCalledTimes(1); + + fitSpy.mockRestore(); + }); + + it("does not refit when refitSignal is left undefined (no-op for callers that don't pass it)", async () => { + const fitSpy = vi.spyOn(FitAddon.prototype, "fit"); + const open = vi.fn(async () => makeHandle({ sessionId: "no-signal-1" })); + + const { rerender } = renderView(new MockTerminalGateway(), "/cwd", { open }); + await waitFor(() => expect(open).toHaveBeenCalledTimes(1)); + fitSpy.mockClear(); + + rerender( + + + , + ); + + // Give any stray rAF a chance to fire, then assert nothing extra happened. + await new Promise((resolve) => requestAnimationFrame(resolve)); + expect(fitSpy).not.toHaveBeenCalled(); + + fitSpy.mockRestore(); + }); + }); }); diff --git a/frontend/src/features/terminals/TerminalView.tsx b/frontend/src/features/terminals/TerminalView.tsx index fede0f4..58d826f 100644 --- a/frontend/src/features/terminals/TerminalView.tsx +++ b/frontend/src/features/terminals/TerminalView.tsx @@ -102,6 +102,17 @@ interface TerminalViewProps { * the mobile key toolbar. Not called when xterm fails to mount (headless). */ onReady?: (api: TerminalInputApi) => void; + /** + * Bumped by the caller after a structural layout mutation (split/merge, + * ticket #61) so surviving terminals refit even when their container's + * `ResizeObserver` doesn't fire a useful event for the mutation (e.g. a + * sibling appearing/disappearing without this cell's own box changing size + * in a way the observer flags in time). Any value change schedules exactly + * one coalesced refit on the next animation frame, reusing the same + * zero-size guard and rows/cols-changed check as the resize-observer path — + * it never remounts/reopens the terminal. + */ + refitSignal?: number; } /** @@ -132,6 +143,7 @@ export function TerminalView({ agentMode = false, portal, onReady, + refitSignal, }: TerminalViewProps) { const { terminal } = useGateways(); const containerRef = useRef(null); @@ -165,6 +177,10 @@ export function TerminalView({ portalRef.current = portal; const onReadyRef = useRef(onReady); onReadyRef.current = onReady; + // Holds the mounted instance's `refit` closure so the `refitSignal` effect + // below (a separate effect, since it must NOT re-run/reopen the terminal on + // every parent render) can trigger it without depending on `cwd`'s effect. + const refitRef = useRef<(() => void) | null>(null); useEffect(() => { const container = containerRef.current; @@ -337,14 +353,21 @@ export function TerminalView({ void handle.resize(term.rows, term.cols); } }; - const ro = new ResizeObserver(() => { + const scheduleRefit = () => { if (rafId) cancelAnimationFrame(rafId); rafId = requestAnimationFrame(refit); - }); + }; + const ro = new ResizeObserver(scheduleRefit); ro.observe(container); + // Let the `refitSignal` effect below trigger the SAME coalesced refit after + // a structural layout mutation (split/merge, ticket #61) — surviving cells + // don't always get a timely useful ResizeObserver event from a sibling + // appearing/disappearing. + refitRef.current = scheduleRefit; return () => { disposed = true; + refitRef.current = null; if (rafId) cancelAnimationFrame(rafId); ro.disconnect(); onKey.dispose(); @@ -360,6 +383,20 @@ export function TerminalView({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [cwd]); + // Ticket #61 — explicit refit after a structural layout mutation (split / + // merge). A survivor's own container may not fire a timely, useful + // ResizeObserver event when a SIBLING cell appears/disappears, so the caller + // (LayoutGrid, via `useLayout`'s layout-tree version) bumps `refitSignal` on + // every successful mutation and we reuse the mounted instance's coalesced + // `refit` (same zero-size guard, same rows/cols-changed check) instead of + // remounting/reopening the terminal. Deliberately a SEPARATE effect from the + // `[cwd]` one above so bumping the signal never re-runs the open/reattach + // logic. + useEffect(() => { + if (refitSignal === undefined) return; + refitRef.current?.(); + }, [refitSignal]); + return (