fix(terminal): preserve scroll position during split pane drag resize (#865)

This commit is contained in:
Jinwoo Hong 2026-04-20 21:53:20 -04:00 committed by GitHub
parent 3a0fa521be
commit 25971cd32d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 192 additions and 98 deletions

View file

@ -16,6 +16,8 @@ export function createDivider(
styleOptions: PaneStyleOptions,
callbacks: {
refitPanesUnder: (el: HTMLElement) => void
lockDragScroll: (el: HTMLElement) => void
unlockDragScroll: (el: HTMLElement) => void
onLayoutChanged?: () => void
}
): HTMLElement {
@ -45,6 +47,8 @@ function attachDividerDrag(
isVertical: boolean,
callbacks: {
refitPanesUnder: (el: HTMLElement) => void
lockDragScroll: (el: HTMLElement) => void
unlockDragScroll: (el: HTMLElement) => void
onLayoutChanged?: () => void
}
): void {
@ -85,6 +89,9 @@ function attachDividerDrag(
// Store current proportions as flex-basis values
prevFlex = prevSize
nextFlex = nextSize
callbacks.lockDragScroll(prevEl)
callbacks.lockDragScroll(nextEl)
}
// Why: fitAddon.fit() triggers a full xterm.js reflow which can take
@ -141,12 +148,15 @@ function attachDividerDrag(
}
divider.releasePointerCapture(e.pointerId)
divider.classList.remove('is-dragging')
// Final refit at the exact drop position
// Final refit at the exact drop position, then unlock drag scroll state
// so the authoritative restore uses the original pre-drag scroll position
if (prevEl) {
callbacks.refitPanesUnder(prevEl)
callbacks.unlockDragScroll(prevEl)
}
if (nextEl) {
callbacks.refitPanesUnder(nextEl)
callbacks.unlockDragScroll(nextEl)
}
prevEl = null
nextEl = null
@ -165,11 +175,16 @@ function attachDividerDrag(
return
}
callbacks.lockDragScroll(prev)
callbacks.lockDragScroll(next)
prev.style.flex = '1 1 0%'
next.style.flex = '1 1 0%'
callbacks.refitPanesUnder(prev)
callbacks.unlockDragScroll(prev)
callbacks.refitPanesUnder(next)
callbacks.unlockDragScroll(next)
callbacks.onLayoutChanged?.()
}

View file

@ -21,6 +21,8 @@ export type DragReorderCallbacks = {
applyPaneOpacity: () => void
applyDividerStyles: () => void
refitPanesUnder: (el: HTMLElement) => void
lockDragScroll: (el: HTMLElement) => void
unlockDragScroll: (el: HTMLElement) => void
onLayoutChanged?: () => void
}

View file

@ -0,0 +1,49 @@
import type { ManagedPaneInternal } from './pane-manager-types'
import { captureScrollState, restoreScrollState } from './pane-scroll'
// ---------------------------------------------------------------------------
// Drag-scroll locking: capture scroll state once at drag start, reuse for
// every restore during the drag to prevent cumulative drift.
// ---------------------------------------------------------------------------
export function lockDragScroll(el: HTMLElement, panes: Map<number, ManagedPaneInternal>): void {
for (const pane of findManagedPanesUnder(el, panes)) {
if (!pane.pendingDragScrollState) {
pane.pendingDragScrollState = captureScrollState(pane.terminal)
}
}
}
export function unlockDragScroll(el: HTMLElement, panes: Map<number, ManagedPaneInternal>): void {
for (const pane of findManagedPanesUnder(el, panes)) {
if (pane.pendingDragScrollState) {
try {
restoreScrollState(pane.terminal, pane.pendingDragScrollState)
} catch {
/* ignore */
}
pane.pendingDragScrollState = null
}
}
}
function findManagedPanesUnder(
el: HTMLElement,
panes: Map<number, ManagedPaneInternal>
): ManagedPaneInternal[] {
const result: ManagedPaneInternal[] = []
if (el.classList.contains('pane')) {
const pane = panes.get(Number(el.dataset.paneId))
if (pane) {
result.push(pane)
}
} else if (el.classList.contains('pane-split')) {
for (const paneEl of el.querySelectorAll('.pane[data-pane-id]')) {
const pane = panes.get(Number((paneEl as HTMLElement).dataset.paneId))
if (pane) {
result.push(pane)
}
}
}
return result
}

View file

@ -134,7 +134,8 @@ export function createPaneDOM(
webLinksAddon,
webglAddon: null,
compositionHandler: null,
pendingSplitScrollState: null
pendingSplitScrollState: null,
pendingDragScrollState: null
}
// Focus handler: clicking a pane makes it active and explicitly focuses
@ -273,6 +274,9 @@ export function attachWebgl(pane: ManagedPaneInternal): void {
try {
if (pane.pendingSplitScrollState) {
pane.fitAddon.fit()
} else if (pane.pendingDragScrollState) {
pane.fitAddon.fit()
restoreScrollState(pane.terminal, pane.pendingDragScrollState)
} else {
const scrollState = captureScrollState(pane.terminal)
pane.fitAddon.fit()

View file

@ -71,6 +71,12 @@ export type ManagedPaneInternal = {
// intermediate fit paths skip their own scroll restoration, deferring to
// the splitPane's final authoritative restore.
pendingSplitScrollState: ScrollState | null
// Why: during divider drag, each safeFit capture→fit→restore cycle uses
// approximate content-based matching that can drift by a line or two.
// Over dozens of rapid drag frames the error compounds, scrolling the
// terminal to a completely wrong position. Capturing once at drag start
// and reusing that state for every restore eliminates accumulation.
pendingDragScrollState: ScrollState | null
} & ManagedPane
export type DropZone = 'top' | 'bottom' | 'left' | 'right'

View file

@ -35,6 +35,7 @@ import {
captureScrollState,
refitPanesUnder
} from './pane-tree-ops'
import { lockDragScroll, unlockDragScroll } from './pane-drag-scroll'
import { scheduleSplitScrollRestore } from './pane-split-scroll'
export type { PaneManagerOptions, PaneStyleOptions, ManagedPane, DropZone }
@ -333,6 +334,8 @@ export class PaneManager {
private createDividerWrapped(isVertical: boolean): HTMLElement {
return createDivider(isVertical, this.styleOptions, {
refitPanesUnder: (el) => refitPanesUnder(el, this.panes),
lockDragScroll: (el) => lockDragScroll(el, this.panes),
unlockDragScroll: (el) => unlockDragScroll(el, this.panes),
onLayoutChanged: this.options.onLayoutChanged
})
}
@ -365,6 +368,8 @@ export class PaneManager {
applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions),
applyDividerStyles: () => this.applyDividerStylesWrapped(),
refitPanesUnder: (el: HTMLElement) => refitPanesUnder(el, this.panes),
lockDragScroll: (el: HTMLElement) => lockDragScroll(el, this.panes),
unlockDragScroll: (el: HTMLElement) => unlockDragScroll(el, this.panes),
onLayoutChanged: this.options.onLayoutChanged
}
}

View file

@ -0,0 +1,91 @@
import type { Terminal } from '@xterm/xterm'
import type { ScrollState } from './pane-manager-types'
// ---------------------------------------------------------------------------
// Scroll restoration after reflow
// ---------------------------------------------------------------------------
// Why: xterm.js does NOT adjust viewportY for partially-scrolled buffers
// during resize/reflow. Line N before reflow shows different content than
// line N after reflow when wrapping changes (e.g. 80→40 cols makes each
// line wrap to 2 rows). To preserve the user's scroll position, we find
// the buffer line whose content matches what was at the top of the viewport
// before the reflow, then scroll to it.
//
// Why hintRatio: terminals frequently contain duplicate short lines (shell
// prompts, repeated log prefixes). A prefix-only search returns the first
// match which may be far from the actual scroll position. The proportional
// hint (viewportY / totalLines before reflow) disambiguates by preferring
// the match closest to the expected position in the reflowed buffer.
export function findLineByContent(terminal: Terminal, content: string, hintRatio?: number): number {
if (!content) {
return -1
}
const buf = terminal.buffer.active
const totalLines = buf.baseY + terminal.rows
const prefix = content.substring(0, Math.min(content.length, 40))
if (!prefix) {
return -1
}
const hintLine = hintRatio !== undefined ? Math.round(hintRatio * totalLines) : -1
let bestMatch = -1
let bestDistance = Infinity
for (let i = 0; i < totalLines; i++) {
const line = buf.getLine(i)?.translateToString(true)?.trimEnd() ?? ''
if (line.startsWith(prefix)) {
if (hintLine < 0) {
return i
}
const distance = Math.abs(i - hintLine)
if (distance < bestDistance) {
bestDistance = distance
bestMatch = i
}
}
}
return bestMatch
}
export function captureScrollState(terminal: Terminal): ScrollState {
const buf = terminal.buffer.active
const viewportY = buf.viewportY
const wasAtBottom = viewportY >= buf.baseY
const firstVisibleLineContent = buf.getLine(viewportY)?.translateToString(true)?.trimEnd() ?? ''
const totalLines = buf.baseY + terminal.rows
return { wasAtBottom, firstVisibleLineContent, viewportY, totalLines }
}
export function restoreScrollState(terminal: Terminal, state: ScrollState): void {
if (state.wasAtBottom) {
terminal.scrollToBottom()
forceViewportScrollbarSync(terminal)
return
}
const hintRatio = state.totalLines > 0 ? state.viewportY / state.totalLines : undefined
const target = findLineByContent(terminal, state.firstVisibleLineContent, hintRatio)
if (target >= 0) {
terminal.scrollToLine(target)
forceViewportScrollbarSync(terminal)
}
}
// Why: xterm 6's Viewport._sync() updates scrollDimensions after resize but
// skips the scrollPosition update when ydisp matches _latestYDisp (a stale
// internal value). This leaves the scrollbar thumb at a wrong position even
// though the rendered content is correct. A scroll jiggle (-1/+1) in the
// same JS turn forces _sync() to fire with a differing ydisp, which triggers
// setScrollPosition and syncs the scrollbar. No paint occurs between the two
// synchronous calls so the intermediate state is never visible.
function forceViewportScrollbarSync(terminal: Terminal): void {
const buf = terminal.buffer.active
if (buf.viewportY > 0) {
terminal.scrollLines(-1)
terminal.scrollLines(1)
} else if (buf.viewportY < buf.baseY) {
terminal.scrollLines(1)
terminal.scrollLines(-1)
}
}

View file

@ -1,5 +1,5 @@
import type { ManagedPaneInternal, ScrollState } from './pane-manager-types'
import { restoreScrollState } from './pane-tree-ops'
import { restoreScrollState } from './pane-scroll'
// Why: reparenting a terminal container during split resets the viewport
// scroll position (browser clears scrollTop on DOM move). This schedules a

View file

@ -1,100 +1,8 @@
import type { Terminal } from '@xterm/xterm'
import type {
DropZone,
ManagedPaneInternal,
PaneStyleOptions,
ScrollState
} from './pane-manager-types'
import type { DropZone, ManagedPaneInternal, PaneStyleOptions } from './pane-manager-types'
import { createDivider } from './pane-divider'
import { captureScrollState, restoreScrollState } from './pane-scroll'
// ---------------------------------------------------------------------------
// Scroll restoration after reflow
// ---------------------------------------------------------------------------
// Why: xterm.js does NOT adjust viewportY for partially-scrolled buffers
// during resize/reflow. Line N before reflow shows different content than
// line N after reflow when wrapping changes (e.g. 80→40 cols makes each
// line wrap to 2 rows). To preserve the user's scroll position, we find
// the buffer line whose content matches what was at the top of the viewport
// before the reflow, then scroll to it.
//
// Why hintRatio: terminals frequently contain duplicate short lines (shell
// prompts, repeated log prefixes). A prefix-only search returns the first
// match which may be far from the actual scroll position. The proportional
// hint (viewportY / totalLines before reflow) disambiguates by preferring
// the match closest to the expected position in the reflowed buffer.
export function findLineByContent(terminal: Terminal, content: string, hintRatio?: number): number {
if (!content) {
return -1
}
const buf = terminal.buffer.active
const totalLines = buf.baseY + terminal.rows
const prefix = content.substring(0, Math.min(content.length, 40))
if (!prefix) {
return -1
}
const hintLine = hintRatio !== undefined ? Math.round(hintRatio * totalLines) : -1
let bestMatch = -1
let bestDistance = Infinity
for (let i = 0; i < totalLines; i++) {
const line = buf.getLine(i)?.translateToString(true)?.trimEnd() ?? ''
if (line.startsWith(prefix)) {
if (hintLine < 0) {
return i
}
const distance = Math.abs(i - hintLine)
if (distance < bestDistance) {
bestDistance = distance
bestMatch = i
}
}
}
return bestMatch
}
export function captureScrollState(terminal: Terminal): ScrollState {
const buf = terminal.buffer.active
const viewportY = buf.viewportY
const wasAtBottom = viewportY >= buf.baseY
const firstVisibleLineContent = buf.getLine(viewportY)?.translateToString(true)?.trimEnd() ?? ''
const totalLines = buf.baseY + terminal.rows
return { wasAtBottom, firstVisibleLineContent, viewportY, totalLines }
}
export function restoreScrollState(terminal: Terminal, state: ScrollState): void {
if (state.wasAtBottom) {
terminal.scrollToBottom()
forceViewportScrollbarSync(terminal)
return
}
const hintRatio = state.totalLines > 0 ? state.viewportY / state.totalLines : undefined
const target = findLineByContent(terminal, state.firstVisibleLineContent, hintRatio)
if (target >= 0) {
terminal.scrollToLine(target)
forceViewportScrollbarSync(terminal)
}
}
// Why: xterm 6's Viewport._sync() updates scrollDimensions after resize but
// skips the scrollPosition update when ydisp matches _latestYDisp (a stale
// internal value). This leaves the scrollbar thumb at a wrong position even
// though the rendered content is correct. A scroll jiggle (-1/+1) in the
// same JS turn forces _sync() to fire with a differing ydisp, which triggers
// setScrollPosition and syncs the scrollbar. No paint occurs between the two
// synchronous calls so the intermediate state is never visible.
function forceViewportScrollbarSync(terminal: Terminal): void {
const buf = terminal.buffer.active
if (buf.viewportY > 0) {
terminal.scrollLines(-1)
terminal.scrollLines(1)
} else if (buf.viewportY < buf.baseY) {
terminal.scrollLines(1)
terminal.scrollLines(-1)
}
}
export { findLineByContent, captureScrollState, restoreScrollState } from './pane-scroll'
// ---------------------------------------------------------------------------
// Split-tree manipulation: detach, insert, promote sibling
@ -105,6 +13,8 @@ type TreeOpsCallbacks = {
getStyleOptions: () => PaneStyleOptions
safeFit: (pane: ManagedPaneInternal) => void
refitPanesUnder: (el: HTMLElement) => void
lockDragScroll: (el: HTMLElement) => void
unlockDragScroll: (el: HTMLElement) => void
onLayoutChanged?: () => void
}
@ -114,6 +24,11 @@ export function safeFit(pane: ManagedPaneInternal): void {
pane.fitAddon.fit()
return
}
if (pane.pendingDragScrollState) {
pane.fitAddon.fit()
restoreScrollState(pane.terminal, pane.pendingDragScrollState)
return
}
const state = captureScrollState(pane.terminal)
pane.fitAddon.fit()
restoreScrollState(pane.terminal, state)
@ -133,6 +48,11 @@ export function fitAllPanesInternal(panes: Map<number, ManagedPaneInternal>): vo
pane.fitAddon.fit()
continue
}
if (pane.pendingDragScrollState) {
pane.fitAddon.fit()
restoreScrollState(pane.terminal, pane.pendingDragScrollState)
continue
}
const state = captureScrollState(pane.terminal)
pane.fitAddon.fit()
restoreScrollState(pane.terminal, state)
@ -241,6 +161,8 @@ export function insertPaneNextTo(
// Create divider
const divider = createDivider(isVertical, callbacks.getStyleOptions(), {
refitPanesUnder: callbacks.refitPanesUnder,
lockDragScroll: callbacks.lockDragScroll,
unlockDragScroll: callbacks.unlockDragScroll,
onLayoutChanged: callbacks.onLayoutChanged
})