Keep run graph zoom/pan when switching tabs (#561)

Switching from a run's Overview tab to another tab and back reset the
graph zoom and position to the default. Now it holds.

## Why

The viewport (pan and zoom) lived in `RunOverview` component state.
Overview and Stages are sibling routes under `runs/:id`, so switching
tabs unmounts Overview and drops that state.

## Fix

`apps/fabro-web/app/routes/run-overview.tsx`: cache the viewport per run
outside the component so it survives the remount, and reset it when the
run id changes, since the route instance is reused when only the id
changes.

Added two tests: viewport restores on remount for the same run, and does
not carry across runs.

Does not persist across a full page reload (in-memory only).

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Bryan Helmkamp <bryan@brynary.com>

apps/fabro-web/app/components/graph-toolbar.tsx

@@ -1,23 +1,24 @@
 import { ArrowDownIcon, ArrowRightIcon, MinusIcon, PlusIcon } from "@heroicons/react/20/solid";
 
+import { GRAPH_MAX_ZOOM, GRAPH_MIN_ZOOM } from "../lib/graph-viewport";
+
 type Direction = "LR" | "TB";
 
+// +/- button step. Zoom-out uses the reciprocal, keeping it symmetric with zoom-in.
+const ZOOM_STEP_FACTOR = 1.25;
+
 export function GraphToolbar({
   direction,
   setDirection,
   fitToWindow,
-  onZoomIn,
-  onZoomOut,
-  canZoomIn,
-  canZoomOut,
+  zoom,
+  onZoomBy,
 }: {
   direction: Direction;
   setDirection: (d: Direction) => void;
   fitToWindow: () => void;
-  onZoomIn: () => void;
-  onZoomOut: () => void;
-  canZoomIn: boolean;
-  canZoomOut: boolean;
+  zoom: number;
+  onZoomBy: (factor: number) => void;
 }) {
   const group =
     "flex items-center gap-0.5 px-0.5 [&:not(:first-child)]:border-l [&:not(:first-child)]:border-line-strong [&:not(:first-child)]:pl-1 [&:not(:first-child)]:ml-1";
@@ -70,8 +71,8 @@ export function GraphToolbar({
         <button
           type="button"
           title="Zoom out"
-          onClick={onZoomOut}
-          disabled={!canZoomOut}
+          onClick={() => onZoomBy(1 / ZOOM_STEP_FACTOR)}
+          disabled={zoom <= GRAPH_MIN_ZOOM}
           className={`${btn} ${btnDisabled}`}
         >
           <MinusIcon className="size-4" aria-hidden="true" />
@@ -79,8 +80,8 @@ export function GraphToolbar({
         <button
           type="button"
           title="Zoom in"
-          onClick={onZoomIn}
-          disabled={!canZoomIn}
+          onClick={() => onZoomBy(ZOOM_STEP_FACTOR)}
+          disabled={zoom >= GRAPH_MAX_ZOOM}
           className={`${btn} ${btnDisabled}`}
         >
           <PlusIcon className="size-4" aria-hidden="true" />

apps/fabro-web/app/hooks/effects.ts

@@ -71,6 +71,48 @@ export function useDebouncedValue<T>(value: T, delayMs: number): T {
   return debounced;
 }
 
+/**
+ * Shared body of the event-listener hooks below. The target is read when the
+ * effect runs (so `window`/`document` absence and not-yet-mounted elements are
+ * handled uniformly); the listener is removed before resubscribe and on unmount;
+ * the handler sees the latest render without forcing a resubscribe. Callers must
+ * pass a stable `options` value — a fresh object every render resubscribes the
+ * listener every render.
+ */
+function useEventTargetEvent<E extends Event>(
+  target: { readonly current: EventTarget | null },
+  type: string,
+  handler: (event: E) => void,
+  options: AddEventListenerOptions | boolean | undefined,
+  active: boolean,
+): void {
+  const handlerRef = useRef(handler);
+  handlerRef.current = handler;
+
+  useEffect(() => {
+    const el = target.current;
+    if (!active || !el) return undefined;
+    const listener: EventListener = (event) => handlerRef.current(event as E);
+    el.addEventListener(type, listener, options);
+    return () => {
+      el.removeEventListener(type, listener, options);
+    };
+  }, [active, options, target, type]);
+}
+
+// Lazy getters so `window`/`document` are only touched when an effect runs, never
+// at module load (matching the previous per-hook `typeof window` checks).
+const WINDOW_TARGET = {
+  get current(): EventTarget | null {
+    return typeof window === "undefined" ? null : window;
+  },
+};
+const DOCUMENT_TARGET = {
+  get current(): EventTarget | null {
+    return typeof document === "undefined" ? null : document;
+  },
+};
+
 /**
  * Synchronizes React with a browser `window` event listener. The listener is
  * removed before resubscribe and on unmount; the handler sees the latest render.
@@ -81,17 +123,7 @@ export function useWindowEvent<K extends keyof WindowEventMap>(
   options?: AddEventListenerOptions | boolean,
   active = true,
 ): void {
-  const handlerRef = useRef(handler);
-  handlerRef.current = handler;
-
-  useEffect(() => {
-    if (!active || typeof window === "undefined") return undefined;
-    const listener = (event: WindowEventMap[K]) => handlerRef.current(event);
-    window.addEventListener(type, listener as EventListener, options);
-    return () => {
-      window.removeEventListener(type, listener as EventListener, options);
-    };
-  }, [active, options, type]);
+  useEventTargetEvent(WINDOW_TARGET, type, handler, options, active);
 }
 
 /**
@@ -104,17 +136,7 @@ export function useDocumentEvent<K extends keyof DocumentEventMap>(
   options?: AddEventListenerOptions | boolean,
   active = true,
 ): void {
-  const handlerRef = useRef(handler);
-  handlerRef.current = handler;
-
-  useEffect(() => {
-    if (!active || typeof document === "undefined") return undefined;
-    const listener = (event: DocumentEventMap[K]) => handlerRef.current(event);
-    document.addEventListener(type, listener as EventListener, options);
-    return () => {
-      document.removeEventListener(type, listener as EventListener, options);
-    };
-  }, [active, options, type]);
+  useEventTargetEvent(DOCUMENT_TARGET, type, handler, options, active);
 }
 
 /**
@@ -122,6 +144,9 @@ export function useDocumentEvent<K extends keyof DocumentEventMap>(
  * removed before resubscribe and on unmount; the handler sees the latest render.
  * Unlike a JSX event prop, this can pass `{ passive: false }` so the handler may
  * `preventDefault()` (e.g. to own wheel/⌘-scroll instead of the browser).
+ *
+ * The element must exist when the effect runs: if it renders conditionally, gate
+ * with `active` on the same condition so the effect re-runs once it mounts.
  */
 export function useElementEvent<K extends keyof HTMLElementEventMap>(
   ref: RefObject<HTMLElement | null>,
@@ -130,18 +155,7 @@ export function useElementEvent<K extends keyof HTMLElementEventMap>(
   options?: AddEventListenerOptions | boolean,
   active = true,
 ): void {
-  const handlerRef = useRef(handler);
-  handlerRef.current = handler;
-
-  useEffect(() => {
-    const el = ref.current;
-    if (!active || !el) return undefined;
-    const listener = (event: HTMLElementEventMap[K]) => handlerRef.current(event);
-    el.addEventListener(type, listener as EventListener, options);
-    return () => {
-      el.removeEventListener(type, listener as EventListener, options);
-    };
-  }, [active, options, ref, type]);
+  useEventTargetEvent(ref, type, handler, options, active);
 }
 
 /**

apps/fabro-web/app/hooks/use-remembered-graph-view.ts

@@ -0,0 +1,39 @@
+import { useEffect, useRef, useState } from "react";
+import type { Dispatch, SetStateAction } from "react";
+
+import { DEFAULT_GRAPH_VIEW, type GraphView } from "../lib/graph-viewport";
+
+// Remember each run's graph pan/zoom so it survives leaving and returning to the Overview
+// tab — that route unmounts when you switch to Stages/Files/etc. and remounts on return, so
+// the viewport can't live only in component state. In-memory for the session; a full reload
+// starts fresh. Tradeoff: unpruned Map, entries are three numbers each and a session views
+// few runs — swap for an LRU if that ever stops holding.
+const graphViewByRun = new Map<string, GraphView>();
+
+const loadGraphView = (runId: string | undefined): GraphView =>
+  (runId ? graphViewByRun.get(runId) : undefined) ?? DEFAULT_GRAPH_VIEW;
+
+/**
+ * Synchronizes a run's graph viewport with the session-scoped store above, so the
+ * viewport is remembered per run across unmounts. `runId` controls which entry the
+ * state binds to: when it changes, the returned view resets to that run's remembered
+ * (or default) viewport instead of carrying the previous run's over. Every update is
+ * written back to the store; the store itself needs no cleanup. Safe under Strict
+ * Mode — the write is idempotent and re-runs persist the same committed value.
+ */
+export function useRememberedGraphView(
+  runId: string | undefined,
+): [GraphView, Dispatch<SetStateAction<GraphView>>] {
+  const [view, setView] = useState<GraphView>(() => loadGraphView(runId));
+  // Callers keep their component instance when only the route param changes, so the
+  // rebind is a render-phase reset rather than a mount.
+  const viewedRunId = useRef(runId);
+  if (viewedRunId.current !== runId) {
+    viewedRunId.current = runId;
+    setView(loadGraphView(runId));
+  }
+  useEffect(() => {
+    if (runId) graphViewByRun.set(runId, view);
+  }, [runId, view]);
+  return [view, setView];
+}

apps/fabro-web/app/lib/graph-viewport.test.ts

@@ -4,6 +4,7 @@ import {
   GRAPH_MAX_ZOOM,
   GRAPH_MIN_ZOOM,
   clampZoom,
+  wheelZoomFactor,
   zoomAtPoint,
   type GraphView,
 } from "./graph-viewport";
@@ -55,3 +56,10 @@ test("clampZoom respects bounds", () => {
   expect(clampZoom(500)).toBe(GRAPH_MAX_ZOOM);
   expect(clampZoom(75)).toBe(75);
 });
+
+test("wheelZoomFactor is positive and symmetric: equal scrolls up and down cancel", () => {
+  expect(wheelZoomFactor(120)).toBeGreaterThan(0);
+  expect(wheelZoomFactor(120)).toBeLessThan(1); // scroll down zooms out
+  expect(wheelZoomFactor(-120)).toBeGreaterThan(1); // scroll up zooms in
+  expect(wheelZoomFactor(120) * wheelZoomFactor(-120)).toBeCloseTo(1);
+});

apps/fabro-web/app/lib/graph-viewport.ts

@@ -11,14 +11,27 @@ export const GRAPH_MAX_ZOOM = 200;
 
 export type GraphView = { zoom: number; pan: { x: number; y: number } };
 
+// Initial viewport shown when a graph first loads: 75% zoom, centered.
+export const DEFAULT_GRAPH_VIEW: GraphView = { zoom: 75, pan: { x: 0, y: 0 } };
+
 export const clampZoom = (zoom: number): number =>
   Math.min(GRAPH_MAX_ZOOM, Math.max(GRAPH_MIN_ZOOM, zoom));
 
+// How fast wheel/pinch input zooms; tune to taste.
+const WHEEL_SENSITIVITY = 0.002;
+
+/**
+ * Zoom factor for a wheel event's `deltaY`, for feeding into `zoomAtPoint`.
+ * exp() keeps equal scrolls up and down exact inverses and the factor above 0.
+ */
+export const wheelZoomFactor = (deltaY: number): number =>
+  Math.exp(-deltaY * WHEEL_SENSITIVITY);
+
 /**
  * Scale `view.zoom` by `factor`, keeping the content point under `cursor` fixed on
  * screen. `cursor` is measured from the container CENTER (matching the graph's
- * `transform-origin: center center`); pass {x:0,y:0} to zoom toward the center, which
- * is what the toolbar +/- buttons want.
+ * `transform-origin: center center`) and defaults to it, which is what the toolbar
+ * +/- buttons want.
  *
  * Derivation: with `translate(pan) scale(s)` about the center, a content point at
  * pre-transform offset q sits at screen offset `pan + s*q`. Holding the point under
@@ -27,7 +40,7 @@ export const clampZoom = (zoom: number): number =>
 export function zoomAtPoint(
   view: GraphView,
   factor: number,
-  cursor: { x: number; y: number },
+  cursor: { x: number; y: number } = { x: 0, y: 0 },
 ): GraphView {
   const zoom = clampZoom(view.zoom * factor);
   const k = zoom / view.zoom; // applied ratio after clamping

apps/fabro-web/app/routes/run-overview.test.tsx

@@ -1,8 +1,9 @@
-import { afterEach, describe, expect, mock, test } from "bun:test";
+import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
 import TestRenderer, { act } from "react-test-renderer";
 import { MemoryRouter, Route, Routes } from "react-router";
 
 import { ApiError } from "../lib/api-client";
+import { setupReactTestEnv } from "../lib/test-utils";
 
 let currentGraphData: string | null | undefined;
 let currentGraphError: Error | undefined;
@@ -44,12 +45,13 @@ function textFromNode(
   return (node.children ?? []).map(textFromNode).join(" ");
 }
 
-function render(): TestRenderer.ReactTestRenderer {
-  (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+// Zooming writes to useRememberedGraphView's module-scoped store, so tests that
+// change the viewport must each use a run id no other test zooms.
+function renderAt(entry: string): TestRenderer.ReactTestRenderer {
   let renderer!: TestRenderer.ReactTestRenderer;
   act(() => {
     renderer = TestRenderer.create(
-      <MemoryRouter initialEntries={["/runs/run-1"]}>
+      <MemoryRouter initialEntries={[entry]}>
         <Routes>
           <Route path="/runs/:id" element={<RunOverview />} />
         </Routes>
@@ -60,6 +62,30 @@ function render(): TestRenderer.ReactTestRenderer {
   return renderer;
 }
 
+function render(): TestRenderer.ReactTestRenderer {
+  return renderAt("/runs/run-1");
+}
+
+// The inner graph node carries `transform: translate(...) scale(...)` reflecting view.zoom/pan.
+function graphTransform(renderer: TestRenderer.ReactTestRenderer): string {
+  const node = renderer.root.findAll(
+    (n) => (n.props?.style as { transformOrigin?: string })?.transformOrigin === "center center",
+  )[0];
+  return (node.props.style as { transform: string }).transform;
+}
+
+function clickTitle(renderer: TestRenderer.ReactTestRenderer, title: string): void {
+  act(() => {
+    renderer.root.findAll((n) => n.props?.title === title)[0].props.onClick();
+  });
+}
+
+let teardownReactTestEnv: () => void;
+
+beforeEach(() => {
+  teardownReactTestEnv = setupReactTestEnv();
+});
+
 afterEach(() => {
   for (const renderer of mountedRenderers.splice(0)) {
     act(() => renderer.unmount());
@@ -68,7 +94,7 @@ afterEach(() => {
   currentGraphError = undefined;
   currentGraphLoading = false;
   graphMutateMock.mockClear();
-  delete (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT;
+  teardownReactTestEnv();
 });
 
 describe("RunOverview", () => {
@@ -87,4 +113,32 @@ describe("RunOverview", () => {
     expect(text).toContain("failed to parse DOT source");
     expect(text).not.toContain("No workflow graph");
   });
+
+  test("restores graph zoom/pan when the route remounts for the same run", () => {
+    currentGraphData = "<svg viewBox='0 0 100 100'></svg>";
+
+    const first = renderAt("/runs/zoom-persist");
+    const initial = graphTransform(first);
+    clickTitle(first, "Zoom in");
+    const zoomed = graphTransform(first);
+    expect(zoomed).not.toBe(initial);
+
+    // Switching tabs unmounts this route; returning remounts it for the same run.
+    act(() => first.unmount());
+    const second = renderAt("/runs/zoom-persist");
+    expect(graphTransform(second)).toBe(zoomed);
+  });
+
+  test("does not carry one run's zoom over to a different run", () => {
+    currentGraphData = "<svg viewBox='0 0 100 100'></svg>";
+
+    const renderer = renderAt("/runs/run-a");
+    const initial = graphTransform(renderer);
+    clickTitle(renderer, "Zoom in");
+    expect(graphTransform(renderer)).not.toBe(initial);
+
+    act(() => renderer.unmount());
+    const other = renderAt("/runs/run-b");
+    expect(graphTransform(other)).toBe(initial);
+  });
 });

apps/fabro-web/app/routes/run-overview.tsx

@@ -6,14 +6,9 @@ import { FloatingTooltip } from "../components/floating-tooltip";
 import { RunSummaryPanel } from "../components/run-summary-panel";
 import { StagePopover } from "../components/stage-popover";
 import { StageSidebar } from "../components/stage-sidebar";
-import {
-  GRAPH_MAX_ZOOM,
-  GRAPH_MIN_ZOOM,
-  clampZoom,
-  zoomAtPoint,
-  type GraphView,
-} from "../lib/graph-viewport";
+import { clampZoom, wheelZoomFactor, zoomAtPoint } from "../lib/graph-viewport";
 import { useElementEvent } from "../hooks/effects";
+import { useRememberedGraphView } from "../hooks/use-remembered-graph-view";
 import { GraphToolbar } from "../components/graph-toolbar";
 import { EmptyState, ErrorState } from "../components/state";
 import {
@@ -37,14 +32,6 @@ function parseSourceDirection(source: string | undefined): Direction | undefined
   return value === "LR" || value === "TB" ? value : undefined;
 }
 
-// Initial zoom shown when the graph first loads, in percent.
-const GRAPH_DEFAULT_ZOOM = 75;
-// Toolbar +/- step. Using 1/1.25 for zoom-out keeps it symmetric with zoom-in.
-const GRAPH_ZOOM_BUTTON_FACTOR = 1.25;
-// How fast ⌘-scroll zooms; tune to taste. exp() keeps it symmetric and always above 0.
-const GRAPH_ZOOM_WHEEL_SENSITIVITY = 0.002;
-// Zoom toward the container center. The toolbar +/- buttons anchor here, not the cursor.
-const CENTER = { x: 0, y: 0 };
 // Non-passive so the wheel handler can call preventDefault on the browser's own ⌘-zoom.
 // Kept at module scope for a stable identity, since the effect resubscribes when its
 // options object changes.
@@ -80,7 +67,7 @@ export default function RunOverview() {
   const innerRef = useRef<HTMLDivElement>(null);
   const svgRef = useRef<SVGSVGElement | null>(null);
   const navigate = useNavigate();
-  const [view, setView] = useState<GraphView>({ zoom: GRAPH_DEFAULT_ZOOM, pan: { x: 0, y: 0 } });
+  const [view, setView] = useRememberedGraphView(id);
   const dragState = useRef<{ startX: number; startY: number; startPanX: number; startPanY: number } | null>(null);
   const [hoveredNode, setHoveredNode] = useState<RunGraphNodeHover | null>(null);
 
@@ -99,8 +86,7 @@ export default function RunOverview() {
   });
 
   const onPointerDown = useCallback((e: React.PointerEvent) => {
-    if ((e.target as HTMLElement).closest("button")) return;
-    if ((e.target as HTMLElement).closest(".node")) return;
+    if (e.target instanceof Element && e.target.closest("button, .node")) return;
     e.currentTarget.setPointerCapture(e.pointerId);
     dragState.current = { startX: e.clientX, startY: e.clientY, startPanX: view.pan.x, startPanY: view.pan.y };
   }, [view.pan]);
@@ -126,12 +112,11 @@ export default function RunOverview() {
   const onWheel = useCallback((e: WheelEvent) => {
     const el = containerRef.current;
     if (!el) return;
-    if ((e.target as HTMLElement).closest('[role="toolbar"]')) return;
     e.preventDefault();
     if (e.ctrlKey || e.metaKey) {
       const r = el.getBoundingClientRect();
       const cursor = { x: e.clientX - (r.left + r.width / 2), y: e.clientY - (r.top + r.height / 2) };
-      setView((v) => zoomAtPoint(v, Math.exp(-e.deltaY * GRAPH_ZOOM_WHEEL_SENSITIVITY), cursor));
+      setView((v) => zoomAtPoint(v, wheelZoomFactor(e.deltaY), cursor));
     } else {
       setView((v) => ({ ...v, pan: { x: v.pan.x - e.deltaX, y: v.pan.y - e.deltaY } }));
     }
@@ -173,10 +158,8 @@ export default function RunOverview() {
               direction={activeDirection}
               setDirection={setDirection}
               fitToWindow={fitToWindow}
-              onZoomIn={() => setView((v) => zoomAtPoint(v, GRAPH_ZOOM_BUTTON_FACTOR, CENTER))}
-              onZoomOut={() => setView((v) => zoomAtPoint(v, 1 / GRAPH_ZOOM_BUTTON_FACTOR, CENTER))}
-              canZoomIn={view.zoom < GRAPH_MAX_ZOOM}
-              canZoomOut={view.zoom > GRAPH_MIN_ZOOM}
+              zoom={view.zoom}
+              onZoomBy={(factor) => setView((v) => zoomAtPoint(v, factor))}
             />
 
             <div