From 13bde9f161c33e86a54e84d81eb115d02fa82fa5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 29 May 2026 22:28:25 -0700 Subject: [PATCH] fix: cancel terminal pane refresh frames (#3474) --- .../components/terminal-pane/TerminalPane.tsx | 7 ++ .../terminal-pane/expand-collapse.test.ts | 96 +++++++++++++++++++ .../terminal-pane/expand-collapse.ts | 33 ++++++- 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/components/terminal-pane/expand-collapse.test.ts diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 321fee386bb..763bb6d1301 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -23,6 +23,7 @@ import { EMPTY_LAYOUT, serializeTerminalLayout } from './layout-serialization' import { makePaneKey } from '../../../../shared/stable-pane-id' import { applyExpandedLayoutTo, + cancelPendingPaneSizeRefreshFrames, createExpandCollapseActions, restoreExpandedLayoutFrom } from './expand-collapse' @@ -126,6 +127,7 @@ export default function TerminalPane({ const expandedStyleSnapshotRef = useRef>( new Map() ) + const pendingPaneSizeRefreshFrameIdsRef = useRef([]) // Why (separate from expandedStyleSnapshotRef): Activity isolation is a // transient view override that must not collide with the user-facing // expanded-pane state or the layout snapshot. Keeping its own snapshot @@ -629,6 +631,7 @@ export default function TerminalPane({ expandedStyleSnapshotRef, containerRef, managerRef, + pendingPaneSizeRefreshFrameIdsRef, setExpandedPaneId, setTabPaneExpanded, tabId, @@ -902,6 +905,10 @@ export default function TerminalPane({ } }, []) + useEffect(() => { + return () => cancelPendingPaneSizeRefreshFrames({ pendingPaneSizeRefreshFrameIdsRef }) + }, []) + const handleRestartCodexPane = useCallback( (paneId: number) => { const manager = managerRef.current diff --git a/src/renderer/src/components/terminal-pane/expand-collapse.test.ts b/src/renderer/src/components/terminal-pane/expand-collapse.test.ts new file mode 100644 index 00000000000..16abac05c36 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/expand-collapse.test.ts @@ -0,0 +1,96 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { cancelPendingPaneSizeRefreshFrames, createExpandCollapseActions } from './expand-collapse' + +type ExpandCollapseStateForTest = Parameters[0] + +function ref(current: T): React.MutableRefObject { + return { current } +} + +function createState( + overrides: Partial = {} +): ExpandCollapseStateForTest { + return { + expandedPaneIdRef: ref(null), + expandedStyleSnapshotRef: ref(new Map()), + containerRef: ref(null), + managerRef: ref(null), + pendingPaneSizeRefreshFrameIdsRef: ref([]), + setExpandedPaneId: vi.fn(), + setTabPaneExpanded: vi.fn(), + tabId: 'tab-1', + persistLayoutSnapshot: vi.fn(), + ...overrides + } +} + +describe('createExpandCollapseActions', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('cancels pending pane-size refresh frames', () => { + const cancelAnimationFrame = vi.fn() + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn(() => 42) + ) + vi.stubGlobal('cancelAnimationFrame', cancelAnimationFrame) + const state = createState() + + createExpandCollapseActions(state).refreshPaneSizes(true) + + expect(state.pendingPaneSizeRefreshFrameIdsRef.current).toEqual([42]) + + cancelPendingPaneSizeRefreshFrames(state) + + expect(cancelAnimationFrame).toHaveBeenCalledWith(42) + expect(state.pendingPaneSizeRefreshFrameIdsRef.current).toEqual([]) + }) + + it('forgets completed pane-size refresh frames', () => { + const callbacks: FrameRequestCallback[] = [] + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((next: FrameRequestCallback) => { + callbacks.push(next) + return 7 + }) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + const state = createState({ + managerRef: ref({ + getPanes: () => [], + getActivePane: () => null + } as never) + }) + + createExpandCollapseActions(state).refreshPaneSizes(false) + + expect(state.pendingPaneSizeRefreshFrameIdsRef.current).toEqual([7]) + + const callback = callbacks[0] + if (!callback) { + throw new Error('expected pane-size refresh frame to be scheduled') + } + callback(16) + + expect(state.pendingPaneSizeRefreshFrameIdsRef.current).toEqual([]) + }) + + it('does not retain synchronously completed pane-size refresh frames', () => { + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(16) + return 9 + }) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + const state = createState() + + createExpandCollapseActions(state).refreshPaneSizes(false) + + expect(state.pendingPaneSizeRefreshFrameIdsRef.current).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/expand-collapse.ts b/src/renderer/src/components/terminal-pane/expand-collapse.ts index a8b499154f7..c2d7b6966a1 100644 --- a/src/renderer/src/components/terminal-pane/expand-collapse.ts +++ b/src/renderer/src/components/terminal-pane/expand-collapse.ts @@ -10,6 +10,7 @@ type ExpandCollapseState = { managerRef: React.RefObject setExpandedPaneId: (paneId: number | null) => void setTabPaneExpanded: (tabId: string, expanded: boolean) => void + pendingPaneSizeRefreshFrameIdsRef: React.MutableRefObject tabId: string persistLayoutSnapshot: () => void } @@ -81,6 +82,36 @@ export function applyExpandedLayoutTo( return true } +export function cancelPendingPaneSizeRefreshFrames( + state: Pick +): void { + for (const frameId of state.pendingPaneSizeRefreshFrameIdsRef.current) { + cancelAnimationFrame(frameId) + } + state.pendingPaneSizeRefreshFrameIdsRef.current = [] +} + +function requestPaneSizeRefreshFrame( + state: ExpandCollapseState, + callback: FrameRequestCallback +): void { + let completed = false + let frameId: number | undefined + frameId = requestAnimationFrame((timestamp) => { + completed = true + if (frameId !== undefined) { + state.pendingPaneSizeRefreshFrameIdsRef.current = + state.pendingPaneSizeRefreshFrameIdsRef.current.filter( + (pendingFrameId) => pendingFrameId !== frameId + ) + } + callback(timestamp) + }) + if (!completed) { + state.pendingPaneSizeRefreshFrameIdsRef.current.push(frameId) + } +} + export function createExpandCollapseActions(state: ExpandCollapseState) { const setExpandedPane = (paneId: number | null): void => { state.expandedPaneIdRef.current = paneId @@ -99,7 +130,7 @@ export function createExpandCollapseActions(state: ExpandCollapseState) { // safeFit owns scroll preservation; content matching here jumped to the // wrong duplicate scrollback line in long sessions. const refreshPaneSizes = (focusActive: boolean): void => { - requestAnimationFrame(() => { + requestPaneSizeRefreshFrame(state, () => { const manager = state.managerRef.current if (!manager) { return