From c7974783d14649f6b975c28a95e7b42bec40fc05 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 12:59:54 -0700 Subject: [PATCH] perf: skip hidden terminal sync fits Skip sidebar sync-fit work for hidden display:none terminal panes while preserving visible and measurable startup panes. --- .../components/terminal-pane/TerminalPane.tsx | 3 + .../use-terminal-container-fit-sync.ts | 12 ++- .../use-terminal-pane-global-effects.test.ts | 76 +++++++++++++++++++ .../use-terminal-pane-global-effects.ts | 4 +- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index bc1678af4..125bd8ee1 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -1047,6 +1047,9 @@ export default function TerminalPane({ cwd, isActive, isVisible, + // Why: hidden startup probes are opacity-hidden but measurable; ordinary + // hidden tabs are display:none and refit on visibility resume instead. + isSyncFitEnabled: isVisible || shouldMeasureHiddenStartup, paneCount, managerRef, containerRef, diff --git a/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts b/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts index b7a2fb963..070320276 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-container-fit-sync.ts @@ -5,12 +5,14 @@ import { fitPanes } from './pane-helpers' type UseTerminalContainerFitSyncArgs = { isVisible: boolean + isSyncFitEnabled: boolean managerRef: React.RefObject containerRef: React.RefObject } export function useTerminalContainerFitSync({ isVisible, + isSyncFitEnabled, managerRef, containerRef }: UseTerminalContainerFitSyncArgs): void { @@ -21,10 +23,12 @@ export function useTerminalContainerFitSync({ // ResizeObserver rAF would otherwise produce. The subsequent per-pane // ResizeObserver rAF and the 150ms debounced global fit become no-ops // because proposeDimensions() will match current cols/rows (early-return - // branch in safeFit). Listener is global (not gated on isVisible/isActive) - // so background tabs also fit, keeping their scroll position intact for - // when the user switches back. + // branch in safeFit). Hidden display:none panes cannot be measured + // accurately, so they skip this global path and refit on visibility resume. useEffect(() => { + if (!isSyncFitEnabled) { + return + } const onSyncFit = (): void => { managerRef.current?.fitAllPanes() } @@ -32,7 +36,7 @@ export function useTerminalContainerFitSync({ return () => { window.removeEventListener(SYNC_FIT_PANES_EVENT, onSyncFit) } - }, [managerRef]) + }, [isSyncFitEnabled, managerRef]) useEffect(() => { if (!isVisible) { 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 b60d030b5..d7f580d94 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 @@ -1,5 +1,7 @@ +/* eslint-disable max-lines -- Why: these hook tests share a mocked React lifecycle harness with global event cases. */ import type * as ReactModule from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { SYNC_FIT_PANES_EVENT } from '@/constants/terminal' import { useTerminalPaneGlobalEffects } from './use-terminal-pane-global-effects' const mocks = vi.hoisted(() => ({ @@ -82,6 +84,7 @@ function useMountForFileDrop( cwd?: string isActive?: boolean isVisible?: boolean + isSyncFitEnabled?: boolean paneCount?: number } = {} ): { @@ -116,6 +119,7 @@ function useMountForFileDrop( cwd: options.cwd, isActive: options.isActive ?? true, isVisible: options.isVisible ?? true, + isSyncFitEnabled: options.isSyncFitEnabled ?? options.isVisible ?? true, paneCount: options.paneCount ?? 0, managerRef: { current: manager as never }, containerRef: { current: null }, @@ -187,6 +191,7 @@ describe('useTerminalPaneGlobalEffects', () => { worktreeId: 'wt-1', isActive: true, isVisible: true, + isSyncFitEnabled: true, paneCount: 2, managerRef: { current: manager as never }, containerRef: { current: null }, @@ -244,6 +249,7 @@ describe('useTerminalPaneGlobalEffects', () => { isActiveRef: { current: false }, isVisibleRef: { current: false }, paneCount: 1, + isSyncFitEnabled: true, toggleExpandPane: vi.fn() } @@ -330,4 +336,74 @@ describe('useTerminalPaneGlobalEffects', () => { expect(mocks.handleTerminalFileDrop).not.toHaveBeenCalled() }) + + it('skips global sync-fit registration for hidden non-measurable terminal panes', () => { + const manager = { + getPanes: vi.fn(() => []), + resumeRendering: vi.fn(), + suspendRendering: vi.fn(), + fitAllPanes: vi.fn(), + getActivePane: vi.fn(() => null) + } + + beginHookRender() + useTerminalPaneGlobalEffects({ + tabId: 'tab-1', + worktreeId: 'wt-1', + isActive: false, + isVisible: false, + isSyncFitEnabled: false, + paneCount: 0, + managerRef: { current: manager as never }, + containerRef: { current: null }, + paneTransportsRef: { current: new Map() }, + isActiveRef: { current: false }, + isVisibleRef: { current: false }, + toggleExpandPane: vi.fn() + }) + + const syncFitListener = vi + .mocked(window.addEventListener) + .mock.calls.find(([eventName]) => eventName === SYNC_FIT_PANES_EVENT) + + expect(syncFitListener).toBeUndefined() + }) + + it('registers global sync-fit for measurable hidden startup panes', () => { + const manager = { + getPanes: vi.fn(() => []), + resumeRendering: vi.fn(), + suspendRendering: vi.fn(), + fitAllPanes: vi.fn(), + getActivePane: vi.fn(() => null) + } + + beginHookRender() + useTerminalPaneGlobalEffects({ + tabId: 'tab-1', + worktreeId: 'wt-1', + isActive: false, + isVisible: false, + isSyncFitEnabled: true, + paneCount: 0, + managerRef: { current: manager as never }, + containerRef: { current: null }, + paneTransportsRef: { current: new Map() }, + isActiveRef: { current: false }, + isVisibleRef: { current: false }, + toggleExpandPane: vi.fn() + }) + + const syncFitListener = vi + .mocked(window.addEventListener) + .mock.calls.find(([eventName]) => eventName === SYNC_FIT_PANES_EVENT) + + expect(syncFitListener).toBeDefined() + const listener = syncFitListener?.[1] + if (typeof listener !== 'function') { + throw new Error('expected sync-fit listener') + } + listener(new Event(SYNC_FIT_PANES_EVENT)) + expect(manager.fitAllPanes).toHaveBeenCalledTimes(1) + }) }) 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 d2bb906d9..8a35a9abd 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 @@ -30,6 +30,7 @@ type UseTerminalPaneGlobalEffectsArgs = { cwd?: string isActive: boolean isVisible: boolean + isSyncFitEnabled: boolean paneCount: number managerRef: React.RefObject containerRef: React.RefObject @@ -45,6 +46,7 @@ export function useTerminalPaneGlobalEffects({ cwd, isActive, isVisible, + isSyncFitEnabled, paneCount, managerRef, containerRef, @@ -73,7 +75,7 @@ export function useTerminalPaneGlobalEffects({ visibleResumeCompleteRef: wasVisibleRef, paneCount }) - useTerminalContainerFitSync({ isVisible, managerRef, containerRef }) + useTerminalContainerFitSync({ isVisible, isSyncFitEnabled, managerRef, containerRef }) useEffect(() => { const manager = managerRef.current