From cc89f2c22fdafbc7fc5c6bbe58c17297e0a0f0ee Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:11:20 -0700 Subject: [PATCH] Restore terminal scrollbar with an Orca-owned overlay xterm's own scrollbar stays disabled (width 0): a non-zero width makes FitAddon reserve a gutter that underfits wide table/TUI output by a column (#4877), and its render pipeline destabilizes deferred scroll restore after workspace switches. Instead, draw a slim overlay thumb from public xterm scroll state that is completely inert while hidden and reveals only on wheel/hover/drag. Co-authored-by: Orca --- src/renderer/src/assets/terminal.css | 39 ++- .../lib/pane-manager/pane-lifecycle.test.ts | 5 +- .../src/lib/pane-manager/pane-lifecycle.ts | 5 + .../lib/pane-manager/pane-manager-types.ts | 2 + .../pane-overlay-scrollbar.test.ts | 139 ++++++++++ .../pane-manager/pane-overlay-scrollbar.ts | 248 ++++++++++++++++++ .../lib/pane-manager/pane-terminal-options.ts | 8 +- 7 files changed, 440 insertions(+), 6 deletions(-) create mode 100644 src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.test.ts create mode 100644 src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.ts diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index 682e38d89cb..0953387abe4 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -42,10 +42,43 @@ background-color: transparent; } -/* Round the xterm DOM scrollbar thumb; xterm has no option for this, and its - default .xterm-slider has square corners. */ -.xterm .xterm-scrollable-element > .xterm-scrollbar > .xterm-slider { +/* Orca overlay scrollbar — xterm's own scrollbar stays disabled (it reserves + a FitAddon gutter and destabilizes restore scroll sync; see + pane-overlay-scrollbar.ts). The track is a 10px hover/hit strip that only + accepts pointer input while visible so the terminal's right edge never + dead-zones when the bar is faded out. */ +.pane-overlay-scrollbar { + position: absolute; + top: 0; + right: 0; + bottom: 0; + width: 10px; + z-index: 9; /* above terminal content, below drag handle (z:10) */ + opacity: 0; + pointer-events: none; + transition: opacity 150ms ease; +} + +.pane-overlay-scrollbar.is-visible { + opacity: 1; + pointer-events: auto; +} + +.pane-overlay-scrollbar-thumb { + position: absolute; + top: 0; + right: 1px; + width: 7px; border-radius: 4px; + background: rgba(180, 180, 185, 0.4); +} + +.pane-overlay-scrollbar-thumb:hover { + background: rgba(180, 180, 185, 0.6); +} + +.pane-overlay-scrollbar.is-dragging .pane-overlay-scrollbar-thumb { + background: rgba(180, 180, 185, 0.8); } /* xterm.css sets position:absolute;top:0 on .xterm-helpers but omits left, diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index af6fa0c2b30..f5e256b6ed1 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -76,7 +76,10 @@ describe('buildDefaultTerminalOptions', () => { expect(buildDefaultTerminalOptions().cursorInactiveStyle).toBe('outline') }) - it('does not reserve a classic scrollbar gutter when fitting terminal columns', () => { + it('keeps the xterm scrollbar disabled in favor of the Orca overlay scrollbar', () => { + // Why: a non-zero width makes FitAddon reserve a column-eating gutter + // (#4877) and destabilizes restore scroll sync; the visible scrollbar is + // Orca's own overlay (pane-overlay-scrollbar.ts). expect(buildDefaultTerminalOptions().scrollbar?.width).toBe(0) }) diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index f24043f2d8c..64ec5560fa8 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -26,6 +26,7 @@ import { clearPendingSplitScrollRestore } from './pane-split-scroll' import { buildDefaultTerminalOptions } from './pane-terminal-options' import { activateOrcaTerminalUnicodeProvider } from './pane-terminal-unicode-provider' import { attachDomRendererFocusClassSync } from './pane-dom-focus-class-sync' +import { attachPaneOverlayScrollbar } from './pane-overlay-scrollbar' import { ENABLE_WEBGL_RENDERER, attachWebgl, @@ -151,6 +152,7 @@ export function createPaneDOM( paneDragCleanup, compositionHandler: null, focusClassSyncCleanup: null, + overlayScrollbarCleanup: null, pendingSplitScrollState: null, pendingSplitScrollRafIds: [], pendingSplitScrollTimerId: null, @@ -244,6 +246,7 @@ export function openTerminal(pane: ManagedPaneInternal): void { } pane.focusClassSyncCleanup = attachDomRendererFocusClassSync(terminal.element) + pane.overlayScrollbarCleanup = attachPaneOverlayScrollbar(terminal, xtermContainer) if (pane.gpuRenderingEnabled) { attachWebgl(pane) @@ -334,6 +337,8 @@ export function disposePane( pane.paneDragCleanup = null pane.focusClassSyncCleanup?.() pane.focusClassSyncCleanup = null + pane.overlayScrollbarCleanup?.() + pane.overlayScrollbarCleanup = null if (pane.compositionHandler) { pane.terminal.element?.removeEventListener('compositionstart', pane.compositionHandler, true) pane.compositionHandler = null diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index 6213b32003b..604754446b1 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -133,6 +133,8 @@ export type ManagedPaneInternal = { compositionHandler: (() => void) | null // Stored so disposePane() can remove DOM-renderer focus synchronization. focusClassSyncCleanup?: (() => void) | null + // Stored so disposePane() can tear down the Orca overlay scrollbar. + overlayScrollbarCleanup?: (() => void) | null // Why: splitPane reparents DOM; its delayed restore owns scroll until the // browser settles, so intermediate fits must not compete with it. pendingSplitScrollState: ScrollState | null diff --git a/src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.test.ts b/src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.test.ts new file mode 100644 index 00000000000..2d6e5559492 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.test.ts @@ -0,0 +1,139 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from 'vitest' +import type { Terminal } from '@xterm/xterm' +import { attachPaneOverlayScrollbar } from './pane-overlay-scrollbar' + +type ScrollListener = () => void + +function createFakeTerminal({ + baseY = 100, + viewportY = 100 +}: { baseY?: number; viewportY?: number } = {}): { + terminal: Terminal + emitScroll: () => void + scrollLines: ReturnType + scrollToLine: ReturnType + buffer: { baseY: number; viewportY: number; type: string } +} { + const scrollListeners: ScrollListener[] = [] + const buffer = { baseY, viewportY, type: 'normal' } + const scrollLines = vi.fn((delta: number) => { + buffer.viewportY = Math.max(0, Math.min(buffer.baseY, buffer.viewportY + delta)) + }) + const scrollToLine = vi.fn((line: number) => { + buffer.viewportY = Math.max(0, Math.min(buffer.baseY, line)) + }) + const subscribe = (listeners: ScrollListener[]) => (listener: ScrollListener) => { + listeners.push(listener) + return { dispose: () => listeners.splice(listeners.indexOf(listener), 1) } + } + const terminal = { + rows: 24, + buffer: { active: buffer }, + onScroll: subscribe(scrollListeners), + onWriteParsed: subscribe([]), + onResize: subscribe([]), + scrollLines, + scrollToLine + } as unknown as Terminal + return { + terminal, + emitScroll: () => scrollListeners.forEach((listener) => listener()), + scrollLines, + scrollToLine, + buffer + } +} + +function stubHeights(host: HTMLElement): { track: HTMLElement; thumb: HTMLElement } { + const track = host.querySelector('.pane-overlay-scrollbar')! + const thumb = host.querySelector('.pane-overlay-scrollbar-thumb')! + Object.defineProperty(track, 'clientHeight', { value: 400, configurable: true }) + Object.defineProperty(thumb, 'clientHeight', { value: 80, configurable: true }) + track.getBoundingClientRect = () => + ({ top: 0, height: 400, left: 0, width: 10, right: 10, bottom: 400, x: 0, y: 0 }) as DOMRect + return { track, thumb } +} + +describe('attachPaneOverlayScrollbar', () => { + it('mounts the track in the host and removes it on cleanup', () => { + const host = document.createElement('div') + const { terminal } = createFakeTerminal() + const cleanup = attachPaneOverlayScrollbar(terminal, host) + + expect(host.querySelector('.pane-overlay-scrollbar-thumb')).not.toBeNull() + cleanup() + expect(host.querySelector('.pane-overlay-scrollbar')).toBeNull() + }) + + it('reveals on wheel input and fades after the idle delay', () => { + vi.useFakeTimers() + try { + const host = document.createElement('div') + const { terminal } = createFakeTerminal() + const cleanup = attachPaneOverlayScrollbar(terminal, host) + const { track } = stubHeights(host) + + host.dispatchEvent(new WheelEvent('wheel', { deltaY: -120 })) + expect(track.classList.contains('is-visible')).toBe(true) + + vi.advanceTimersByTime(1500) + expect(track.classList.contains('is-visible')).toBe(false) + cleanup() + } finally { + vi.useRealTimers() + } + }) + + it('stays inert on programmatic scroll events while hidden', () => { + // Why: workspace-switch restores fire scroll storms; the hidden overlay + // must not add per-event work or reveal itself during them. + const host = document.createElement('div') + const { terminal, emitScroll } = createFakeTerminal() + const cleanup = attachPaneOverlayScrollbar(terminal, host) + const { track } = stubHeights(host) + + emitScroll() + expect(track.classList.contains('is-visible')).toBe(false) + cleanup() + }) + + it('does not reveal when there is no scrollback', () => { + const host = document.createElement('div') + const { terminal } = createFakeTerminal({ baseY: 0, viewportY: 0 }) + const cleanup = attachPaneOverlayScrollbar(terminal, host) + const { track } = stubHeights(host) + + host.dispatchEvent(new WheelEvent('wheel', { deltaY: -120 })) + expect(track.classList.contains('is-visible')).toBe(false) + cleanup() + }) + + it('scrolls the terminal proportionally while dragging the thumb', () => { + const host = document.createElement('div') + const { terminal, scrollLines, buffer } = createFakeTerminal({ baseY: 100, viewportY: 0 }) + const cleanup = attachPaneOverlayScrollbar(terminal, host) + const { thumb } = stubHeights(host) + + thumb.dispatchEvent(new PointerEvent('pointerdown', { button: 0, clientY: 0, bubbles: true })) + // 320px of scrollable track maps to 100 lines of scrollback; 160px = 50 lines. + thumb.dispatchEvent(new PointerEvent('pointermove', { clientY: 160, bubbles: true })) + + expect(scrollLines).toHaveBeenCalledWith(50) + expect(buffer.viewportY).toBe(50) + cleanup() + }) + + it('jump-scrolls when clicking the track outside the thumb', () => { + const host = document.createElement('div') + const { terminal, scrollToLine } = createFakeTerminal({ baseY: 100, viewportY: 100 }) + const cleanup = attachPaneOverlayScrollbar(terminal, host) + const { track } = stubHeights(host) + + track.dispatchEvent(new PointerEvent('pointerdown', { button: 0, clientY: 200, bubbles: true })) + + // Click at the track midpoint: half of 124 total lines, centered on the viewport. + expect(scrollToLine).toHaveBeenCalledWith(50) + cleanup() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.ts b/src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.ts new file mode 100644 index 00000000000..63855549651 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-overlay-scrollbar.ts @@ -0,0 +1,248 @@ +import type { Terminal } from '@xterm/xterm' + +// Why: xterm's built-in scrollbar stays disabled (see pane-terminal-options.ts) — +// it makes FitAddon reserve a column-eating gutter and destabilizes scroll +// restore after workspace switches. This overlay only *reads* xterm scroll +// state and renders its own thumb, so it cannot feed back into xterm's +// viewport sync; dragging goes through the public scrollLines/scrollToLine API +// like any user scroll. + +const HIDE_DELAY_MS = 1200 +const MIN_THUMB_HEIGHT_PX = 20 + +export function attachPaneOverlayScrollbar(terminal: Terminal, host: HTMLElement): () => void { + if (typeof document === 'undefined') { + return () => {} + } + const track = document.createElement('div') + track.className = 'pane-overlay-scrollbar' + const thumb = document.createElement('div') + thumb.className = 'pane-overlay-scrollbar-thumb' + track.appendChild(thumb) + host.appendChild(track) + + let rafId: number | null = null + let hideTimer: ReturnType | null = null + let hovering = false + let dragging = false + let disposed = false + + // Why: reading clientHeight forces synchronous layout, and the thumb relays + // out on every scroll event during PTY output storms. Cache the height and + // let the observer refresh it so scroll-driven updates stay layout-free. + let trackHeight = track.clientHeight + const trackResizeObserver = + typeof ResizeObserver === 'function' + ? new ResizeObserver(() => { + trackHeight = track.clientHeight + scheduleLayout() + }) + : null + trackResizeObserver?.observe(track) + + function totalLines(): number { + return terminal.buffer.active.baseY + terminal.rows + } + + function hasScrollback(): boolean { + return terminal.buffer.active.type === 'normal' && terminal.buffer.active.baseY > 0 + } + + function layoutThumb(): void { + if (!hasScrollback() || trackHeight <= 0) { + track.classList.remove('is-visible') + return + } + const total = totalLines() + const thumbHeight = Math.max( + MIN_THUMB_HEIGHT_PX, + Math.round((terminal.rows / total) * trackHeight) + ) + const maxTop = trackHeight - thumbHeight + const scrollFrac = terminal.buffer.active.viewportY / terminal.buffer.active.baseY + thumb.style.height = `${thumbHeight}px` + thumb.style.transform = `translateY(${Math.round(maxTop * scrollFrac)}px)` + } + + function scheduleLayout(): void { + if (rafId !== null) { + return + } + rafId = requestAnimationFrame(() => { + rafId = null + layoutThumb() + }) + } + + function scheduleHide(): void { + if (hideTimer !== null) { + clearTimeout(hideTimer) + } + hideTimer = setTimeout(() => { + hideTimer = null + if (!hovering && !dragging) { + track.classList.remove('is-visible') + } + }, HIDE_DELAY_MS) + } + + function isVisible(): boolean { + return track.classList.contains('is-visible') + } + + function reveal(): void { + if (disposed || !hasScrollback()) { + return + } + if (!isVisible()) { + // One layout read per reveal, not per scroll event while visible. + trackHeight = track.clientHeight + track.classList.add('is-visible') + } + scheduleLayout() + scheduleHide() + } + + // Why: while hidden the overlay must be completely inert. Workspace-switch + // restores fire scroll/write storms, and any per-event work in that window + // can starve xterm's deferred scroll-restore (the instability that got the + // previous xterm scrollbar removed). Reveal only on direct user input. + const onScroll = terminal.onScroll(() => { + if (isVisible()) { + scheduleLayout() + scheduleHide() + } + }) + // Output growth shifts the thumb even when the viewport line is unchanged. + const onWrite = terminal.onWriteParsed(() => { + if (isVisible()) { + scheduleLayout() + } + }) + const onResize = terminal.onResize(() => { + if (isVisible()) { + scheduleLayout() + } + }) + + // Capture phase: xterm's scrollable element stops propagation of consumed + // wheel events, so a bubble listener on the host would never fire. + const onHostWheel = (): void => reveal() + + function lineAtTrackDelta(deltaPx: number, startLine: number): number { + const scrollableHeight = track.clientHeight - thumb.clientHeight + if (scrollableHeight <= 0) { + return startLine + } + const lineDelta = Math.round((deltaPx / scrollableHeight) * terminal.buffer.active.baseY) + return Math.max(0, Math.min(terminal.buffer.active.baseY, startLine + lineDelta)) + } + + let dragStartY = 0 + let dragStartLine = 0 + + const onThumbPointerDown = (event: PointerEvent): void => { + if (event.button !== 0) { + return + } + event.preventDefault() + event.stopPropagation() + dragging = true + dragStartY = event.clientY + dragStartLine = terminal.buffer.active.viewportY + try { + thumb.setPointerCapture(event.pointerId) + } catch { + // Why: capture is best-effort; synthetic events carry no live pointerId. + } + track.classList.add('is-dragging') + } + + const onThumbPointerMove = (event: PointerEvent): void => { + if (!dragging) { + return + } + const target = lineAtTrackDelta(event.clientY - dragStartY, dragStartLine) + terminal.scrollLines(target - terminal.buffer.active.viewportY) + } + + const endThumbDrag = (): void => { + if (!dragging) { + return + } + dragging = false + track.classList.remove('is-dragging') + scheduleHide() + } + + // Jump-scroll when clicking the track outside the thumb. + const onTrackPointerDown = (event: PointerEvent): void => { + if (event.button !== 0 || event.target === thumb || !hasScrollback()) { + return + } + event.preventDefault() + const rect = track.getBoundingClientRect() + const frac = rect.height > 0 ? (event.clientY - rect.top) / rect.height : 0 + const target = Math.max( + 0, + Math.min(terminal.buffer.active.baseY, Math.round(frac * totalLines() - terminal.rows / 2)) + ) + terminal.scrollToLine(target) + } + + // Forward wheel input so the visible track doesn't dead-zone the right edge. + const onTrackWheel = (event: WheelEvent): void => { + event.preventDefault() + const cellHeight = host.clientHeight / Math.max(1, terminal.rows) + const lines = Math.round(event.deltaY / Math.max(1, cellHeight)) + if (lines !== 0) { + terminal.scrollLines(lines) + } + } + + const onTrackPointerEnter = (): void => { + hovering = true + reveal() + } + + const onTrackPointerLeave = (): void => { + hovering = false + scheduleHide() + } + + host.addEventListener('wheel', onHostWheel, { capture: true, passive: true }) + thumb.addEventListener('pointerdown', onThumbPointerDown) + thumb.addEventListener('pointermove', onThumbPointerMove) + thumb.addEventListener('pointerup', endThumbDrag) + thumb.addEventListener('pointercancel', endThumbDrag) + track.addEventListener('pointerdown', onTrackPointerDown) + track.addEventListener('wheel', onTrackWheel, { passive: false }) + track.addEventListener('pointerenter', onTrackPointerEnter) + track.addEventListener('pointerleave', onTrackPointerLeave) + + return () => { + disposed = true + onScroll.dispose() + onWrite.dispose() + onResize.dispose() + trackResizeObserver?.disconnect() + if (rafId !== null) { + cancelAnimationFrame(rafId) + rafId = null + } + if (hideTimer !== null) { + clearTimeout(hideTimer) + hideTimer = null + } + host.removeEventListener('wheel', onHostWheel, { capture: true }) + thumb.removeEventListener('pointerdown', onThumbPointerDown) + thumb.removeEventListener('pointermove', onThumbPointerMove) + thumb.removeEventListener('pointerup', endThumbDrag) + thumb.removeEventListener('pointercancel', endThumbDrag) + track.removeEventListener('pointerdown', onTrackPointerDown) + track.removeEventListener('wheel', onTrackWheel) + track.removeEventListener('pointerenter', onTrackPointerEnter) + track.removeEventListener('pointerleave', onTrackPointerLeave) + track.remove() + } +} diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-options.ts b/src/renderer/src/lib/pane-manager/pane-terminal-options.ts index 8d44391dbf9..2b8820891b8 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-options.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-options.ts @@ -32,8 +32,12 @@ export function buildDefaultTerminalOptions(): ITerminalOptions { macOptionClickForcesSelection: true, drawBoldTextInBrightColors: true, scrollbar: { - // Why: xterm's DOM scrollbar overlays the terminal in Electron; reserving - // gutter width in FitAddon underfits wide table/TUI output by a column. + // Why: xterm's own scrollbar must stay disabled. A non-zero width makes + // FitAddon reserve a gutter (underfits wide table/TUI output by a + // column, #4877) and its render pipeline destabilizes scroll restore + // after workspace switches (terminal-long-table-scroll-restore e2e). + // Orca draws its own overlay scrollbar instead — see + // pane-overlay-scrollbar.ts. width: 0 }, // Why: advertise kitty keyboard protocol support so CLIs that probe