From de53ecf68973712b34daa370ff7d2e00d771e356 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 19 May 2026 16:55:52 -0700 Subject: [PATCH] Preserve terminal scroll across hidden worktree resume Preserve the last visible terminal scroll state before hiding so worktree switches and hidden layout changes restore the expected viewport. --- .../use-terminal-pane-global-effects.test.ts | 81 ++++++++++++++++++- .../use-terminal-pane-global-effects.ts | 17 +++- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts index 44d1a6a9e8c..66500f56bba 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts @@ -11,6 +11,20 @@ const mocks = vi.hoisted(() => ({ restoreScrollState: vi.fn() })) +const reactRefState = vi.hoisted(() => ({ + slots: [] as { current: unknown }[], + index: 0 +})) + +function beginHookRender(): void { + reactRefState.index = 0 +} + +function resetHookRefs(): void { + reactRefState.slots = [] + reactRefState.index = 0 +} + vi.mock('react', async (importOriginal) => { const actual = await importOriginal() return { @@ -18,7 +32,14 @@ vi.mock('react', async (importOriginal) => { useEffect: (effect: () => void | (() => void)) => { effect() }, - useRef: (value: T) => ({ current: value }) + useRef: (value: T) => { + const index = reactRefState.index + reactRefState.index += 1 + if (!reactRefState.slots[index]) { + reactRefState.slots[index] = { current: value } + } + return reactRefState.slots[index] as { current: T } + } } }) @@ -80,6 +101,7 @@ function useMountForFileDrop( } const paneTransports = new Map() + beginHookRender() useTerminalPaneGlobalEffects({ tabId: options.tabId ?? 'tab-1', worktreeId: options.worktreeId ?? 'wt-1', @@ -99,6 +121,7 @@ function useMountForFileDrop( describe('useTerminalPaneGlobalEffects', () => { beforeEach(() => { + resetHookRefs() vi.clearAllMocks() ;(globalThis as unknown as { window: unknown }).window = { addEventListener: vi.fn(), @@ -146,6 +169,7 @@ describe('useTerminalPaneGlobalEffects', () => { const isActiveRef = { current: false } const isVisibleRef = { current: false } + beginHookRender() useTerminalPaneGlobalEffects({ tabId: 'tab-1', worktreeId: 'wt-1', @@ -174,6 +198,61 @@ describe('useTerminalPaneGlobalEffects', () => { expect(isVisibleRef.current).toBe(true) }) + it('restores from the pre-hide scroll state when hidden layout changes the viewport', () => { + const terminalA = { name: 'terminal-a' } + const manager = { + getPanes: vi.fn(() => [{ id: 1, terminal: terminalA }]), + resumeRendering: vi.fn(), + suspendRendering: vi.fn(), + fitAllPanes: vi.fn(), + getActivePane: vi.fn(() => null), + setActivePane: vi.fn() + } + const initialState = { marker: 'initial' } + const preHideState = { marker: 'before-hide' } + const corruptedHiddenState = { marker: 'hidden-corrupted' } + let nextCapturedState = initialState + mocks.captureScrollState.mockImplementation(() => nextCapturedState) + + const baseArgs = { + tabId: 'tab-1', + worktreeId: 'wt-1', + managerRef: { current: manager as never }, + containerRef: { current: null }, + paneTransportsRef: { current: new Map() }, + isActiveRef: { current: false }, + isVisibleRef: { current: false }, + toggleExpandPane: vi.fn() + } + + beginHookRender() + useTerminalPaneGlobalEffects({ + ...baseArgs, + isActive: true, + isVisible: true + }) + + nextCapturedState = preHideState + beginHookRender() + useTerminalPaneGlobalEffects({ + ...baseArgs, + isActive: false, + isVisible: false + }) + + nextCapturedState = corruptedHiddenState + beginHookRender() + useTerminalPaneGlobalEffects({ + ...baseArgs, + isActive: true, + isVisible: true + }) + + expect(mocks.captureScrollState).toHaveBeenCalledTimes(2) + expect(manager.suspendRendering).toHaveBeenCalledTimes(1) + expect(mocks.restoreScrollState).toHaveBeenLastCalledWith(terminalA, preHideState) + }) + it('ignores terminal file drops for another terminal tab', () => { const { onFileDrop } = useMountForFileDrop() diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index 028a20987da..4f5048e4e24 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -16,6 +16,7 @@ import { handleFocusTerminalPaneDetail } from './focus-terminal-pane-event' import { surfaceStaleAgentRow } from './stale-agent-row' import { useAppStore } from '@/store' import { captureScrollState, restoreScrollState } from '@/lib/pane-manager/pane-scroll' +import type { ScrollState } from '@/lib/pane-manager/pane-manager-types' type UseTerminalPaneGlobalEffectsArgs = { tabId: string @@ -53,6 +54,7 @@ export function useTerminalPaneGlobalEffects({ // otherwise leak WebGL contexts — openTerminal() unconditionally creates // one — and exhaust Chromium's ~8-context budget across worktrees. const wasVisibleRef = useRef(true) + const scrollStatesBeforeHideRef = useRef | null>(null) useEffect(() => { const manager = managerRef.current @@ -64,9 +66,11 @@ export function useTerminalPaneGlobalEffects({ // post-resume fit runs. Capture numeric viewport positions first; the // restore path avoids content matching so duplicate agent log lines do // not jump to the wrong history entry. - const viewportPositions = new Map( - manager.getPanes().map((pane) => [pane.id, captureScrollState(pane.terminal)] as const) - ) + const viewportPositions = + scrollStatesBeforeHideRef.current && scrollStatesBeforeHideRef.current.size > 0 + ? scrollStatesBeforeHideRef.current + : capturePaneScrollStates(manager) + scrollStatesBeforeHideRef.current = null // Why: background PTY output is throttled while a pane is not focused; // flush it before fitting so newly visible terminals paint current state. for (const pane of manager.getPanes()) { @@ -92,6 +96,9 @@ export function useTerminalPaneGlobalEffects({ } } } else if (wasVisibleRef.current) { + // Why: hidden DOM/layout churn can mutate xterm's viewport before the + // pane becomes visible again. Preserve the last visible position. + scrollStatesBeforeHideRef.current = capturePaneScrollStates(manager) // Suspend WebGL when going hidden. xterm.write() continues to land in // the (now DOM-renderer-fallback or paused-canvas) terminal; the // suspend is purely a GPU resource decision. @@ -308,3 +315,7 @@ export function useTerminalPaneGlobalEffects({ }) }, [isActive, isVisible, managerRef, paneTransportsRef, tabId]) } + +function capturePaneScrollStates(manager: PaneManager): Map { + return new Map(manager.getPanes().map((pane) => [pane.id, captureScrollState(pane.terminal)])) +}