fix(editor): preserve folds and horizontal scroll across tab switches (#34810)

* fix(editor): preserve folds and horizontal scroll across tabs

* refactor(editor): keep reading state cache with its lifecycle

* test(editor): give stale-cache model its URI

* refactor(editor): keep editor view state cache beside diff view state

Move editorViewStateCache into scroll-cache.ts, clear it in the shared model
lifetime fixture, drop the e2e evidence spec and the duplicate LRU test.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
2026-10-28 17:48:32 -07:00
committed by GitHub
co-authored by OrcaWin
parent ed7fe7d5dc
commit 8600995271
6 changed files with 99 additions and 14 deletions
@@ -7,7 +7,7 @@ import { useStore } from 'zustand'
import { createEditorStore } from '@/store/slices/editor-slice-test-harness'
import { makeWorktree } from '@/store/slices/worktrees-slice-test-fixtures'
import type { OpenFile } from '@/store/slices/editor'
import { editorSelectionCache, scrollTopCache } from '@/lib/scroll-cache'
import { editorSelectionCache, editorViewStateCache, scrollTopCache } from '@/lib/scroll-cache'
import { getDefaultSettings } from '../../../../shared/constants'
const testState = vi.hoisted(() => {
@@ -59,6 +59,7 @@ afterEach(() => {
model.dispose()
}
editorSelectionCache.clear()
editorViewStateCache.clear()
scrollTopCache.clear()
Reflect.set(HTMLCanvasElement.prototype, 'getContext', originalGetContext)
testState.store = null
@@ -11,6 +11,7 @@ import {
scrollTopCache,
editorSelectionCache,
diffViewStateCache,
editorViewStateCache,
pdfViewPositionCache
} from '@/lib/scroll-cache'
@@ -127,6 +128,7 @@ export function resetModelLifetimeFixtures(): void {
scrollTopCache.clear()
editorSelectionCache.clear()
diffViewStateCache.clear()
editorViewStateCache.clear()
pdfViewPositionCache.clear()
vi.restoreAllMocks()
}
@@ -1,6 +1,6 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { editor, ISelection } from 'monaco-editor'
import { editorSelectionCache, scrollTopCache } from '@/lib/scroll-cache'
import { editorSelectionCache, editorViewStateCache, scrollTopCache } from '@/lib/scroll-cache'
import {
installMonacoViewStateTracking,
restoreMonacoViewState,
@@ -24,6 +24,7 @@ const selections: readonly ISelection[] = [
beforeEach(() => {
editorSelectionCache.clear()
editorViewStateCache.clear()
scrollTopCache.clear()
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => {
callback(0)
@@ -36,8 +37,22 @@ afterEach(() => {
})
describe('Monaco view state persistence', () => {
it('restores selected ranges and their direction after a tab remount', () => {
it('restores the complete viewport, selections and folded regions after a tab remount', () => {
const viewState: editor.ICodeEditorViewState = {
cursorState: [],
viewState: {
scrollLeft: 480,
firstPosition: { lineNumber: 18, column: 1 },
firstPositionDeltaTop: 5
},
contributionsState: {
'editor.contrib.folding': { collapsedRegions: [{ startLineNumber: 3, endLineNumber: 12 }] }
}
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: snapshot accesses only the four stubbed editor methods.
const sourceEditor = {
getModel: () => ({ uri: { toString: () => 'file:///repo/file.ts' } }),
saveViewState: () => viewState,
getScrollTop: () => 320,
getSelections: () => selections
} as unknown as editor.IStandaloneCodeEditor
@@ -46,32 +61,71 @@ describe('Monaco view state persistence', () => {
const setSelections = vi.fn()
const setScrollTop = vi.fn()
const focus = vi.fn()
const restoreViewState = vi.fn()
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: restore accesses only these stubbed editor methods.
const remountedEditor = {
getModel: sourceEditor.getModel,
restoreViewState,
setSelections,
setScrollTop,
focus
} as unknown as editor.IStandaloneCodeEditor
restoreMonacoViewState(remountedEditor, 'file.ts::tab-1')
expect(setSelections).toHaveBeenCalledWith(selections)
expect(setScrollTop).toHaveBeenCalledWith(320)
expect(restoreViewState).toHaveBeenCalledExactlyOnceWith(viewState)
expect(setSelections).not.toHaveBeenCalled()
expect(setScrollTop).not.toHaveBeenCalled()
expect(focus).toHaveBeenCalledOnce()
})
it('restores a sibling pane without claiming focus during a scoped reveal', () => {
scrollTopCache.set('file.ts::sibling', 320)
const pane = { setSelections: vi.fn(), setScrollTop: vi.fn(), focus: vi.fn() }
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: restoreMonacoViewState accesses only these three stubbed editor methods.
const pane = {
getModel: () => null,
setSelections: vi.fn(),
setScrollTop: vi.fn(),
focus: vi.fn()
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: restore accesses only these stubbed editor methods.
const instance = pane as unknown as editor.IStandaloneCodeEditor
restoreMonacoViewState(instance, 'file.ts::sibling', () => false)
expect(pane.setScrollTop).toHaveBeenCalledWith(320)
expect(pane.focus).not.toHaveBeenCalled()
})
it('does not apply saved folds to another host or pane showing the same path', () => {
const source = {
getModel: () => ({ uri: { toString: () => 'file:///repo/file.ts?owner=host-a' } }),
saveViewState: () => ({ cursorState: [], viewState: {}, contributionsState: {} }),
getScrollTop: () => 100,
getSelections: () => selections
}
snapshotMonacoViewState(
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: snapshot accesses only these stubbed editor methods.
{ current: source as unknown as editor.IStandaloneCodeEditor },
'pane-a'
)
const other = {
getModel: () => ({ uri: { toString: () => 'file:///repo/file.ts?owner=host-b' } }),
restoreViewState: vi.fn(),
setScrollTop: vi.fn(),
setSelections: vi.fn(),
focus: vi.fn()
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: restore accesses only these stubbed editor methods.
const instance = other as unknown as editor.IStandaloneCodeEditor
restoreMonacoViewState(instance, 'pane-a')
expect(other.restoreViewState).not.toHaveBeenCalled()
other.getModel = source.getModel
restoreMonacoViewState(instance, 'pane-b')
expect(other.restoreViewState).not.toHaveBeenCalled()
})
it('defers selection caching until the lifecycle snapshot', () => {
let emitCursorPosition:
| ((event: { position: { lineNumber: number; column: number } }) => void)
| undefined
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: tracking accesses only these three stubbed editor methods.
const editorInstance = {
getPosition: () => ({ lineNumber: 1, column: 1 }),
onDidChangeCursorPosition: (
@@ -1,6 +1,11 @@
import type { MutableRefObject } from 'react'
import type { editor } from 'monaco-editor'
import { editorSelectionCache, scrollTopCache, setWithLRU } from '@/lib/scroll-cache'
import {
editorSelectionCache,
editorViewStateCache,
scrollTopCache,
setWithLRU
} from '@/lib/scroll-cache'
type MonacoViewStateTrackingParams = {
editorInstance: editor.IStandaloneCodeEditor
@@ -10,6 +15,14 @@ type MonacoViewStateTrackingParams = {
setEditorCursorLine: (fileId: string, line: number) => void
}
function editorViewStateKey(
editorInstance: editor.IStandaloneCodeEditor,
viewStateKey: string
): string | null {
const model = editorInstance.getModel()
return model ? JSON.stringify([viewStateKey, model.uri.toString()]) : null
}
export function installMonacoViewStateTracking(params: MonacoViewStateTrackingParams): {
cursorPositionSub: { dispose: () => void }
scrollStateSub: { dispose: () => void }
@@ -44,16 +57,22 @@ export function restoreMonacoViewState(
viewStateKey: string,
canFocus?: () => boolean
): void {
const cacheKey = editorViewStateKey(editorInstance, viewStateKey)
const savedViewState = cacheKey === null ? undefined : editorViewStateCache.get(cacheKey)
const savedSelections = editorSelectionCache.get(viewStateKey)
const savedScrollTop = scrollTopCache.get(viewStateKey)
if (savedScrollTop !== undefined || savedSelections) {
if (savedViewState || savedScrollTop !== undefined || savedSelections) {
// Why: Monaco renders synchronously so one RAF suffices; focus inside it to avoid a scroll-0 flash before restore.
requestAnimationFrame(() => {
if (savedSelections) {
editorInstance.setSelections(savedSelections)
}
if (savedScrollTop !== undefined) {
editorInstance.setScrollTop(savedScrollTop)
if (savedViewState) {
editorInstance.restoreViewState(savedViewState)
} else {
if (savedSelections) {
editorInstance.setSelections(savedSelections)
}
if (savedScrollTop !== undefined) {
editorInstance.setScrollTop(savedScrollTop)
}
}
if (canFocus?.() !== false) {
editorInstance.focus()
@@ -71,6 +90,11 @@ export function snapshotMonacoViewState(
): void {
const ed = editorRef.current
if (ed) {
const cacheKey = editorViewStateKey(ed, viewStateKey)
const viewState = ed.saveViewState()
if (cacheKey !== null && viewState) {
setWithLRU(editorViewStateCache, cacheKey, viewState)
}
setWithLRU(scrollTopCache, viewStateKey, ed.getScrollTop())
const selections = ed.getSelections()
if (selections) {
@@ -99,6 +99,7 @@ function setupMount() {
}
const modelText = { value: 'B' }
const model = {
uri: { toString: () => 'file:///repo/file.ts' },
getValue: () => modelText.value,
getEOL: () => '\n',
pushEditOperations: vi.fn(),
+3
View File
@@ -36,3 +36,6 @@ export const mediaPlaybackPositionCache = new Map<string, number>()
// Monaco diff view state includes both cursors/selections and model state; preserve that context beyond numeric scrolling, as VS Code does.
export const diffViewStateCache = new Map<string, editor.IDiffEditorViewState>()
// Full Monaco view state carries folds and horizontal scroll that scrollTop/selection caches miss.
export const editorViewStateCache = new Map<string, editor.ICodeEditorViewState>()