From 18941f7ca9afb7a35c9dfeece508fa4767dc006a Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 24 Jun 2026 19:37:08 -0700 Subject: [PATCH] Fix pinned terminal scroll intent across remounts Co-authored-by: Orca --- .../use-terminal-pane-lifecycle.ts | 2 +- ...-terminal-scroll-visibility-memory.test.ts | 46 ++++++++++- .../use-terminal-scroll-visibility-memory.ts | 15 ++-- .../src/lib/pane-manager/pane-lifecycle.ts | 6 +- .../terminal-scroll-intent.test.ts | 76 +++++++++++++++++- .../pane-manager/terminal-scroll-intent.ts | 78 +++++++++++++++++-- 6 files changed, 205 insertions(+), 18 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 5854cebe27a..f26d9adc198 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -727,7 +727,7 @@ export function useTerminalPaneLifecycle({ if (e.type === 'keydown') { if (e.key === 'PageUp' || e.key === 'Home') { markTerminalPinnedViewport(pane.terminal) - syncTerminalScrollIntentSoon(pane.terminal) + syncTerminalScrollIntentSoon(pane.terminal, { preservePinnedAtBottom: true }) } else if (e.key === 'PageDown' || e.key === 'End') { syncTerminalScrollIntentSoon(pane.terminal) } diff --git a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts index c30d96a5387..3cd22968f4b 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.test.ts @@ -11,7 +11,9 @@ const mocks = vi.hoisted(() => ({ baseY: 0 })), flushTerminalOutput: vi.fn(), - getTerminalOutputEpoch: vi.fn(() => 1) + getTerminalOutputEpoch: vi.fn(() => 1), + getTerminalScrollIntentKind: vi.fn(() => 'followOutput'), + markTerminalFollowOutput: vi.fn() })) const reactRefState = vi.hoisted(() => ({ @@ -68,6 +70,11 @@ vi.mock('@/lib/pane-manager/pane-scroll', () => ({ getTerminalOutputEpoch: mocks.getTerminalOutputEpoch })) +vi.mock('@/lib/pane-manager/terminal-scroll-intent', () => ({ + getTerminalScrollIntentKind: mocks.getTerminalScrollIntentKind, + markTerminalFollowOutput: mocks.markTerminalFollowOutput +})) + describe('useTerminalScrollVisibilityMemory', () => { const originalRequestAnimationFrame = globalThis.requestAnimationFrame const originalCancelAnimationFrame = globalThis.cancelAnimationFrame @@ -75,6 +82,7 @@ describe('useTerminalScrollVisibilityMemory', () => { beforeEach(() => { resetHookRefs() vi.clearAllMocks() + mocks.getTerminalScrollIntentKind.mockReturnValue('followOutput') }) afterEach(() => { @@ -121,6 +129,42 @@ describe('useTerminalScrollVisibilityMemory', () => { maxChars: 256 * 1024 }) expect(terminal.scrollToBottom).toHaveBeenCalled() + expect(mocks.markTerminalFollowOutput).toHaveBeenCalledWith(terminal) + }) + + it('does not turn a pinned viewport into follow-output when pending focus requests catch up', () => { + mocks.getTerminalScrollIntentKind.mockReturnValue('pinnedViewport') + const terminal = { + onScroll: vi.fn(() => ({ dispose: vi.fn() })), + scrollToBottom: vi.fn() + } + const manager = { + getPanes: vi.fn(() => [{ id: 1, terminal }]) + } + const animationFrames: FrameRequestCallback[] = [] + globalThis.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => { + animationFrames.push(callback) + return animationFrames.length + }) + + beginHookRender() + const visibilityMemory = useTerminalScrollVisibilityMemory({ + managerRef: { current: manager as never }, + isVisibleRef: { current: true }, + visibleResumeCompleteRef: { current: true }, + paneCount: 1 + }) + + visibilityMemory.scheduleFollowOutputIfNeeded(1) + animationFrames.shift()?.(16) + animationFrames.shift()?.(32) + + expect(mocks.flushTerminalOutput).toHaveBeenCalledWith(terminal, { + maxChars: 256 * 1024 + }) + expect(mocks.cancelDeferredScrollRestore).not.toHaveBeenCalled() + expect(mocks.markTerminalFollowOutput).not.toHaveBeenCalled() + expect(terminal.scrollToBottom).not.toHaveBeenCalled() }) it('cancels pending follow-output frames on cleanup', () => { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts index 987842ee843..05afb66d5d4 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-scroll-visibility-memory.ts @@ -6,7 +6,10 @@ import { captureScrollState, getTerminalOutputEpoch } from '@/lib/pane-manager/pane-scroll' -import { markTerminalFollowOutput } from '@/lib/pane-manager/terminal-scroll-intent' +import { + getTerminalScrollIntentKind, + markTerminalFollowOutput +} from '@/lib/pane-manager/terminal-scroll-intent' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { ScrollState } from '@/lib/pane-manager/pane-manager-types' @@ -117,11 +120,13 @@ export function useTerminalScrollVisibilityMemory({ const currentEpoch = getTerminalOutputEpoch(pane.terminal) const hasNewOutput = previous ? currentEpoch > previous.outputEpoch : currentEpoch > 0 if (hasNewOutput) { - cancelDeferredScrollRestore(pane.terminal) - markTerminalFollowOutput(pane.terminal) - pane.terminal.scrollToBottom() + if (getTerminalScrollIntentKind(pane.terminal) === 'followOutput') { + cancelDeferredScrollRestore(pane.terminal) + markTerminalFollowOutput(pane.terminal) + pane.terminal.scrollToBottom() + didScroll = true + } rememberVisibleScrollSnapshot(pane.id, pane.terminal) - didScroll = true } pending.delete(pane.id) } diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index 62dfa38ac9c..eb296a1d675 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -52,7 +52,11 @@ export function openTerminal(pane: ManagedPaneInternal): void { attachTerminalMouseWheelMultiplier(terminal, { getTuiMouseWheelMultiplier: terminalTuiScrollSensitivity }) - pane.terminalScrollIntentDisposable = attachTerminalScrollIntentTracking(terminal, xtermContainer) + pane.terminalScrollIntentDisposable = attachTerminalScrollIntentTracking( + terminal, + xtermContainer, + pane.leafId + ) // Activate Orca's Unicode 11 width shim *before* any caller-driven write. CJK / emoji / // ZWJ codepoints get baked into the buffer at the active unicode version on diff --git a/src/renderer/src/lib/pane-manager/terminal-scroll-intent.test.ts b/src/renderer/src/lib/pane-manager/terminal-scroll-intent.test.ts index 898a883905c..f132e786daf 100644 --- a/src/renderer/src/lib/pane-manager/terminal-scroll-intent.test.ts +++ b/src/renderer/src/lib/pane-manager/terminal-scroll-intent.test.ts @@ -80,6 +80,7 @@ class TestElement extends EventTarget { describe('terminal scroll intent', () => { afterEach(() => { + vi.useRealTimers() vi.unstubAllGlobals() }) @@ -165,19 +166,90 @@ describe('terminal scroll intent', () => { terminal.buffer.active.viewportY = 0 enforceTerminalCurrentScrollIntent(terminal) expect(terminal.scrollToLine).toHaveBeenLastCalledWith(80) + disposable.dispose() + }) + + it('returns to followOutput after a downward wheel settles at the bottom', async () => { + const frameCallbacks: FrameRequestCallback[] = [] + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + frameCallbacks.push(callback) + return frameCallbacks.length + }) + vi.stubGlobal('Element', TestElement) + const terminal = createTerminal({ viewportY: 50, baseY: 100 }) + const host = new TestElement() as unknown as HTMLElement + const disposable = attachTerminalScrollIntentTracking(terminal, host) - terminal.buffer.active.viewportY = 100 const wheelDown = new Event('wheel') as WheelEvent Object.defineProperty(wheelDown, 'deltaY', { value: 10 }) host.dispatchEvent(wheelDown) expect(getTerminalScrollIntentKind(terminal)).toBe('pinnedViewport') - frameCallbacks[0]?.(16) + terminal.buffer.active.viewportY = 100 + await Promise.resolve() expect(getTerminalScrollIntentKind(terminal)).toBe('followOutput') disposable.dispose() }) + it('keeps sampling briefly after wheel so delayed xterm scroll updates win', async () => { + const frameCallbacks: FrameRequestCallback[] = [] + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + frameCallbacks.push(callback) + return frameCallbacks.length + }) + vi.useFakeTimers() + vi.stubGlobal('Element', TestElement) + const terminal = createTerminal({ viewportY: 100, baseY: 100 }) + const host = new TestElement() as unknown as HTMLElement + const disposable = attachTerminalScrollIntentTracking(terminal, host) + + const wheelUp = new Event('wheel') as WheelEvent + Object.defineProperty(wheelUp, 'deltaY', { value: -10 }) + host.dispatchEvent(wheelUp) + expect(getTerminalScrollIntentKind(terminal)).toBe('pinnedViewport') + + await Promise.resolve() + frameCallbacks.shift()?.(16) + expect(getTerminalScrollIntentKind(terminal)).toBe('pinnedViewport') + + terminal.buffer.active.viewportY = 76 + frameCallbacks.shift()?.(32) + frameCallbacks.shift()?.(48) + terminal.buffer.active.viewportY = 100 + enforceTerminalCurrentScrollIntent(terminal) + expect(terminal.scrollToLine).toHaveBeenLastCalledWith(76) + + disposable.dispose() + }) + + it('keeps a pane-keyed pinned viewport across a remounted empty terminal', () => { + vi.stubGlobal('Element', TestElement) + const firstTerminal = createTerminal({ viewportY: 76, baseY: 100 }) + const firstHost = new TestElement() as unknown as HTMLElement + const firstDisposable = attachTerminalScrollIntentTracking(firstTerminal, firstHost, 'leaf-1') + markTerminalPinnedViewport(firstTerminal) + + const remountedTerminal = createTerminal({ viewportY: 0, baseY: 0 }) + const remountedHost = new TestElement() as unknown as HTMLElement + const remountedDisposable = attachTerminalScrollIntentTracking( + remountedTerminal, + remountedHost, + 'leaf-1' + ) + + syncTerminalScrollIntentFromViewport(remountedTerminal) + remountedTerminal.buffer.active.baseY = 100 + remountedTerminal.buffer.active.viewportY = 100 + enforceTerminalCurrentScrollIntent(remountedTerminal) + + expect(remountedTerminal.scrollToLine).toHaveBeenCalledWith(76) + expect(getTerminalScrollIntentKind(remountedTerminal)).toBe('pinnedViewport') + + firstDisposable.dispose() + remountedDisposable.dispose() + }) + it('tracks pointer-driven scrollbar scrolls without using output scroll as intent', () => { vi.stubGlobal('Element', TestElement) const terminal = createTerminal({ viewportY: 100, baseY: 100 }) diff --git a/src/renderer/src/lib/pane-manager/terminal-scroll-intent.ts b/src/renderer/src/lib/pane-manager/terminal-scroll-intent.ts index a8bea3ac9f6..35537c11f24 100644 --- a/src/renderer/src/lib/pane-manager/terminal-scroll-intent.ts +++ b/src/renderer/src/lib/pane-manager/terminal-scroll-intent.ts @@ -16,6 +16,8 @@ type TerminalScrollIntentTarget = { scrollToLine?: (line: number) => void } +type TerminalScrollIntentKey = string + type TerminalScrollIntent = { kind: TerminalScrollIntentKind bufferType: BufferType @@ -33,6 +35,11 @@ const terminalScrollIntentByTerminal = new WeakMap< TerminalScrollIntentTarget, TerminalScrollIntent >() +const terminalScrollIntentKeyByTerminal = new WeakMap< + TerminalScrollIntentTarget, + TerminalScrollIntentKey +>() +const terminalScrollIntentByKey = new Map() const BOTTOM_TOLERANCE_ROWS = 1 @@ -66,9 +73,37 @@ function writeIntent( } const intent = { kind, ...snapshot } terminalScrollIntentByTerminal.set(terminal, intent) + const key = terminalScrollIntentKeyByTerminal.get(terminal) + if (key) { + terminalScrollIntentByKey.set(key, intent) + } return intent } +function readStoredIntent(terminal: TerminalScrollIntentTarget): TerminalScrollIntent | undefined { + const terminalIntent = terminalScrollIntentByTerminal.get(terminal) + if (terminalIntent) { + return terminalIntent + } + const key = terminalScrollIntentKeyByTerminal.get(terminal) + return key ? terminalScrollIntentByKey.get(key) : undefined +} + +function bindTerminalScrollIntentKey( + terminal: TerminalScrollIntentTarget, + key: TerminalScrollIntentKey | undefined +): TerminalScrollIntent | undefined { + if (!key) { + return terminalScrollIntentByTerminal.get(terminal) + } + terminalScrollIntentKeyByTerminal.set(terminal, key) + const existing = terminalScrollIntentByKey.get(key) + if (existing) { + terminalScrollIntentByTerminal.set(terminal, existing) + } + return existing +} + function clampViewportY(viewportY: number, baseY: number): number { return Math.max(0, Math.min(viewportY, baseY)) } @@ -93,27 +128,49 @@ export function markTerminalPinnedViewport(terminal: TerminalScrollIntentTarget) writeIntent(terminal, 'pinnedViewport') } -export function syncTerminalScrollIntentFromViewport(terminal: TerminalScrollIntentTarget): void { +export function syncTerminalScrollIntentFromViewport( + terminal: TerminalScrollIntentTarget, + options: { preservePinnedAtBottom?: boolean } = {} +): void { const snapshot = readBufferSnapshot(terminal) if (!snapshot) { return } + const existing = readStoredIntent(terminal) + // Why: a remounted/replayed terminal can briefly report an empty or shorter + // scrollback. That transient state must not erase a durable pinned viewport. + if (existing?.kind === 'pinnedViewport' && snapshot.baseY < existing.baseY) { + terminalScrollIntentByTerminal.set(terminal, existing) + return + } + if ( + options.preservePinnedAtBottom && + existing?.kind === 'pinnedViewport' && + isAtBottom(snapshot.viewportY, snapshot.baseY) + ) { + return + } writeIntent( terminal, isAtBottom(snapshot.viewportY, snapshot.baseY) ? 'followOutput' : 'pinnedViewport' ) } -export function syncTerminalScrollIntentSoon(terminal: TerminalScrollIntentTarget): void { - const sync = (): void => syncTerminalScrollIntentFromViewport(terminal) +export function syncTerminalScrollIntentSoon( + terminal: TerminalScrollIntentTarget, + options: { preservePinnedAtBottom?: boolean } = {} +): void { + const sync = (): void => syncTerminalScrollIntentFromViewport(terminal, options) queueMicrotask(sync) requestAnimationFrame(sync) + requestAnimationFrame(() => requestAnimationFrame(sync)) + setTimeout(sync, 80) } export function getTerminalScrollIntentKind( terminal: TerminalScrollIntentTarget ): TerminalScrollIntentKind { - const existing = terminalScrollIntentByTerminal.get(terminal) + const existing = readStoredIntent(terminal) if (existing) { return existing.kind } @@ -131,7 +188,7 @@ export function captureTerminalWriteScrollIntent( if (!snapshot) { return null } - const existing = terminalScrollIntentByTerminal.get(terminal) + const existing = readStoredIntent(terminal) const kind = existing?.kind ?? (isAtBottom(snapshot.viewportY, snapshot.baseY) ? 'followOutput' : 'pinnedViewport') @@ -167,7 +224,7 @@ export function enforceTerminalWriteScrollIntent( } export function enforceTerminalCurrentScrollIntent(terminal: TerminalScrollIntentTarget): void { - const existing = terminalScrollIntentByTerminal.get(terminal) + const existing = readStoredIntent(terminal) const snapshot = existing ? { kind: existing.kind, @@ -180,14 +237,19 @@ export function enforceTerminalCurrentScrollIntent(terminal: TerminalScrollInten export function attachTerminalScrollIntentTracking( terminal: TerminalScrollIntentTarget, - host: HTMLElement + host: HTMLElement, + intentKey?: TerminalScrollIntentKey ): IDisposable { - syncTerminalScrollIntentFromViewport(terminal) + if (!bindTerminalScrollIntentKey(terminal, intentKey)) { + syncTerminalScrollIntentFromViewport(terminal) + } let pointerScrollActive = false const onWheel = (event: WheelEvent): void => { if (event.deltaY < 0) { markTerminalPinnedViewport(terminal) + syncTerminalScrollIntentSoon(terminal, { preservePinnedAtBottom: true }) + return } syncTerminalScrollIntentSoon(terminal) }