From 7c6d4c7cc70da9cc16488e0dd8dc0c95d9b87b28 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Tue, 21 Jul 2026 14:53:04 -0700 Subject: [PATCH 01/12] feat(diffs): Add targeted editor focus Call `editor.focus()` with a one-based line number and optional zero-based character offset to replace the current selection with a collapsed caret at that position. Normalize out-of-range targets through the text document and preserve `preventScroll` behavior. Calls made before attachment remain no-ops. --- packages/diffs/src/editor/editor.ts | 38 ++++++++++- packages/diffs/test/editorPublicApi.test.ts | 75 ++++++++++++++++++++- 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index 64c2b4105..e6f13e7cc 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -244,6 +244,13 @@ export interface EditorOptions { __debug?: boolean; } +export interface EditorFocusOptions extends FocusOptions { + /** One-based document line number to place the caret on. */ + lineNumber?: number; + /** Zero-based character offset for a numeric line. Defaults to 0. */ + character?: number; +} + // Cap on how far an edit may widen the virtualized render window, as a multiple // of the bounded window the virtualizer last synced (~viewport + 2*hunkLineCount). // Edits within this many lines of the window bottom widen so their caret renders; @@ -681,8 +688,21 @@ export class Editor implements DiffsEditor { this.#updateSelections(this.#selections ?? []); } - focus(options?: FocusOptions): void { + focus(options?: EditorFocusOptions): void { const preventScroll = options?.preventScroll ?? false; + const lineNumber = options?.lineNumber; + if (typeof lineNumber === 'number') { + const textDocument = this.#textDocument; + if (textDocument === undefined || this.#fileInstance === undefined) { + return; + } + const position = textDocument.normalizePosition({ + line: lineNumber - 1, + character: options?.character ?? 0, + }); + this.#focusAtPosition(position, preventScroll); + return; + } const primarySelection = this.#selections?.at(-1); if (primarySelection !== undefined) { const pos = @@ -3262,6 +3282,22 @@ export class Editor implements DiffsEditor { } } + #focusAtPosition(position: Position, preventScroll: boolean): void { + const selection: EditorSelection = { + start: position, + end: position, + direction: DirectionNone, + }; + this.#canMountSelectionAction = false; + this.#updateSelections([selection]); + this.#revealLineIfCollapsed(position.line); + if (preventScroll) { + this.#focus(position, true); + } else { + this.#scrollToPrimaryCaret(); + } + } + // set window native selection to match the selection #setWindowSelection(selection: EditorSelection) { const winSelection = window.getSelection(); diff --git a/packages/diffs/test/editorPublicApi.test.ts b/packages/diffs/test/editorPublicApi.test.ts index 16ec45e0b..62d408eeb 100644 --- a/packages/diffs/test/editorPublicApi.test.ts +++ b/packages/diffs/test/editorPublicApi.test.ts @@ -2,7 +2,11 @@ import { afterAll, describe, expect, mock, spyOn, test } from 'bun:test'; import { File } from '../src/components/File'; import { DEFAULT_THEMES } from '../src/constants'; -import { Editor, type EditorOptions } from '../src/editor/editor'; +import { + Editor, + type EditorFocusOptions, + type EditorOptions, +} from '../src/editor/editor'; import type { Marker } from '../src/editor/marker'; import { disposeHighlighter } from '../src/highlighter/shared_highlighter'; import type { DiffsEditableComponent, FileContents } from '../src/types'; @@ -275,6 +279,75 @@ describe('Editor focus lifecycle', () => { cleanup(); } }); + + test('focus() targets and normalizes a one-based document line', async () => { + const { cleanup, editor } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + const firstTarget: EditorFocusOptions = { + lineNumber: 2, + preventScroll: true, + }; + editor.focus(firstTarget); + expect(editor.getState().selections).toEqual([ + { + start: { line: 1, character: 0 }, + end: { line: 1, character: 0 }, + direction: 0, + }, + ]); + + editor.focus({ lineNumber: 99, character: 99, preventScroll: true }); + expect(editor.getState().selections).toEqual([ + { + start: { line: 2, character: 7 }, + end: { line: 2, character: 7 }, + direction: 0, + }, + ]); + + editor.focus({ + lineNumber: Number.NaN, + character: Number.NaN, + preventScroll: true, + }); + expect(editor.getState().selections).toEqual([ + { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + direction: 0, + }, + ]); + } finally { + cleanup(); + } + }); + + test('targeted focus honors preventScroll', async () => { + const { cleanup, editor } = await createEditorFixture('alpha\nbravo'); + const scrollIntoView = spyOn(HTMLElement.prototype, 'scrollIntoView'); + try { + editor.focus({ lineNumber: 2, preventScroll: true }); + expect(scrollIntoView).not.toHaveBeenCalled(); + + editor.focus({ lineNumber: 1 }); + expect(scrollIntoView).toHaveBeenCalledTimes(1); + } finally { + scrollIntoView.mockRestore(); + cleanup(); + } + }); + + test('targeted focus before attachment is a no-op', () => { + const editor = new Editor(); + try { + editor.focus({ lineNumber: 2 }); + expect(editor.getState().selections).toBeUndefined(); + } finally { + editor.cleanUp(); + } + }); }); describe('Editor.setMarkers', () => { From e4f79ab8ca9553c6b5dc54a8b392553ec4dc5324 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Tue, 21 Jul 2026 15:31:37 -0700 Subject: [PATCH 02/12] feat(diffs): Add first-visible editor focus Call `editor.focus()` with `lineNumber: "first-visible"` to place the caret on the first editable row whose top is inside the virtualized viewport. Use `offset` to keep the target below additional overlays. Expose viewport roots for virtualized files and diffs, account for sticky headers, and preserve focus when no eligible row exists. --- .../diffs/src/components/VirtualizedFile.ts | 6 + .../src/components/VirtualizedFileDiff.ts | 6 + packages/diffs/src/editor/editor.ts | 84 ++++- packages/diffs/src/types.ts | 4 + packages/diffs/test/editorPublicApi.test.ts | 297 +++++++++++++++++- .../test/virtualizedEditorViewport.test.ts | 47 +++ 6 files changed, 432 insertions(+), 12 deletions(-) create mode 100644 packages/diffs/test/virtualizedEditorViewport.test.ts diff --git a/packages/diffs/src/components/VirtualizedFile.ts b/packages/diffs/src/components/VirtualizedFile.ts index e214dfb09..8df0cec20 100644 --- a/packages/diffs/src/components/VirtualizedFile.ts +++ b/packages/diffs/src/components/VirtualizedFile.ts @@ -399,6 +399,12 @@ export class VirtualizedFile< return root instanceof HTMLElement ? root : root?.documentElement; } + public getEditorViewport(): HTMLElement | Document | undefined { + return this.virtualizer.type === 'simple' + ? this.virtualizer.getRoot() + : this.virtualizer.getContainerElement(); + } + public getNumericScrollAnchor( localViewportTop: number ): NumericScrollLineAnchor | undefined { diff --git a/packages/diffs/src/components/VirtualizedFileDiff.ts b/packages/diffs/src/components/VirtualizedFileDiff.ts index 05282ed9f..bf865b39b 100644 --- a/packages/diffs/src/components/VirtualizedFileDiff.ts +++ b/packages/diffs/src/components/VirtualizedFileDiff.ts @@ -617,6 +617,12 @@ export class VirtualizedFileDiff< return root instanceof HTMLElement ? root : root?.documentElement; } + public getEditorViewport(): HTMLElement | Document | undefined { + return this.virtualizer.type === 'simple' + ? this.virtualizer.getRoot() + : this.virtualizer.getContainerElement(); + } + public getNumericScrollAnchor( localViewportTop: number ): NumericScrollLineAnchor | undefined { diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index e6f13e7cc..457aa7591 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -245,10 +245,12 @@ export interface EditorOptions { } export interface EditorFocusOptions extends FocusOptions { - /** One-based document line number to place the caret on. */ - lineNumber?: number; + /** One-based document line number or first editable line with a visible top. */ + lineNumber?: number | 'first-visible'; /** Zero-based character offset for a numeric line. Defaults to 0. */ character?: number; + /** Non-negative CSS pixels below the viewport or sticky header. */ + offset?: number; } // Cap on how far an edit may widen the virtualized render window, as a multiple @@ -691,14 +693,22 @@ export class Editor implements DiffsEditor { focus(options?: EditorFocusOptions): void { const preventScroll = options?.preventScroll ?? false; const lineNumber = options?.lineNumber; - if (typeof lineNumber === 'number') { + if (lineNumber === 'first-visible' || typeof lineNumber === 'number') { const textDocument = this.#textDocument; - if (textDocument === undefined || this.#fileInstance === undefined) { + if (textDocument == null || this.#fileInstance == null) { + return; + } + const targetLineNumber = + lineNumber === 'first-visible' + ? this.#getFirstVisibleLineNumber(options?.offset) + : lineNumber; + if (targetLineNumber == null) { return; } const position = textDocument.normalizePosition({ - line: lineNumber - 1, - character: options?.character ?? 0, + line: targetLineNumber - 1, + character: + lineNumber === 'first-visible' ? 0 : (options?.character ?? 0), }); this.#focusAtPosition(position, preventScroll); return; @@ -3282,6 +3292,68 @@ export class Editor implements DiffsEditor { } } + #getFirstVisibleLineNumber(offset = 0): number | undefined { + const contentElement = this.#contentElement; + const fileContainer = this.#fileContainer; + const viewport = this.#fileInstance?.getEditorViewport?.(); + if ( + this.#textDocument == null || + contentElement == null || + fileContainer == null || + viewport == null + ) { + return undefined; + } + + let viewportTop: number; + let viewportBottom: number; + if (viewport instanceof HTMLElement) { + const viewportRect = viewport.getBoundingClientRect(); + viewportTop = viewportRect.top; + viewportBottom = viewportRect.bottom; + } else { + const viewportWindow = viewport.defaultView; + if (viewportWindow == null) { + return undefined; + } + viewportTop = 0; + viewportBottom = viewportWindow.innerHeight; + } + + const stickyHeader = fileContainer.shadowRoot?.querySelector( + '[data-diffs-header][data-sticky]' + ); + if (stickyHeader != null) { + viewportTop = Math.max( + viewportTop, + stickyHeader.getBoundingClientRect().bottom + ); + } + if (Number.isFinite(offset)) { + viewportTop += Math.max(0, offset); + } + if (viewportBottom <= viewportTop) { + return undefined; + } + + for (const child of contentElement.children) { + const row = child as HTMLElement; + const lineNumber = getLineNumberAttr(row); + const lineType = row.dataset.lineType; + if (lineNumber == null || lineType == null || !isLineEditable(lineType)) { + continue; + } + const rowRect = row.getBoundingClientRect(); + if (rowRect.bottom <= rowRect.top) { + continue; + } + if (rowRect.top >= viewportTop && rowRect.top < viewportBottom) { + return lineNumber; + } + } + return undefined; + } + #focusAtPosition(position: Position, preventScroll: boolean): void { const selection: EditorSelection = { start: position, diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index 47cb5d959..457a4cf78 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -1046,6 +1046,10 @@ export interface DiffsEditableComponent< * Return the scroll container element. */ getScrollContainer?: () => HTMLElement | undefined; + /** + * Return the viewport that bounds visible editor rows. + */ + getEditorViewport?: () => HTMLElement | Document | undefined; /** * Whether the given one-based new-file line currently has (or will have on * scroll) a rendered row. False only for lines hidden inside a collapsed diff --git a/packages/diffs/test/editorPublicApi.test.ts b/packages/diffs/test/editorPublicApi.test.ts index 62d408eeb..4d77afc5e 100644 --- a/packages/diffs/test/editorPublicApi.test.ts +++ b/packages/diffs/test/editorPublicApi.test.ts @@ -1,6 +1,7 @@ import { afterAll, describe, expect, mock, spyOn, test } from 'bun:test'; import { File } from '../src/components/File'; +import { FileDiff } from '../src/components/FileDiff'; import { DEFAULT_THEMES } from '../src/constants'; import { Editor, @@ -20,12 +21,15 @@ async function waitForEditableContent( container: HTMLElement ): Promise { for (let attempt = 0; attempt < 20; attempt++) { - const content = container.shadowRoot?.querySelector('[data-content]'); - if ( - content instanceof HTMLElement && - (content.contentEditable === 'true' || - content.getAttribute('contenteditable') === 'true') - ) { + const content = Array.from( + container.shadowRoot?.querySelectorAll('[data-content]') ?? + [] + ).find( + (element) => + element.contentEditable === 'true' || + element.getAttribute('contenteditable') === 'true' + ); + if (content != null) { return content; } await wait(0); @@ -41,6 +45,14 @@ interface EditorFixture { file: File; } +interface DiffEditorFixture { + cleanup(): void; + container: HTMLElement; + content: HTMLElement; + editor: Editor; + fileDiff: FileDiff; +} + // Mounts a real File-backed editor, mirroring the harness the applyEdits and // marker suites use, and returns the editor plus its contenteditable element. async function createEditorFixture( @@ -75,6 +87,75 @@ async function createEditorFixture( }; } +async function createDiffEditorFixture( + diffStyle: 'split' | 'unified' +): Promise { + const dom = installDom(); + const container = document.createElement('div'); + document.body.appendChild(container); + + const fileDiff = new FileDiff({ + disableFileHeader: true, + diffStyle, + theme: DEFAULT_THEMES, + }); + const editor = new Editor(); + fileDiff.render({ + oldFile: { name: 'edits.ts', contents: 'alpha\nold\ncharlie' }, + newFile: { name: 'edits.ts', contents: 'alpha\nnew\ncharlie' }, + fileContainer: container, + forceRender: true, + }); + editor.edit(fileDiff); + + const content = await waitForEditableContent(container); + return { + cleanup() { + editor.cleanUp(); + fileDiff.cleanUp(); + dom.cleanup(); + }, + container, + content, + editor, + fileDiff, + }; +} + +function setEditorViewport( + file: DiffsEditableComponent, + viewport: HTMLElement | Document +): void { + file.getEditorViewport = () => viewport; +} + +function setRect(element: Element, top: number, bottom: number): void { + Object.defineProperty(element, 'getBoundingClientRect', { + configurable: true, + value: () => + ({ + bottom, + height: bottom - top, + left: 0, + right: 100, + top, + width: 100, + x: 0, + y: top, + toJSON() { + return {}; + }, + }) as DOMRect, + }); +} + +function getLineRows(content: HTMLElement): HTMLElement[] { + return Array.from(content.children).filter( + (child): child is HTMLElement => + child instanceof HTMLElement && child.dataset.line != null + ); +} + function insertText( editor: Editor, line: number, @@ -348,6 +429,210 @@ describe('Editor focus lifecycle', () => { editor.cleanUp(); } }); + + test('first-visible focus targets the first row whose top is in an element viewport', async () => { + const { cleanup, content, editor, file } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + const viewport = document.createElement('div'); + setEditorViewport(file, viewport); + setRect(viewport, 10, 50); + + const rows = getLineRows(content); + setRect(rows[0], 0, 20); + setRect(rows[1], 40, 60); + setRect(rows[2], 60, 80); + + editor.focus({ + lineNumber: 'first-visible', + character: 4, + preventScroll: true, + }); + expect(editor.getState().selections).toEqual([ + { + start: { line: 1, character: 0 }, + end: { line: 1, character: 0 }, + direction: 0, + }, + ]); + } finally { + cleanup(); + } + }); + + test('first-visible focus uses document viewport bounds', async () => { + const { cleanup, content, editor, file } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + setEditorViewport(file, document); + Object.defineProperty(window, 'innerHeight', { + configurable: true, + value: 50, + }); + + const rows = getLineRows(content); + setRect(rows[0], -1, 19); + setRect(rows[1], 19, 39); + setRect(rows[2], 39, 59); + + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + expect(editor.getState().selections?.[0]?.start).toEqual({ + line: 1, + character: 0, + }); + } finally { + cleanup(); + } + }); + + test('first-visible focus applies a top offset after sticky headers', async () => { + const { cleanup, content, editor, file } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + const viewport = document.createElement('div'); + setEditorViewport(file, viewport); + setRect(viewport, 0, 60); + + const shadowRoot = content.getRootNode() as ShadowRoot; + const header = document.createElement('div'); + header.dataset.diffsHeader = 'file'; + header.dataset.sticky = ''; + setRect(header, 0, 20); + shadowRoot.prepend(header); + + for (const lineType of [ + 'annotation', + 'separator', + 'buffer', + 'change-deletion', + ]) { + const row = document.createElement('div'); + row.dataset.line = '99'; + row.dataset.lineType = lineType; + setRect(row, 35, 40); + content.prepend(row); + } + + const rows = getLineRows(content).filter( + (row) => row.dataset.line !== '99' + ); + setRect(rows[0], 10, 20); + setRect(rows[1], 25, 35); + setRect(rows[2], 35, 45); + + editor.focus({ + lineNumber: 'first-visible', + preventScroll: true, + offset: 10, + }); + expect(editor.getState().selections?.[0]?.start.line).toBe(2); + } finally { + cleanup(); + } + }); + + test('first-visible focus preserves focus and selection without a visible row top', async () => { + const { cleanup, content, editor, file } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + editor.focus({ lineNumber: 2, preventScroll: true }); + await wait(0); + const selections = editor.getState().selections; + const button = document.createElement('button'); + document.body.appendChild(button); + button.focus(); + + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + expect(editor.getState().selections).toEqual(selections); + expect(document.activeElement).toBe(button); + + const viewport = document.createElement('div'); + setEditorViewport(file, viewport); + setRect(viewport, 0, 10); + const rows = getLineRows(content); + setRect(rows[0], -5, 5); + setRect(rows[1], 10, 20); + setRect(rows[2], 15, 25); + + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + expect(editor.getState().selections).toEqual(selections); + expect(document.activeElement).toBe(button); + } finally { + cleanup(); + } + }); + + test('first-visible focus skips unified deletion rows', async () => { + const { cleanup, content, editor, fileDiff } = + await createDiffEditorFixture('unified'); + try { + const viewport = document.createElement('div'); + setEditorViewport(fileDiff, viewport); + setRect(viewport, 0, 60); + + const rows = getLineRows(content); + for (const row of rows) { + setRect(row, 100, 120); + } + const deletion = rows.find( + (row) => row.dataset.lineType === 'change-deletion' + ); + const addition = rows.find( + (row) => row.dataset.lineType === 'change-addition' + ); + const trailingContext = rows.find( + (row) => row.dataset.line === '3' && row.dataset.lineType === 'context' + ); + expect(deletion).toBeDefined(); + expect(addition).toBeDefined(); + expect(trailingContext).toBeDefined(); + setRect(deletion!, 10, 30); + setRect(addition!, -5, 15); + setRect(trailingContext!, 30, 50); + + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + expect(editor.getState().selections?.[0]?.start.line).toBe(2); + } finally { + cleanup(); + } + }); + + test('first-visible focus scans the additions column in split diffs', async () => { + const { cleanup, container, content, editor, fileDiff } = + await createDiffEditorFixture('split'); + try { + const viewport = document.createElement('div'); + setEditorViewport(fileDiff, viewport); + setRect(viewport, 0, 60); + + const deletionContent = container.shadowRoot?.querySelector( + '[data-code][data-deletions] [data-content]' + ); + expect(deletionContent).toBeDefined(); + for (const row of getLineRows(deletionContent!)) { + setRect(row, 10, 30); + } + + const additionRows = getLineRows(content); + for (const row of additionRows) { + setRect(row, 100, 120); + } + const target = additionRows.find( + (row) => row.dataset.line === '3' && row.dataset.lineType === 'context' + ); + expect(target).toBeDefined(); + setRect(target!, 30, 50); + + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + expect(editor.getState().selections?.[0]?.start.line).toBe(2); + } finally { + cleanup(); + } + }); }); describe('Editor.setMarkers', () => { diff --git a/packages/diffs/test/virtualizedEditorViewport.test.ts b/packages/diffs/test/virtualizedEditorViewport.test.ts new file mode 100644 index 000000000..e3afd0014 --- /dev/null +++ b/packages/diffs/test/virtualizedEditorViewport.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, test } from 'bun:test'; + +import { VirtualizedFile } from '../src/components/VirtualizedFile'; +import { VirtualizedFileDiff } from '../src/components/VirtualizedFileDiff'; +import { installDom } from './domHarness'; + +describe('virtualized editor viewport', () => { + test('uses the simple Virtualizer root', () => { + const dom = installDom(); + const root = document.createElement('div'); + const virtualizer = { + type: 'simple', + getRoot: () => root, + } as never; + const file = new VirtualizedFile({}, virtualizer); + const fileDiff = new VirtualizedFileDiff({}, virtualizer); + + try { + expect(file.getEditorViewport()).toBe(root); + expect(fileDiff.getEditorViewport()).toBe(root); + } finally { + file.cleanUp(); + fileDiff.cleanUp(); + dom.cleanup(); + } + }); + + test('uses the advanced CodeView container', () => { + const dom = installDom(); + const root = document.createElement('div'); + const codeView = { + type: 'advanced', + getContainerElement: () => root, + } as never; + const file = new VirtualizedFile({}, codeView); + const fileDiff = new VirtualizedFileDiff({}, codeView); + + try { + expect(file.getEditorViewport()).toBe(root); + expect(fileDiff.getEditorViewport()).toBe(root); + } finally { + file.cleanUp(); + fileDiff.cleanUp(); + dom.cleanup(); + } + }); +}); From dcd00dded2bba727c8165809ddc0e7c03ea81485 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Tue, 21 Jul 2026 15:34:26 -0700 Subject: [PATCH 03/12] test(diffs): Cover CodeView editor attach options Pass an `onAttach` callback through React CodeView `editOptions` for simultaneous file and diff sessions. Assert the provider receives the same callback while item-specific `onChange` routing remains isolated. --- packages/diffs/test/reactCodeViewEditOptions.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/diffs/test/reactCodeViewEditOptions.test.ts b/packages/diffs/test/reactCodeViewEditOptions.test.ts index 1a4a0836b..e6a1b366d 100644 --- a/packages/diffs/test/reactCodeViewEditOptions.test.ts +++ b/packages/diffs/test/reactCodeViewEditOptions.test.ts @@ -337,6 +337,7 @@ describe('React CodeView editor factory', () => { document.body.appendChild(container); const { createEditor, editors, receivedOptions } = createEditorHarness(); const attemptedOnChange = mock((_file: FileContents) => {}); + const onAttach = mock(() => {}); const onItemEditChange = mock( (_item: CodeViewItem, _file: FileContents) => {} ); @@ -344,6 +345,7 @@ describe('React CodeView editor factory', () => { // A loosely typed caller can still carry onChange at runtime. CodeView's // item router must overwrite it before invoking the provider factory. historyMaxEntries: 17, + onAttach, onChange: attemptedOnChange, roundedSelection: false, }; @@ -368,6 +370,7 @@ describe('React CodeView editor factory', () => { expect(receivedOptions).toHaveLength(2); for (const options of receivedOptions) { expect(options.historyMaxEntries).toBe(17); + expect(options.onAttach).toBe(onAttach); expect(options.roundedSelection).toBe(false); expect(options.onChange).toBeDefined(); expect(options.onChange).not.toBe(attemptedOnChange); From 6e44dfcb25673f6d8c498aa1ac8a363fef3f1851 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Tue, 21 Jul 2026 17:25:05 -0700 Subject: [PATCH 04/12] fix(diffs): Stabilize edit-session focus Enter edit mode from the playground or a virtualized surface: focus could miss the first visible line, and later row replacement could reclaim focus from another control. Fall back to the nearest scrollable viewport, defer onAttach until the live editable DOM is synchronized, and restore focus only while the editor already owns it. Document and demonstrate the opt-in autofocus recipe. --- .../app/(diffs)/docs/CodeView/content.mdx | 38 ++++++ .../docs/app/(diffs)/docs/Editor/constants.ts | 28 ++++- apps/docs/app/(diffs)/docs/Editor/content.mdx | 54 +++++++++ .../(diffs)/playground/PlaygroundClient.tsx | 1 + .../(diffs)/playground/PlaygroundCodeView.tsx | 8 ++ .../PlaygroundVirtualizerElementView.tsx | 3 + .../playground/PlaygroundVirtualizerView.tsx | 6 + packages/diffs/src/editor/editor.ts | 49 +++++++- packages/diffs/src/types.ts | 3 +- packages/diffs/test/editorPublicApi.test.ts | 33 +++++- packages/diffs/test/editorRecycle.test.ts | 109 +++++++++++++++++- 11 files changed, 320 insertions(+), 12 deletions(-) diff --git a/apps/docs/app/(diffs)/docs/CodeView/content.mdx b/apps/docs/app/(diffs)/docs/CodeView/content.mdx index fe54c5a54..fc378c389 100644 --- a/apps/docs/app/(diffs)/docs/CodeView/content.mdx +++ b/apps/docs/app/(diffs)/docs/CodeView/content.mdx @@ -82,6 +82,44 @@ For vanilla `CodeView`, keep using `CodeViewOptions.createEditor`. It exposes the same item-aware callbacks, and CodeView owns each returned editor's lifecycle. +#### Autofocus on Attach + +Autofocus is opt-in per edit session. In React, pass a stable `editOptions` +object whose `onAttach` callback targets the first editable row with a visible +top edge: + +```tsx +const editOptions: EditorOptions = { + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, +}; + +; +``` + +For vanilla `CodeView`, add the callback while constructing each item editor: + +```ts +const viewer = new CodeView({ + createEditor(options) { + return new Editor({ + ...options, + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, + }); + }, +}); +``` + +`preventScroll: true` preserves the viewer's scroll position. `CodeView` keeps +the editor alive while an item is recycled, so the callback does not steal focus +again when that item re-enters the virtualized window. If several items start an +autofocusing edit session together, the last callback to run owns focus. See +[Autofocus on Attach](#edit-mode-autofocus-on-attach) for explicit line targets, +viewport fallback, offsets, and selection-state behavior. + ### Padding & Gap For controlling layout inside and between items in `CodeView`, you can use the diff --git a/apps/docs/app/(diffs)/docs/Editor/constants.ts b/apps/docs/app/(diffs)/docs/Editor/constants.ts index d6b538b7d..e865891b5 100644 --- a/apps/docs/app/(diffs)/docs/Editor/constants.ts +++ b/apps/docs/app/(diffs)/docs/Editor/constants.ts @@ -230,7 +230,12 @@ root.style.overflow = 'auto'; const viewer = new CodeView({ theme: { dark: 'pierre-dark', light: 'pierre-light' }, createEditor(options) { - return new Editor(options); + return new Editor({ + ...options, + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, + }); }, onItemEditChange(item, _file, nextAnnotations) { if ( @@ -737,6 +742,12 @@ const initialItems: CodeViewItem[] = [ const codeViewStyle = { height: '24rem', overflow: 'auto' } as const; +const editOptions: EditorOptions = { + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, +}; + function createEditor(options: EditorOptions) { return new Editor(options); } @@ -823,6 +834,7 @@ export function EditableCodeView() { ( @@ -1001,7 +1013,7 @@ export const EDITOR_PUBLIC_API: PreloadFileOptions = { type EditorState, type FileContents, } from '@pierre/diffs'; -import { Editor } from '@pierre/diffs/editor'; +import { Editor, type EditorFocusOptions } from '@pierre/diffs/editor'; // Editor // Most methods require an attached surface via edit(). @@ -1089,6 +1101,18 @@ editor.setMarkers([]); // Blur removes focus from the content area. editor.focus(); editor.focus({ preventScroll: true }); + +// Numeric line numbers are one-based; character offsets are zero-based. +editor.focus({ lineNumber: 13, character: 4 }); + +// Target the first editable row whose top is visible. offset adds a +// non-negative CSS-pixel inset below the viewport or sticky file header. +const focusOptions: EditorFocusOptions = { + lineNumber: 'first-visible', + offset: 8, + preventScroll: true, +}; +editor.focus(focusOptions); editor.blur(); // Whether there is an edit to undo or redo. diff --git a/apps/docs/app/(diffs)/docs/Editor/content.mdx b/apps/docs/app/(diffs)/docs/Editor/content.mdx index b0b781cb4..6175e84de 100644 --- a/apps/docs/app/(diffs)/docs/Editor/content.mdx +++ b/apps/docs/app/(diffs)/docs/Editor/content.mdx @@ -255,6 +255,60 @@ sessions keep their current editor, while the latest values apply to the next item that enters edit mode. Simultaneously edited items always receive independent editor instances. +### Autofocus on Attach + +Use `onAttach` to opt into initial caret placement. At this point the text +document and editable DOM are ready. In React, pass a stable `editOptions` +object to any editable surface, including `CodeView`: + +```tsx +const editOptions = useMemo>( + () => ({ + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, + }), + [] +); + +return ; +``` + +Vanilla `CodeView` can add the same behavior in its editor factory: + +```ts +const viewer = new CodeView({ + createEditor(options) { + return new Editor({ + ...options, + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, + }); + }, +}); +``` + +`'first-visible'` selects the first editable row whose top falls inside the +usable viewport and places the caret at character zero. If no editable row top +is visible, the call does nothing. Set `offset` to a non-negative number of CSS +pixels to move the usable top below the viewport if needed. With +`preventScroll: true`, the focus request does not change the vertical scroll +position. + +You can also target a specific document position: + +```ts +editor.focus({ lineNumber: 13, character: 4 }); +``` + +Numeric `lineNumber` values are one-based, while `character` values are +zero-based. Numeric targeting works on every attached editor. Any targeted focus +replaces the current selection, so use either this initial placement or restored +selection/view state as the owner of the edit session's starting position, not +both. `CodeView` retains an editor while its item is recycled, so `onAttach` is +not repeated for virtualized remounts in the same edit session. + ### Editor Options Pass these when constructing `new Editor({ ... })`, or update them later with diff --git a/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx b/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx index b7a992cfd..08783a39c 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx @@ -727,6 +727,7 @@ export function PlaygroundClient({ prerenderedDiff }: PlaygroundClientProps) { () => ({ onAttach(editor: Editor) { editorRef.current = editor; + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); }, onChange: (_file, lineAnnotations) => { if ( diff --git a/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx b/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx index 8c43c5a6a..350a4bbae 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx @@ -12,6 +12,7 @@ import { parseDiffFromFile, type SelectedLineRange, } from '@pierre/diffs'; +import type { EditorOptions } from '@pierre/diffs/editor'; import { CodeView, type CodeViewReactOptions, @@ -32,6 +33,12 @@ const CODE_VIEW_STYLES = { height: '70vh', overflow: 'auto' } as const; type PlaygroundItem = CodeViewItem; +const CODE_VIEW_EDIT_OPTIONS: EditorOptions = { + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, +}; + interface PlaygroundCodeViewProps { items: PlaygroundItem[]; options: CodeViewReactOptions; @@ -420,6 +427,7 @@ export function PlaygroundCodeView({ return ( >( () => ({ + onAttach(editor) { + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + }, onChange(_file, lineAnnotations) { if ( lineAnnotations != null && diff --git a/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx b/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx index af3a803ed..fe752421c 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx @@ -151,6 +151,12 @@ export function PlaygroundVirtualizerView({ // pre-edit lines. An annotation whose line was deleted is dropped from // the set; retire its orphaned React root. const editor = new Editor({ + onAttach(attachedEditor) { + attachedEditor.focus({ + lineNumber: 'first-visible', + preventScroll: true, + }); + }, onChange: (_file, lineAnnotations) => { if ( lineAnnotations == null || diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index 457aa7591..b4051c313 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -1104,6 +1104,7 @@ export class Editor implements DiffsEditor { } else if ( this.#selections !== undefined && this.#selections.length > 0 && + this.#contentHasFocus && !this.#retainSearchPanelFocus ) { this.focus({ preventScroll: true }); @@ -1432,6 +1433,17 @@ export class Editor implements DiffsEditor { ) { return; } + const contentElement = this.#contentElement; + const shadowRoot = this.#fileContainer?.shadowRoot; + // A host rerender queued ahead of this callback may have replaced the + // editable DOM while its asynchronous editor sync is still pending. + if ( + contentElement == null || + shadowRoot == null || + !shadowRoot.contains(contentElement) + ) { + return; + } attachState.delivered = true; this.#options.onAttach?.(this, fileInstance); }; @@ -2965,7 +2977,9 @@ export class Editor implements DiffsEditor { this.#markerRenderer !== undefined ) { this.#updateSelections(this.#selections ?? []); - if (this.#selections !== undefined) { + // Virtualized row swaps can resize a blurred editor. Repaint its overlays + // without letting that layout work reclaim DOM focus. + if (this.#selections !== undefined && this.#contentHasFocus) { this.focus(); } } @@ -3295,15 +3309,16 @@ export class Editor implements DiffsEditor { #getFirstVisibleLineNumber(offset = 0): number | undefined { const contentElement = this.#contentElement; const fileContainer = this.#fileContainer; - const viewport = this.#fileInstance?.getEditorViewport?.(); if ( this.#textDocument == null || contentElement == null || - fileContainer == null || - viewport == null + fileContainer == null ) { return undefined; } + const viewport = + this.#fileInstance?.getEditorViewport?.() ?? + this.#getDefaultEditorViewport(fileContainer); let viewportTop: number; let viewportBottom: number; @@ -3354,6 +3369,32 @@ export class Editor implements DiffsEditor { return undefined; } + // Non-virtualized surfaces inherit visibility from their nearest vertical + // scrollport; page-scrolling surfaces use the owning document. + #getDefaultEditorViewport( + fileContainer: HTMLElement + ): HTMLElement | Document { + const ownerDocument = fileContainer.ownerDocument; + let element = fileContainer.parentElement; + while ( + element != null && + element !== ownerDocument.body && + element !== ownerDocument.documentElement + ) { + const overflowY = + element.ownerDocument.defaultView?.getComputedStyle(element).overflowY; + if ( + overflowY === 'auto' || + overflowY === 'scroll' || + overflowY === 'overlay' + ) { + return element; + } + element = element.parentElement; + } + return ownerDocument; + } + #focusAtPosition(position: Position, preventScroll: boolean): void { const selection: EditorSelection = { start: position, diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index 457a4cf78..c64502e9c 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -1047,7 +1047,8 @@ export interface DiffsEditableComponent< */ getScrollContainer?: () => HTMLElement | undefined; /** - * Return the viewport that bounds visible editor rows. + * Return an explicit viewport that bounds visible editor rows. Components + * without one fall back to their nearest scrollable ancestor or document. */ getEditorViewport?: () => HTMLElement | Document | undefined; /** diff --git a/packages/diffs/test/editorPublicApi.test.ts b/packages/diffs/test/editorPublicApi.test.ts index 4d77afc5e..4e36ce7db 100644 --- a/packages/diffs/test/editorPublicApi.test.ts +++ b/packages/diffs/test/editorPublicApi.test.ts @@ -461,12 +461,11 @@ describe('Editor focus lifecycle', () => { } }); - test('first-visible focus uses document viewport bounds', async () => { - const { cleanup, content, editor, file } = await createEditorFixture( + test('first-visible focus falls back to document viewport bounds', async () => { + const { cleanup, content, editor } = await createEditorFixture( 'alpha\nbravo\ncharlie' ); try { - setEditorViewport(file, document); Object.defineProperty(window, 'innerHeight', { configurable: true, value: 50, @@ -487,6 +486,34 @@ describe('Editor focus lifecycle', () => { } }); + test('first-visible focus falls back to the nearest scrollable ancestor', async () => { + const { cleanup, content, editor } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + const fileContainer = (content.getRootNode() as ShadowRoot) + .host as HTMLElement; + const viewport = document.createElement('div'); + viewport.style.overflowY = 'auto'; + document.body.appendChild(viewport); + viewport.appendChild(fileContainer); + setRect(viewport, 10, 50); + + const rows = getLineRows(content); + setRect(rows[0], 0, 20); + setRect(rows[1], 20, 40); + setRect(rows[2], 40, 60); + + editor.focus({ lineNumber: 'first-visible', preventScroll: true }); + expect(editor.getState().selections?.[0]?.start).toEqual({ + line: 1, + character: 0, + }); + } finally { + cleanup(); + } + }); + test('first-visible focus applies a top offset after sticky headers', async () => { const { cleanup, content, editor, file } = await createEditorFixture( 'alpha\nbravo\ncharlie' diff --git a/packages/diffs/test/editorRecycle.test.ts b/packages/diffs/test/editorRecycle.test.ts index 9c7ec661f..c9da4e7ed 100644 --- a/packages/diffs/test/editorRecycle.test.ts +++ b/packages/diffs/test/editorRecycle.test.ts @@ -37,9 +37,22 @@ class TestEditableComponent implements DiffsEditableComponent { #file: FileContents; #lineAnnotations?: DiffLineAnnotation[]; #renderRange?: RenderRange; - - constructor(file: FileContents) { + #queueRerender: boolean; + #onContentFocus?: (content: HTMLElement) => void; + + constructor( + file: FileContents, + { + queueRerender = false, + onContentFocus, + }: { + queueRerender?: boolean; + onContentFocus?: (content: HTMLElement) => void; + } = {} + ) { this.#file = file; + this.#queueRerender = queueRerender; + this.#onContentFocus = onContentFocus; this.#renderShadowDom(); } @@ -81,6 +94,13 @@ class TestEditableComponent implements DiffsEditableComponent { } rerender(): void { + if (this.#queueRerender) { + queueRender(() => { + this.#renderShadowDom(); + void Promise.resolve().then(() => this.#syncRenderView()); + }); + return; + } this.#renderShadowDom(); this.#syncRenderView(); } @@ -129,6 +149,9 @@ class TestEditableComponent implements DiffsEditableComponent { this.fileContainer.shadowRoot ?? this.fileContainer.attachShadow({ mode: 'open' }); shadowRoot.replaceChildren(); + if (this.#renderRange?.totalLines === 0) { + return; + } const code = document.createElement('div'); code.dataset.code = ''; @@ -138,6 +161,9 @@ class TestEditableComponent implements DiffsEditableComponent { const content = document.createElement('div'); content.dataset.content = ''; + if (this.#onContentFocus !== undefined) { + content.focus = () => this.#onContentFocus?.(content); + } const lines = this.#file.contents.split('\n'); for (const [index, line] of lines.entries()) { @@ -186,6 +212,31 @@ function insertAtStart(editor: Editor, text: string): void { } describe('Editor onAttach lifecycle', () => { + test('waits for a queued host rerender to synchronize before notifying', async () => { + const dom = installDom(); + const focusTargets: HTMLElement[] = []; + const onAttach = mock((attachedEditor: Editor) => { + attachedEditor.focus({ preventScroll: true }); + }); + const editor = new Editor({ onAttach }); + const component = new TestEditableComponent(createFile(), { + queueRerender: true, + onContentFocus: (content) => focusTargets.push(content), + }); + try { + editor.edit(component); + await wait(20); + + expect(onAttach).toHaveBeenCalledTimes(1); + expect(focusTargets).toHaveLength(1); + expect(focusTargets[0] === component.contentElement).toBe(true); + } finally { + editor.cleanUp(); + component.cleanUp(); + dom.cleanup(); + } + }); + test('ignores pending notifications and late syncs after full cleanup', async () => { const dom = installDom(); const onAttach = mock( @@ -382,6 +433,60 @@ describe('Editor recycle cleanUp', () => { } }); + test('an empty virtualized window preserves selections without restoring focus', async () => { + const dom = installDom(); + const onAttach = mock((attachedEditor: Editor) => { + attachedEditor.focus({ lineNumber: 2, preventScroll: true }); + }); + const editor = new Editor({ onAttach }); + const component = new TestEditableComponent(createFile()); + try { + editor.edit(component); + await wait(20); + + expect(onAttach).toHaveBeenCalledTimes(1); + expect(editor.getState().selections?.[0]?.start.line).toBe(1); + + component.contentElement.dispatchEvent(new Event('blur')); + component.render({ + renderRange: { + startingLine: 0, + totalLines: 0, + bufferBefore: 0, + bufferAfter: 60, + }, + }); + + component.render({ + renderRange: { + startingLine: 0, + totalLines: 3, + bufferBefore: 0, + bufferAfter: 0, + }, + }); + const restoredFocus = mock((_options?: FocusOptions) => {}); + component.contentElement.focus = restoredFocus; + await wait(20); + + expect(restoredFocus).not.toHaveBeenCalled(); + Object.defineProperty(component.contentElement, 'offsetWidth', { + configurable: true, + value: 100, + }); + dom.triggerResizeObserver(component.contentElement); + await wait(20); + + expect(restoredFocus).not.toHaveBeenCalled(); + expect(onAttach).toHaveBeenCalledTimes(1); + expect(editor.getState().selections?.[0]?.start.line).toBe(1); + } finally { + editor.cleanUp(); + component.cleanUp(); + dom.cleanup(); + } + }); + test('recycled re-attach recreates a tokenizer so edits still paint', () => { const dom = installDom(); try { From f0772f02d9d217ced0e44701a5cfcba3def30458 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Tue, 21 Jul 2026 17:33:57 -0700 Subject: [PATCH 05/12] Silly content fix (why link to the section you are in, lol...) --- apps/docs/app/(diffs)/docs/CodeView/content.mdx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/docs/app/(diffs)/docs/CodeView/content.mdx b/apps/docs/app/(diffs)/docs/CodeView/content.mdx index fc378c389..a63e11fd6 100644 --- a/apps/docs/app/(diffs)/docs/CodeView/content.mdx +++ b/apps/docs/app/(diffs)/docs/CodeView/content.mdx @@ -116,9 +116,7 @@ const viewer = new CodeView({ `preventScroll: true` preserves the viewer's scroll position. `CodeView` keeps the editor alive while an item is recycled, so the callback does not steal focus again when that item re-enters the virtualized window. If several items start an -autofocusing edit session together, the last callback to run owns focus. See -[Autofocus on Attach](#edit-mode-autofocus-on-attach) for explicit line targets, -viewport fallback, offsets, and selection-state behavior. +autofocusing edit session together, the last callback to run owns focus. ### Padding & Gap From 4eb854c5a6d6697cd51c1cd88c9d9b61cea2ff84 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Tue, 21 Jul 2026 17:35:34 -0700 Subject: [PATCH 06/12] Revert "Silly content fix" This reverts commit f0772f02d9d217ced0e44701a5cfcba3def30458. --- apps/docs/app/(diffs)/docs/CodeView/content.mdx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/docs/app/(diffs)/docs/CodeView/content.mdx b/apps/docs/app/(diffs)/docs/CodeView/content.mdx index a63e11fd6..fc378c389 100644 --- a/apps/docs/app/(diffs)/docs/CodeView/content.mdx +++ b/apps/docs/app/(diffs)/docs/CodeView/content.mdx @@ -116,7 +116,9 @@ const viewer = new CodeView({ `preventScroll: true` preserves the viewer's scroll position. `CodeView` keeps the editor alive while an item is recycled, so the callback does not steal focus again when that item re-enters the virtualized window. If several items start an -autofocusing edit session together, the last callback to run owns focus. +autofocusing edit session together, the last callback to run owns focus. See +[Autofocus on Attach](#edit-mode-autofocus-on-attach) for explicit line targets, +viewport fallback, offsets, and selection-state behavior. ### Padding & Gap From 9bd7eec02d35ea0187fecdf63904d4fc264a525c Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Wed, 22 Jul 2026 15:13:49 -0700 Subject: [PATCH 07/12] fix(diffs): Preserve editor focus across DOM replacement A focused editor loses its caret when a full render replaces or moves the editable DOM, so typing stops until the user focuses it again. Capture focus intent before renderer mutations and restore it after the new DOM synchronizes. Cancel stale restoration when focus moves elsewhere, and coordinate deferred focus through the shared render queue. --- packages/diffs/src/components/File.ts | 4 + packages/diffs/src/components/FileDiff.ts | 5 + packages/diffs/src/editor/editor.ts | 187 ++++++++++++- packages/diffs/src/editor/utils.ts | 2 +- packages/diffs/src/types.ts | 2 + packages/diffs/test/CodeView.edit.test.ts | 1 + packages/diffs/test/e2e/e2e-globals.d.ts | 2 + packages/diffs/test/e2e/edit.pw.ts | 247 ++++++++++++++++++ packages/diffs/test/e2e/fixtures/edit.html | 26 ++ .../test/editorDisplayOptionResync.test.ts | 37 ++- .../test/reactCodeViewEditOptions.test.ts | 1 + .../virtualizedFilePersistedLayout.test.ts | 1 + 12 files changed, 505 insertions(+), 10 deletions(-) diff --git a/packages/diffs/src/components/File.ts b/packages/diffs/src/components/File.ts index 22bf92f01..72bd16cee 100644 --- a/packages/diffs/src/components/File.ts +++ b/packages/diffs/src/components/File.ts @@ -1476,6 +1476,9 @@ export class File< previousContainer ?? document.createElement(DIFFS_TAG_NAME); const containerChanged = previousContainer !== nextContainer; + if (previousContainer != null && containerChanged) { + this.editor?.__captureFocusForDOMReplacement(); + } if (containerChanged) { this.emitPostRender(true); } @@ -1555,6 +1558,7 @@ export class File< // If we have a new parent container for the pre element, lets go ahead and // move it into the new container else if (this.pre.parentNode !== shadowRoot) { + this.editor?.__captureFocusForDOMReplacement(); container.shadowRoot?.appendChild(this.pre); this.appliedPreAttributes = undefined; } diff --git a/packages/diffs/src/components/FileDiff.ts b/packages/diffs/src/components/FileDiff.ts index 7a0cdb8f4..c11c5c003 100644 --- a/packages/diffs/src/components/FileDiff.ts +++ b/packages/diffs/src/components/FileDiff.ts @@ -1862,6 +1862,9 @@ export class FileDiff< previousContainer ?? document.createElement(DIFFS_TAG_NAME); const containerChanged = previousContainer !== nextContainer; + if (previousContainer != null && containerChanged) { + this.editor?.__captureFocusForDOMReplacement(); + } if (containerChanged) { this.emitPostRender(true); } @@ -1943,6 +1946,7 @@ export class FileDiff< // If we have a new parent container for the pre element, lets go ahead and // move it into the new container else if (this.pre.parentNode !== shadowRoot) { + this.editor?.__captureFocusForDOMReplacement(); shadowRoot.appendChild(this.pre); this.appliedPreAttributes = undefined; } @@ -2212,6 +2216,7 @@ export class FileDiff< const unifiedAST = this.hunksRenderer.renderCodeAST('unified', result); const deletionsAST = this.hunksRenderer.renderCodeAST('deletions', result); const additionsAST = this.hunksRenderer.renderCodeAST('additions', result); + this.editor?.__captureFocusForDOMReplacement(); if (unifiedAST != null) { shouldReplace = this.codeUnified == null || diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index b4051c313..52a9ca321 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -362,6 +362,10 @@ export class Editor implements DiffsEditor { // deferred to a rAF. Lets applyEdits skip focus/scroll only on unfocused // editors, without regressing a same-tick setSelections-then-applyEdits flow. #contentHasFocus = false; + // A render-owned request object prevents an older deferred frame from + // consuming a newer replacement's focus intent. Once synchronized, target + // identifies the new content so a subsequent user blur can cancel it. + #replacementFocusRequest?: { target?: HTMLElement }; #isComposing = false; #isGutterMouseDown = false; #isContentMouseDown = false; @@ -726,6 +730,9 @@ export class Editor implements DiffsEditor { } blur(): void { + this.#replacementFocusRequest = undefined; + this.#contentHasFocus = false; + this.#shouldIgnoreSelectionChange = false; this.#contentElement?.blur(); } @@ -793,6 +800,7 @@ export class Editor implements DiffsEditor { this.#gutterElement = undefined; this.#contentElement?.removeAttribute('contentEditable'); this.#contentElement = undefined; + this.#replacementFocusRequest = undefined; this.#contentHasFocus = false; this.#overlayElement?.remove(); this.#overlayElement = undefined; @@ -849,6 +857,21 @@ export class Editor implements DiffsEditor { } } + /** @internal */ + __captureFocusForDOMReplacement(): void { + const contentElement = this.#contentElement; + const shadowActiveElement = this.#fileContainer?.shadowRoot?.activeElement; + if ( + contentElement != null && + (this.#contentHasFocus || + shadowActiveElement === contentElement || + (shadowActiveElement != null && + contentElement.contains(shadowActiveElement))) + ) { + this.#replacementFocusRequest = {}; + } + } + /** @internal */ __syncRenderView: DiffsEditor['__syncRenderView'] = ( highlighter: DiffsHighlighter, @@ -886,6 +909,7 @@ export class Editor implements DiffsEditor { } } if (codeElement === undefined || contentEl === undefined) { + this.#replacementFocusRequest = undefined; return; } @@ -1031,6 +1055,9 @@ export class Editor implements DiffsEditor { console.log('[diffs/editor] full re-render triggered !!!'); } } + if (this.#replacementFocusRequest != null) { + this.#replacementFocusRequest.target = contentEl; + } // The contenteditable host advertises role="textbox", so without an // accessible name screen readers announce an unlabeled text field. Label it @@ -1101,6 +1128,8 @@ export class Editor implements DiffsEditor { this.#scrollingToLineChar, this.#scrollingToLineNoFocus ); + } else if (this.#replacementFocusRequest !== undefined) { + this.#restoreReplacementFocus(); } else if ( this.#selections !== undefined && this.#selections.length > 0 && @@ -1479,7 +1508,46 @@ export class Editor implements DiffsEditor { dataset: 'editorOverlay', }); + const replacementFocusTargetIsContent = (event: Event) => { + const target = event.composedPath()[0]; + return ( + target instanceof Node && + this.#contentElement?.contains(target) === true + ); + }; + const cancelReplacementFocus = () => { + if (this.#replacementFocusRequest == null) { + return; + } + + // A focus or pointer target outside the editor owns the newer intent. + this.#replacementFocusRequest = undefined; + this.#contentHasFocus = false; + this.#shouldIgnoreSelectionChange = false; + }; this.#globalEventDisposes = [ + addEventListener( + document, + 'focusin', + (event) => { + if (!replacementFocusTargetIsContent(event)) { + cancelReplacementFocus(); + } + }, + { passive: true } + ), + addEventListener( + document, + 'pointerdown', + (event) => { + if (!replacementFocusTargetIsContent(event)) { + // Target handlers can synchronously render and arm the request. + queueMicrotask(cancelReplacementFocus); + } + }, + { capture: true, passive: true } + ), + addEventListener( document, 'selectionchange', @@ -1721,6 +1789,10 @@ export class Editor implements DiffsEditor { contentEl, 'blur', () => { + if (this.#replacementFocusRequest?.target === contentEl) { + this.#replacementFocusRequest = undefined; + this.#shouldIgnoreSelectionChange = false; + } this.#contentHasFocus = false; onBlur?.(); }, @@ -3279,7 +3351,11 @@ export class Editor implements DiffsEditor { } } - #focus(position?: Position, preventScroll = true) { + #focus( + position?: Position, + preventScroll = true, + shouldFocus?: () => boolean + ) { // Mark focus eagerly: the positional branch defers the real focus() to a // rAF, so a same-tick applyEdits would otherwise see the editor as // unfocused and skip repositioning while this focus still lands afterward. @@ -3293,17 +3369,103 @@ export class Editor implements DiffsEditor { }); // call focus in a request animation frame to prevent conflict with // the `setBaseAndExtent` method - requestAnimationFrame(() => { + queueRender(() => { + if (shouldFocus?.() === false) { + this.#shouldIgnoreSelectionChange = false; + return; + } this.#contentElement?.focus({ preventScroll }); // another request animation frame since the `focus` call // may trigger a selectionchange event, which should be ignored - requestAnimationFrame(() => { + queueRender(() => { this.#shouldIgnoreSelectionChange = false; }); }); - } else { + } else if (shouldFocus?.() !== false) { this.#contentElement?.focus({ preventScroll }); + } else { + this.#contentHasFocus = false; + } + } + + // A full host render can remove the focused contenteditable before the + // replacement view synchronizes. Restore only after the new content is live + // and only while focus has not moved to another real control in the gap. + #restoreReplacementFocus(position?: Position): void { + const request = this.#replacementFocusRequest; + if (request == null) { + return; + } + if (this.#retainSearchPanelFocus) { + this.#replacementFocusRequest = undefined; + this.#contentHasFocus = false; + this.#shouldIgnoreSelectionChange = false; + return; + } + const contentElement = this.#contentElement; + if ( + contentElement == null || + !this.#replacementFocusIsAvailable(contentElement) + ) { + this.#replacementFocusRequest = undefined; + this.#contentHasFocus = false; + return; + } + + const shouldFocus = () => { + if (this.#replacementFocusRequest !== request) { + return false; + } + const shouldRestore = this.#replacementFocusIsAvailable(contentElement); + this.#replacementFocusRequest = undefined; + return shouldRestore; + }; + if (position != null) { + this.#focus(position, true, shouldFocus); + return; + } + const primarySelection = this.#selections?.at(-1); + if (primarySelection == null) { + this.#focus(undefined, true, shouldFocus); + return; } + const primaryPosition = + primarySelection.direction === DirectionBackward + ? primarySelection.end + : primarySelection.start; + this.#focus(primaryPosition, true, shouldFocus); + } + + #replacementFocusIsAvailable(contentElement: HTMLElement): boolean { + const fileContainer = this.#fileContainer; + const shadowRoot = fileContainer?.shadowRoot; + if ( + fileContainer == null || + shadowRoot == null || + this.#contentElement !== contentElement || + !shadowRoot.contains(contentElement) + ) { + return false; + } + const { activeElement: shadowActiveElement } = shadowRoot; + if ( + shadowActiveElement === contentElement || + (shadowActiveElement != null && + contentElement.contains(shadowActiveElement)) + ) { + return true; + } + if (shadowActiveElement != null) { + return false; + } + const { ownerDocument } = fileContainer; + const { activeElement } = ownerDocument; + return ( + activeElement == null || + activeElement === ownerDocument.body || + activeElement === ownerDocument.documentElement || + activeElement === fileContainer + ); } #getFirstVisibleLineNumber(offset = 0): number | undefined { @@ -3469,11 +3631,15 @@ export class Editor implements DiffsEditor { inline: 'nearest', }); if (!noFocus) { - this.#focus( + const position = primarySelection.direction === DirectionBackward ? primarySelection.end - : primarySelection.start - ); + : primarySelection.start; + if (this.#replacementFocusRequest !== undefined) { + this.#restoreReplacementFocus(position); + } else { + this.#focus(position); + } } } else { const pos = getCaretPosition(primarySelection); @@ -3515,7 +3681,12 @@ export class Editor implements DiffsEditor { this.#overlayElement?.appendChild(virtualCaret); virtualCaret.scrollIntoView({ block: 'center', inline: 'nearest' }); if (!noFocus) { - this.#focus({ line, character: char }); + const position = { line, character: char }; + if (this.#replacementFocusRequest !== undefined) { + this.#restoreReplacementFocus(position); + } else { + this.#focus(position); + } } this.#scrollingToLine = undefined; this.#scrollingToLineChar = undefined; diff --git a/packages/diffs/src/editor/utils.ts b/packages/diffs/src/editor/utils.ts index 282df0a07..26ca9e920 100644 --- a/packages/diffs/src/editor/utils.ts +++ b/packages/diffs/src/editor/utils.ts @@ -68,7 +68,7 @@ export function addEventListener( options?: AddEventListenerOptions ) { el.addEventListener(event, listener, options); - return () => el.removeEventListener(event, listener); + return () => el.removeEventListener(event, listener, options); } export function getLineNumberAttr( diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index c64502e9c..ee7324e4a 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -1114,6 +1114,8 @@ export interface DiffsEditor { /** @internal */ __prepareFile?(file: FileContents): FileContents; __postponeBgTokenizeToNextFrame(): void; + /** @internal Capture focus intent before replacing the editable view. */ + __captureFocusForDOMReplacement(): void; __syncRenderView( highlighter: DiffsHighlighter, fileContainer: HTMLElement, diff --git a/packages/diffs/test/CodeView.edit.test.ts b/packages/diffs/test/CodeView.edit.test.ts index 96ab540c7..9ab1c05e2 100644 --- a/packages/diffs/test/CodeView.edit.test.ts +++ b/packages/diffs/test/CodeView.edit.test.ts @@ -73,6 +73,7 @@ function createEditorHarness({ detach?.(recycle); detach = undefined; }, + __captureFocusForDOMReplacement() {}, __postponeBgTokenizeToNextFrame() {}, __syncRenderView() {}, }; diff --git a/packages/diffs/test/e2e/e2e-globals.d.ts b/packages/diffs/test/e2e/e2e-globals.d.ts index adb6aac07..c6144aebd 100644 --- a/packages/diffs/test/e2e/e2e-globals.d.ts +++ b/packages/diffs/test/e2e/e2e-globals.d.ts @@ -77,6 +77,8 @@ interface Window { // Editor handle exposed by the editable fixtures. __editor?: E2EEditor; + __forceEditorFullRender?: () => void; + __moveEditorContainer?: () => void; // edit-collapsed.html helpers: rendered new-file line numbers in the // editable column, and the primary caret's zero-based line. diff --git a/packages/diffs/test/e2e/edit.pw.ts b/packages/diffs/test/e2e/edit.pw.ts index 6afc4877c..13cc8cf08 100644 --- a/packages/diffs/test/e2e/edit.pw.ts +++ b/packages/diffs/test/e2e/edit.pw.ts @@ -18,6 +18,12 @@ const canRedo = (page: Page): Promise => page.evaluate(() => window.__editor?.canRedo ?? false); const changeCount = (page: Page): Promise => page.evaluate(() => window.__editorEvents?.length ?? 0); +const editorHasFocus = (page: Page): Promise => + page.evaluate((selector) => { + const root = document.querySelector('diffs-container')?.shadowRoot; + const content = root?.querySelector(selector); + return root?.activeElement === content; + }, ADDITIONS); // Real user path: a plain click into the editable additions column places the // caret, then typing flows through the genuine keyboard -> beforeinput -> @@ -30,6 +36,77 @@ async function clickIntoAdditions(page: Page): Promise { await page.locator(ADDITIONS).click(); } +async function moveFocusDuringFullRender( + page: Page, + target: 'background' | 'external-link' | 'host', + timing: 'replacement' | 'after-sync' +): Promise { + await page.evaluate( + ({ selector, target, timing }) => + new Promise((resolve, reject) => { + const host = document.querySelector('diffs-container'); + const root = host?.shadowRoot; + const content = root?.querySelector(selector); + const code = content?.parentElement; + const externalLink = document.querySelector('[data-fixtures-index]'); + const focusTarget = + target === 'host' + ? host + : target === 'background' + ? document.body + : externalLink; + if ( + !(host instanceof HTMLElement) || + !(code instanceof HTMLElement) || + !(focusTarget instanceof HTMLElement) + ) { + reject(new Error('missing editor or external focus target')); + return; + } + if (target === 'host') { + host.tabIndex = -1; + } + const observer = new MutationObserver(() => { + const replacement = root?.querySelector(selector); + if ( + replacement === content || + (timing === 'after-sync' && + replacement?.getAttribute('contenteditable') !== 'true') + ) { + return; + } + observer.disconnect(); + if (target === 'background') { + focusTarget.dispatchEvent( + new PointerEvent('pointerdown', { + bubbles: true, + composed: true, + }) + ); + } else { + focusTarget.focus(); + if ( + target === 'host' && + (document.activeElement !== host || root?.activeElement != null) + ) { + reject(new Error('host did not receive focus')); + return; + } + } + resolve(); + }); + observer.observe(code, { + attributes: true, + attributeFilter: ['contenteditable'], + childList: true, + subtree: true, + }); + window.__forceEditorFullRender?.(); + }), + { selector: ADDITIONS, target, timing } + ); +} + test.describe('edit mode', () => { test('a plain click seeds the caret so typing works (regression)', async ({ page, @@ -84,6 +161,176 @@ test.describe('edit mode', () => { await expect.poll(() => canUndo(page)).toBe(true); }); + test('focused editor survives a full render and accepts input', async ({ + page, + }) => { + await openFixture(page); + + const additions = page.locator(ADDITIONS); + await additions.click(); + await expect.poll(() => editorHasFocus(page)).toBe(true); + const previousContent = await additions.elementHandle(); + if (previousContent == null) { + throw new Error('missing additions content'); + } + + await page.evaluate(() => window.__forceEditorFullRender?.()); + + await expect + .poll(() => + additions.evaluate( + (content, previous) => content !== previous, + previousContent + ) + ) + .toBe(true); + await expect.poll(() => editorHasFocus(page)).toBe(true); + await page.keyboard.type('Z'); + await expect(additions).toContainText('Z'); + }); + + test('focused editor survives overlapping full renders', async ({ page }) => { + await openFixture(page); + + const additions = page.locator(ADDITIONS); + await additions.click(); + await page.evaluate( + (selector) => + new Promise((resolve, reject) => { + const root = document.querySelector('diffs-container')?.shadowRoot; + const content = root?.querySelector(selector); + const code = content?.parentElement; + if (!(code instanceof HTMLElement)) { + reject(new Error('missing editor content')); + return; + } + const observer = new MutationObserver(() => { + const replacement = root?.querySelector(selector); + if ( + replacement === content || + replacement?.getAttribute('contenteditable') !== 'true' + ) { + return; + } + observer.disconnect(); + window.__forceEditorFullRender?.(); + resolve(); + }); + observer.observe(code, { + attributes: true, + attributeFilter: ['contenteditable'], + childList: true, + subtree: true, + }); + window.__forceEditorFullRender?.(); + }), + ADDITIONS + ); + + await expect.poll(() => editorHasFocus(page)).toBe(true); + await page.keyboard.type('Z'); + await expect(additions).toContainText('Z'); + }); + + test('focused editor without a selection survives a full render', async ({ + page, + }) => { + await openFixture(page); + + await page.evaluate(() => { + window.__editor?.setSelections([]); + window.__editor?.focus(); + }); + await expect.poll(() => editorHasFocus(page)).toBe(true); + + await page.evaluate(() => window.__forceEditorFullRender?.()); + + await expect.poll(() => editorHasFocus(page)).toBe(true); + }); + + test('focused editor survives moving to a new container', async ({ + page, + }) => { + await openFixture(page); + + const additions = page.locator(ADDITIONS); + await additions.click(); + await page.evaluate(() => window.__moveEditorContainer?.()); + + await expect.poll(() => editorHasFocus(page)).toBe(true); + await page.keyboard.type('Z'); + await expect(additions).toContainText('Z'); + }); + + test('full render does not reclaim focus moved during replacement', async ({ + page, + }) => { + await openFixture(page); + + await clickIntoAdditions(page); + const fixturesLink = page.locator('[data-fixtures-index]'); + await moveFocusDuringFullRender(page, 'external-link', 'replacement'); + + await expect(fixturesLink).toBeFocused(); + }); + + test('full render does not reclaim focus after a background press', async ({ + page, + }) => { + await openFixture(page); + + await clickIntoAdditions(page); + await moveFocusDuringFullRender(page, 'background', 'replacement'); + + await expect.poll(() => editorHasFocus(page)).toBe(false); + }); + + test('stopped background press that starts a full render does not restore focus', async ({ + page, + }) => { + await openFixture(page); + + await clickIntoAdditions(page); + await page.evaluate(() => { + document.body.addEventListener( + 'pointerdown', + (event) => { + window.__forceEditorFullRender?.(); + event.stopPropagation(); + }, + { once: true } + ); + document.body.dispatchEvent( + new PointerEvent('pointerdown', { bubbles: true, composed: true }) + ); + }); + await page.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }) + ); + + await expect.poll(() => editorHasFocus(page)).toBe(false); + }); + + test('full render does not reclaim focus moved to its host', async ({ + page, + }) => { + await openFixture(page); + + await clickIntoAdditions(page); + await moveFocusDuringFullRender(page, 'host', 'after-sync'); + await page.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }) + ); + + await expect.poll(() => editorHasFocus(page)).toBe(false); + }); + test('undo and redo round-trip a typed edit', async ({ page }) => { await openFixture(page); diff --git a/packages/diffs/test/e2e/fixtures/edit.html b/packages/diffs/test/e2e/fixtures/edit.html index 810ad109f..0a1d5ba15 100644 --- a/packages/diffs/test/e2e/fixtures/edit.html +++ b/packages/diffs/test/e2e/fixtures/edit.html @@ -99,6 +99,32 @@ instance.render({ oldFile, newFile, containerWrapper: mount }); editor.edit(instance); + window.__forceEditorFullRender = () => { + instance.setOptions({ + ...instance.options, + disableLineNumbers: !(instance.options.disableLineNumbers ?? false), + }); + instance.render({ oldFile, newFile, forceRender: true }); + }; + window.__moveEditorContainer = () => { + const previousContainer = mount.querySelector('diffs-container'); + const nextContainer = document.createElement('diffs-container'); + instance.setOptions({ + ...instance.options, + onPostRender(container, _, phase) { + if (phase === 'unmount' && container === previousContainer) { + container.remove(); + } + }, + }); + mount.appendChild(nextContainer); + instance.render({ + oldFile, + newFile, + fileContainer: nextContainer, + forceRender: true, + }); + }; const findAdditionsContent = () => { const host = mount.querySelector('diffs-container'); diff --git a/packages/diffs/test/editorDisplayOptionResync.test.ts b/packages/diffs/test/editorDisplayOptionResync.test.ts index 5bc8efe13..9e7995c1b 100644 --- a/packages/diffs/test/editorDisplayOptionResync.test.ts +++ b/packages/diffs/test/editorDisplayOptionResync.test.ts @@ -1,4 +1,4 @@ -import { afterAll, describe, expect, test } from 'bun:test'; +import { afterAll, describe, expect, spyOn, test } from 'bun:test'; import { FileDiff } from '../src/components/FileDiff'; import { DEFAULT_THEMES } from '../src/constants'; @@ -147,6 +147,41 @@ function typeAt( } describe('diff editor: display-option toggle mid-edit', () => { + test('keeps focus when replacement does not emit blur', async () => { + const fixture = await createFixture('alpha\nbravo\n', 'alpha\nCHANGED\n'); + const { container, editor } = fixture; + const focusTargets: HTMLElement[] = []; + const focusSpy = spyOn(HTMLElement.prototype, 'focus').mockImplementation( + function (this: HTMLElement) { + focusTargets.push(this); + this.dispatchEvent(new Event('focus')); + } + ); + + try { + const content = findAdditionContent(container); + if (content == null) { + throw new Error('missing editable additions content'); + } + + editor.focus({ lineNumber: 1, preventScroll: true }); + await wait(10); + expect(focusTargets.at(-1) === content).toBe(true); + + // Firefox and WebKit can remove the focused shadow subtree without a + // blur event, so replacement detection must preserve the focus intent. + await fixture.toggleDisplayOption(); + + const replacement = findAdditionContent(container); + expect(replacement == null).toBe(false); + expect(replacement === content).toBe(false); + expect(focusTargets.at(-1) === replacement).toBe(true); + } finally { + focusSpy.mockRestore(); + await fixture.cleanup(); + } + }); + test('keeps the edited line text visible when a display option is toggled', async () => { // old/new differ so the additions column (the editor's target) renders; the // edit targets line 0 ("alpha"), an unchanged context line — the "rename a diff --git a/packages/diffs/test/reactCodeViewEditOptions.test.ts b/packages/diffs/test/reactCodeViewEditOptions.test.ts index e6a1b366d..26692d238 100644 --- a/packages/diffs/test/reactCodeViewEditOptions.test.ts +++ b/packages/diffs/test/reactCodeViewEditOptions.test.ts @@ -103,6 +103,7 @@ function createTrackedEditor( detach?.(recycle); detach = undefined; }, + __captureFocusForDOMReplacement() {}, __postponeBgTokenizeToNextFrame() {}, __syncRenderView() {}, }; diff --git a/packages/diffs/test/virtualizedFilePersistedLayout.test.ts b/packages/diffs/test/virtualizedFilePersistedLayout.test.ts index eed250b3a..86ffe95bf 100644 --- a/packages/diffs/test/virtualizedFilePersistedLayout.test.ts +++ b/packages/diffs/test/virtualizedFilePersistedLayout.test.ts @@ -57,6 +57,7 @@ describe('VirtualizedFile persisted layout', () => { prepareCalls++; return cachedFile; }, + __captureFocusForDOMReplacement() {}, __postponeBgTokenizeToNextFrame() {}, __syncRenderView() {}, edit() { From 112ba5d225f107932b83e44a0b4f2bc64b93f945 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Wed, 22 Jul 2026 16:02:35 -0700 Subject: [PATCH 08/12] test(diffs): Define editor view state contract Define component-owned view capture and restoration across files, diffs, and CodeView. Cover logical vertical positions, horizontal code scrolling, and recycle behavior before replacing the raw scroll-container API. --- packages/diffs/test/CodeView.edit.test.ts | 103 ++++++++ .../test/CodeView.scrollAnchoring.test.ts | 117 +++++++++ packages/diffs/test/editorState.test.ts | 113 ++++----- .../test/virtualizedEditorViewport.test.ts | 231 ++++++++++++++++++ 4 files changed, 499 insertions(+), 65 deletions(-) diff --git a/packages/diffs/test/CodeView.edit.test.ts b/packages/diffs/test/CodeView.edit.test.ts index 9ab1c05e2..f3112033f 100644 --- a/packages/diffs/test/CodeView.edit.test.ts +++ b/packages/diffs/test/CodeView.edit.test.ts @@ -20,6 +20,7 @@ import { makeFile, renderItems, wait, + waitFor, } from './domHarness'; interface StubEditor extends DiffsEditor { @@ -529,6 +530,108 @@ describe('CodeView item edit mode', () => { } }); + test('recycle restores item-local horizontal state without restoring the shared CodeView scroll position', async () => { + const { cleanup } = installDom(); + const editors: Editor[] = []; + const viewer = new CodeView({ + createEditor(options) { + const editor = new Editor({ + ...options, + persistState: true, + }); + editors.push(editor); + return editor; + }, + }); + const edited = makeTextEditFileItem('edited', true, 30); + if (edited.type !== 'file') { + throw new Error('Expected an edited file item.'); + } + edited.file = { ...edited.file, cacheKey: 'edited' }; + const items: CodeViewItem[] = [ + edited, + ...Array.from({ length: 39 }, (_, index) => + makeTextEditFileItem(`file-${index}`, false, 30) + ), + ]; + + try { + const root = createRoot(); + viewer.setup(root); + await renderItems(viewer, items); + await waitFor(() => { + const rendered = viewer + .getRenderedItems() + .find((item) => item.id === edited.id); + return ( + viewer.getEditor(edited.id) !== undefined && + rendered?.element.shadowRoot?.querySelector('[data-code]') instanceof + HTMLElement + ); + }); + + const editor = viewer.getEditor(edited.id) as Editor; + const firstRendered = viewer + .getRenderedItems() + .find((item) => item.id === edited.id); + const firstCode = + firstRendered?.element.shadowRoot?.querySelector('[data-code]'); + expect(firstCode).toBeInstanceOf(HTMLElement); + editor.setSelections([ + { + start: { line: 2, character: 3 }, + end: { line: 2, character: 3 }, + direction: 'none', + }, + ]); + (firstCode as HTMLElement).scrollLeft = 64; + + root.scrollTop = 20_000; + dispatchScroll(root); + viewer.render(true); + await wait(0); + expect( + viewer.getRenderedItems().some((item) => item.id === edited.id) + ).toBe(false); + + root.scrollTop = 0; + dispatchScroll(root); + viewer.render(true); + await waitFor(() => { + const rendered = viewer + .getRenderedItems() + .find((item) => item.id === edited.id); + return ( + rendered?.element.shadowRoot?.querySelector('[data-code]') instanceof + HTMLElement + ); + }); + await wait(10); + + const remounted = viewer + .getRenderedItems() + .find((item) => item.id === edited.id); + const remountedCode = + remounted?.element.shadowRoot?.querySelector('[data-code]'); + expect(editors).toHaveLength(1); + expect(viewer.getEditor(edited.id)).toBe(editor); + expect(editor.getState().selections).toEqual([ + { + start: { line: 2, character: 3 }, + end: { line: 2, character: 3 }, + direction: 0, + }, + ]); + expect(remountedCode).toBeInstanceOf(HTMLElement); + expect(viewer.getScrollTop()).toBe(0); + expect((remountedCode as HTMLElement).scrollLeft).toBe(64); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + test('remounts an edited file whose document grew without crashing', async () => { const { cleanup } = installDom(); const { createEditor } = createEditorHarness(); diff --git a/packages/diffs/test/CodeView.scrollAnchoring.test.ts b/packages/diffs/test/CodeView.scrollAnchoring.test.ts index 4a7d8e5d3..54ecaffca 100644 --- a/packages/diffs/test/CodeView.scrollAnchoring.test.ts +++ b/packages/diffs/test/CodeView.scrollAnchoring.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import { CodeView } from '../src/components/CodeView'; import { DEFAULT_CODE_VIEW_LAYOUT } from '../src/constants'; +import { Editor } from '../src/editor/editor'; import type { CodeViewItem, FileContents } from '../src/types'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { @@ -11,6 +12,7 @@ import { makeFileItem, renderItems, wait, + waitFor, } from './domHarness'; const ROOT_HEIGHT = 800; @@ -103,6 +105,60 @@ function makeReplacementDiffItem( }; } +function makeEditedFileItem(id: string): CodeViewItem { + return { + id, + type: 'file', + file: { + ...makeFile(`${id}.txt`, 1), + cacheKey: id, + lang: 'text', + }, + edit: true, + }; +} + +async function renderRebasedEditedItem( + viewer: CodeView +): Promise<{ code: HTMLElement; editor: Editor }> { + const editedIndex = 12; + const edited = makeEditedFileItem('edited'); + const items = Array.from({ length: 40 }, (_, index) => + index === editedIndex ? edited : makeFileItem(`file:${index}`, 1) + ); + await renderItems(viewer, items); + + viewer.scrollTo({ + type: 'item', + id: edited.id, + align: 'start', + behavior: 'instant', + }); + viewer.render(true); + await waitFor(() => { + const rendered = viewer + .getRenderedItems() + .find((item) => item.id === edited.id); + return ( + viewer.getEditor(edited.id) !== undefined && + rendered?.element.shadowRoot?.querySelector('[data-code]') instanceof + HTMLElement + ); + }); + + const rendered = viewer + .getRenderedItems() + .find((item) => item.id === edited.id); + const code = rendered?.element.shadowRoot?.querySelector('[data-code]'); + const editor = viewer.getEditor(edited.id); + expect(code).toBeInstanceOf(HTMLElement); + expect(editor).toBeInstanceOf(Editor); + return { + code: code as HTMLElement, + editor: editor as Editor, + }; +} + describe('CodeView scroll anchoring', () => { test('keeps an item anchor fixed when split to unified grows past the old scroll range', async () => { const { cleanup } = installDom(); @@ -206,6 +262,67 @@ describe('CodeView scroll anchoring', () => { } }); + test('captures logical editor state after the DOM scroll position rebases', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView({ + createEditor: (options) => new Editor({ ...options }), + layout: { + ...DEFAULT_CODE_VIEW_LAYOUT, + gap: 1_000_000, + }, + }); + const root = createClampingRoot(); + + try { + viewer.setup(root); + const { code, editor } = await renderRebasedEditedItem(viewer); + const logicalScrollTop = viewer.getScrollTop(); + code.scrollLeft = 73; + + expect(logicalScrollTop).toBeGreaterThan(SCROLL_REBASE_THRESHOLD); + expect(root.scrollTop).not.toBe(logicalScrollTop); + expect(editor.getState().view).toEqual({ + scrollLeft: 73, + scrollTop: logicalScrollTop, + }); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + + test('restores explicit editor state through CodeView logical scrolling', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView({ + createEditor: (options) => new Editor({ ...options }), + layout: { + ...DEFAULT_CODE_VIEW_LAYOUT, + gap: 1_000_000, + }, + }); + const root = createClampingRoot(); + + try { + viewer.setup(root); + const { editor } = await renderRebasedEditedItem(viewer); + const initialScrollTop = viewer.getScrollTop(); + const restoredScrollTop = initialScrollTop - 100_000; + + editor.setState({ + view: { scrollLeft: 41, scrollTop: restoredScrollTop }, + }); + viewer.render(true); + + expect(viewer.getScrollTop()).toBe(restoredScrollTop); + expect(root.scrollTop).not.toBe(restoredScrollTop); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + test('restores the paged scroll height after clearing and reusing the viewer', async () => { const { cleanup } = installDom(); const viewer = new CodeView({ diff --git a/packages/diffs/test/editorState.test.ts b/packages/diffs/test/editorState.test.ts index df5e50a5f..8593d7377 100644 --- a/packages/diffs/test/editorState.test.ts +++ b/packages/diffs/test/editorState.test.ts @@ -6,6 +6,7 @@ import type { DiffsEditableComponent, DiffsEditor, DiffsHighlighter, + EditorState, FileContents, HighlightedToken, RenderRange, @@ -34,11 +35,10 @@ class TestEditableComponent implements DiffsEditableComponent { #editor?: DiffsEditor; #lineAnnotations?: DiffLineAnnotation[]; #renderRange?: RenderRange; + capturedView: EditorState['view']; + restoredViews: NonNullable[] = []; - constructor( - readonly scrollContainer: HTMLElement, - private file: FileContents - ) { + constructor(private file: FileContents) { this.#renderShadowDom(); } @@ -50,8 +50,13 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} - getScrollContainer(): HTMLElement { - return this.scrollContainer; + captureEditorViewState(): EditorState['view'] { + return this.capturedView == null ? undefined : { ...this.capturedView }; + } + + restoreEditorViewState(view: NonNullable): void { + this.capturedView = { ...view }; + this.restoredViews.push({ ...view }); } render({ @@ -138,48 +143,44 @@ class TestEditableComponent implements DiffsEditableComponent { } describe('Editor state', () => { - test('setState restores the saved scroll position', () => { + test('getState captures view state from the editable component', () => { const dom = installDom(); - const scrollContainer = document.createElement('div'); - document.body.appendChild(scrollContainer); - - const scrollCalls: ScrollToOptions[] = []; - scrollContainer.scrollTo = ( - options?: ScrollToOptions | number, - y?: number - ) => { - const left = - typeof options === 'number' - ? options - : (options?.left ?? scrollContainer.scrollLeft); - const top = - typeof options === 'number' - ? (y ?? scrollContainer.scrollTop) - : (options?.top ?? scrollContainer.scrollTop); - scrollContainer.scrollLeft = left; - scrollContainer.scrollTop = top; - scrollCalls.push({ left, top }); - }; - const editor = new Editor(); - const component = new TestEditableComponent(scrollContainer, { + const component = new TestEditableComponent({ name: 'state.ts', contents: 'alpha\nbravo', }); try { editor.edit(component); - scrollContainer.scrollLeft = 24; - scrollContainer.scrollTop = 128; - const state = editor.getState(); + component.capturedView = { scrollLeft: 24, scrollTop: 128 }; + + expect(editor.getState().view).toEqual({ + scrollLeft: 24, + scrollTop: 128, + }); + } finally { + editor.cleanUp(); + component.cleanUp(); + dom.cleanup(); + } + }); - scrollContainer.scrollLeft = 0; - scrollContainer.scrollTop = 0; - editor.setState(state); + test('setState restores view state through the editable component', () => { + const dom = installDom(); + const editor = new Editor(); + const component = new TestEditableComponent({ + name: 'state.ts', + contents: 'alpha\nbravo', + }); + + try { + editor.edit(component); + editor.setState({ view: { scrollLeft: 24, scrollTop: 128 } }); - expect(scrollContainer.scrollLeft).toBe(24); - expect(scrollContainer.scrollTop).toBe(128); - expect(scrollCalls).toEqual([{ left: 24, top: 128 }]); + expect(component.restoredViews).toEqual([ + { scrollLeft: 24, scrollTop: 128 }, + ]); } finally { editor.cleanUp(); component.cleanUp(); @@ -189,38 +190,18 @@ describe('Editor state', () => { // A remount restore often carries both a viewport and a caret that sits // outside that viewport. The saved view must win; scrolling the caret into - // view would overwrite scrollTop/scrollLeft. jsdom's scrollIntoView is a - // no-op, so stub it to mutate the scroll container the way a real browser - // would when bringing an offscreen caret into view. + // view would overwrite it. jsdom's scrollIntoView is a no-op, so record any + // attempt to reveal the caret after restoring the component-owned view. test('setState keeps the saved view when the caret is outside it', () => { const dom = installDom(); - const scrollContainer = document.createElement('div'); - document.body.appendChild(scrollContainer); - - scrollContainer.scrollTo = ( - options?: ScrollToOptions | number, - y?: number - ) => { - const left = - typeof options === 'number' - ? options - : (options?.left ?? scrollContainer.scrollLeft); - const top = - typeof options === 'number' - ? (y ?? scrollContainer.scrollTop) - : (options?.top ?? scrollContainer.scrollTop); - scrollContainer.scrollLeft = left; - scrollContainer.scrollTop = top; - }; - + let scrollIntoViewCalls = 0; const originalScrollIntoView = Element.prototype.scrollIntoView; Element.prototype.scrollIntoView = function scrollIntoView() { - scrollContainer.scrollTop = 999; - scrollContainer.scrollLeft = 999; + scrollIntoViewCalls++; }; const editor = new Editor(); - const component = new TestEditableComponent(scrollContainer, { + const component = new TestEditableComponent({ name: 'state.ts', contents: 'alpha\nbravo\ncharlie\ndelta\necho\nfoxtrot\n', }); @@ -238,8 +219,10 @@ describe('Editor state', () => { view: { scrollLeft: 12, scrollTop: 40 }, }); - expect(scrollContainer.scrollLeft).toBe(12); - expect(scrollContainer.scrollTop).toBe(40); + expect(component.restoredViews).toEqual([ + { scrollLeft: 12, scrollTop: 40 }, + ]); + expect(scrollIntoViewCalls).toBe(0); expect(editor.getState().selections).toEqual([ { start: { line: 5, character: 0 }, diff --git a/packages/diffs/test/virtualizedEditorViewport.test.ts b/packages/diffs/test/virtualizedEditorViewport.test.ts index e3afd0014..a463d2e91 100644 --- a/packages/diffs/test/virtualizedEditorViewport.test.ts +++ b/packages/diffs/test/virtualizedEditorViewport.test.ts @@ -2,8 +2,102 @@ import { describe, expect, test } from 'bun:test'; import { VirtualizedFile } from '../src/components/VirtualizedFile'; import { VirtualizedFileDiff } from '../src/components/VirtualizedFileDiff'; +import { DEFAULT_THEMES } from '../src/constants'; +import type { DiffsEditableComponent } from '../src/types'; +import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { installDom } from './domHarness'; +interface EditorViewState { + scrollLeft?: number; + scrollTop?: number; +} + +interface EditorViewStateOperations { + captureEditorViewState(): EditorViewState | undefined; + restoreEditorViewState(view: EditorViewState): void; +} + +function viewStateOperations( + component: DiffsEditableComponent +): EditorViewStateOperations { + return component as DiffsEditableComponent & + EditorViewStateOperations; +} + +function createSimpleVirtualizer(root: HTMLElement, scrollTop: number) { + const virtualizer = { + type: 'simple', + config: {}, + connect() {}, + disconnect() {}, + getRoot: () => root, + getScrollTop: () => scrollTop, + getWindowSpecs: () => ({ top: 0, bottom: 800 }), + getOffsetInScrollContainer: () => 0, + instanceChanged() {}, + isInstanceVisible: () => true, + markDOMDirty() {}, + requestHeightReconcile() {}, + } as never; + return virtualizer; +} + +function createAdvancedVirtualizer(root: HTMLElement, scrollTop: number) { + const scrollCalls: unknown[] = []; + const codeView = { + type: 'advanced', + getContainerElement: () => root, + getScrollTop: () => scrollTop, + scrollTo(target: unknown) { + scrollCalls.push(target); + }, + } as never; + return { codeView, scrollCalls }; +} + +function renderFile( + file: VirtualizedFile, + root: HTMLElement +): HTMLElement { + const fileContainer = document.createElement('div'); + root.appendChild(fileContainer); + file.render({ + file: { name: 'state.ts', contents: 'alpha\nbravo\n' }, + fileContainer, + forceRender: true, + }); + const code = fileContainer.shadowRoot?.querySelector('[data-code]'); + expect(code).toBeInstanceOf(HTMLElement); + return code as HTMLElement; +} + +function renderFileDiff( + fileDiff: VirtualizedFileDiff, + root: HTMLElement +): { additions: HTMLElement; deletions?: HTMLElement } { + const fileContainer = document.createElement('div'); + root.appendChild(fileContainer); + fileDiff.render({ + fileDiff: parseDiffFromFile( + { name: 'state.ts', contents: 'alpha\nbravo\n' }, + { name: 'state.ts', contents: 'alpha\ncharlie\n' } + ), + fileContainer, + forceRender: true, + }); + const additions = fileContainer.shadowRoot?.querySelector( + '[data-code]:not([data-deletions])' + ); + const deletions = fileContainer.shadowRoot?.querySelector( + '[data-code][data-deletions]' + ); + expect(additions).toBeInstanceOf(HTMLElement); + return { + additions: additions as HTMLElement, + deletions: deletions instanceof HTMLElement ? deletions : undefined, + }; +} + describe('virtualized editor viewport', () => { test('uses the simple Virtualizer root', () => { const dom = installDom(); @@ -44,4 +138,141 @@ describe('virtualized editor viewport', () => { dom.cleanup(); } }); + + test('captures File horizontal state and simple Virtualizer vertical state', () => { + const dom = installDom(); + const root = document.createElement('div'); + root.scrollLeft = 900; + root.scrollTop = 800; + const virtualizer = createSimpleVirtualizer(root, 700); + const file = new VirtualizedFile( + { disableFileHeader: true, theme: DEFAULT_THEMES }, + virtualizer + ); + + try { + const code = renderFile(file, root); + code.scrollLeft = 60; + + expect(viewStateOperations(file).captureEditorViewState()).toEqual({ + scrollLeft: 60, + scrollTop: 700, + }); + expect(file.getEditorViewport()).toBe(root); + } finally { + file.cleanUp(); + dom.cleanup(); + } + }); + + for (const diffStyle of ['unified', 'split'] as const) { + test(`captures ${diffStyle} FileDiff state from the additions code column`, () => { + const dom = installDom(); + const root = document.createElement('div'); + root.scrollLeft = 900; + root.scrollTop = 800; + const virtualizer = createSimpleVirtualizer(root, 700); + const fileDiff = new VirtualizedFileDiff( + { + diffStyle, + disableFileHeader: true, + theme: DEFAULT_THEMES, + }, + virtualizer + ); + + try { + const code = renderFileDiff(fileDiff, root); + code.additions.scrollLeft = 60; + if (code.deletions !== undefined) { + code.deletions.scrollLeft = 40; + } + + expect(viewStateOperations(fileDiff).captureEditorViewState()).toEqual({ + scrollLeft: 60, + scrollTop: 700, + }); + expect(fileDiff.getEditorViewport()).toBe(root); + } finally { + fileDiff.cleanUp(); + dom.cleanup(); + } + }); + } + + test('restores split FileDiff horizontal state to both code columns', () => { + const dom = installDom(); + const root = document.createElement('div'); + const virtualizer = createSimpleVirtualizer(root, 0); + const fileDiff = new VirtualizedFileDiff( + { + diffStyle: 'split', + disableFileHeader: true, + theme: DEFAULT_THEMES, + }, + virtualizer + ); + + try { + const code = renderFileDiff(fileDiff, root); + expect(code.deletions).toBeDefined(); + + viewStateOperations(fileDiff).restoreEditorViewState({ + scrollLeft: 60, + }); + + expect(code.additions.scrollLeft).toBe(60); + expect(code.deletions?.scrollLeft).toBe(60); + } finally { + fileDiff.cleanUp(); + dom.cleanup(); + } + }); + + test('uses CodeView logical scrolling without changing viewport geometry', () => { + const dom = installDom(); + const root = document.createElement('div'); + root.scrollTop = 200; + const { codeView, scrollCalls } = createAdvancedVirtualizer( + root, + 11_100_000 + ); + const file = new VirtualizedFile({}, codeView); + const fileDiff = new VirtualizedFileDiff({}, codeView); + + try { + expect(viewStateOperations(file).captureEditorViewState()).toEqual({ + scrollTop: 11_100_000, + }); + expect(viewStateOperations(fileDiff).captureEditorViewState()).toEqual({ + scrollTop: 11_100_000, + }); + + viewStateOperations(file).restoreEditorViewState({ + scrollTop: 11_200_000, + }); + viewStateOperations(fileDiff).restoreEditorViewState({ + scrollTop: 11_300_000, + }); + + expect(scrollCalls).toEqual([ + { + type: 'position', + position: 11_200_000, + behavior: 'instant', + }, + { + type: 'position', + position: 11_300_000, + behavior: 'instant', + }, + ]); + expect(file.getEditorViewport()).toBe(root); + expect(fileDiff.getEditorViewport()).toBe(root); + } finally { + file.cleanUp(); + fileDiff.cleanUp(); + dom.cleanup(); + } + }); }); From 6f1398342804c07ad696723b424cbea918c6a031 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Wed, 22 Jul 2026 16:23:30 -0700 Subject: [PATCH 09/12] feat(diffs): Expose virtualizer scroll state Allow editor surfaces to read and restore logical vertical positions without reaching into the virtualizer root. Support element and document roots with a browser-style scroll operation while preserving cached DOM reads. --- packages/diffs/src/components/Virtualizer.ts | 50 +++++--- .../test/Virtualizer.scrollState.test.ts | 118 ++++++++++++++++++ 2 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 packages/diffs/test/Virtualizer.scrollState.test.ts diff --git a/packages/diffs/src/components/Virtualizer.ts b/packages/diffs/src/components/Virtualizer.ts index 9f9d0890d..acfc25d24 100644 --- a/packages/diffs/src/components/Virtualizer.ts +++ b/packages/diffs/src/components/Virtualizer.ts @@ -35,6 +35,11 @@ export interface VirtualizerConfig { resizeDebugging: boolean; } +export interface VirtualizerScrollOptions { + top: number; + behavior?: ScrollBehavior; +} + const DEFAULT_VIRTUALIZER_CONFIG: VirtualizerConfig = { overscrollSize: DEFAULT_OVERSCROLL_SIZE, intersectionObserverMargin: INTERSECTION_OBSERVER_MARGIN, @@ -260,6 +265,9 @@ export class Virtualizer { this.scrollTop = 0; this.height = 0; this.scrollHeight = 0; + this.scrollDirty = true; + this.heightDirty = true; + this.scrollHeightDirty = true; } getOffsetInScrollContainer(element: HTMLElement): number { @@ -342,18 +350,6 @@ export class Virtualizer { return; } const wrapperDirty = this.heightDirty || this.scrollHeightDirty; - if ( - !this.scrollDirty && - !this.scrollHeightDirty && - !this.heightDirty && - this.renderedObservers === this.observers.size && - !this.visibleInstancesDirty && - this.instancesChanged.size === 0 && - this.reconcileQueue.size === 0 - ) { - // NOTE(amadeus): Is this a safe assumption/optimization? - return; - } let instancesHaveChanged = this.instancesChanged.size > 0; // If we got an emitted update from a bunch of instances, we should skip @@ -620,7 +616,8 @@ export class Virtualizer { // ); }; - private getScrollTop() { + /** Return the logical vertical position for the scroll container */ + public getScrollTop(): number { if (!this.scrollDirty) { return this.scrollTop; } @@ -637,12 +634,33 @@ export class Virtualizer { // Lets always make sure to clamp scroll position cases of // over/bounce scroll - scrollTop = Math.max( + scrollTop = this.clampScrollTop(scrollTop); + this.scrollTop = scrollTop; + return scrollTop; + } + + /** Scroll to a logical vertical position and queue the normal render pass. */ + public scrollTo({ top, behavior = 'auto' }: VirtualizerScrollOptions): void { + const { root } = this; + if (root == null) { + return; + } + + const scrollTop = this.clampScrollTop(top); + if (root instanceof Document) { + window.scrollTo({ top: scrollTop, behavior }); + } else { + root.scrollTo({ top: scrollTop, behavior }); + } + this.scrollDirty = true; + queueRender(this.computeRenderRangeAndEmit); + } + + private clampScrollTop(scrollTop: number): number { + return Math.max( 0, Math.min(scrollTop, this.getScrollHeight() - this.getHeight()) ); - this.scrollTop = scrollTop; - return scrollTop; } private getScrollHeight() { diff --git a/packages/diffs/test/Virtualizer.scrollState.test.ts b/packages/diffs/test/Virtualizer.scrollState.test.ts new file mode 100644 index 000000000..9266a61d1 --- /dev/null +++ b/packages/diffs/test/Virtualizer.scrollState.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, test } from 'bun:test'; + +import { Virtualizer } from '../src/components/Virtualizer'; +import { createRoot, installDom, wait } from './domHarness'; + +describe('Virtualizer scroll state', () => { + test('reads and restores an element root logical scroll position', async () => { + const dom = installDom(); + const root = createRoot(); + const content = document.createElement('div'); + root.appendChild(content); + Object.defineProperty(root, 'scrollHeight', { + configurable: true, + value: 5_000, + }); + root.scrollTop = 1_200; + const scrollCalls: ScrollToOptions[] = []; + root.scrollTo = (options?: ScrollToOptions | number, y?: number) => { + const top = + typeof options === 'number' + ? (y ?? root.scrollTop) + : (options?.top ?? root.scrollTop); + root.scrollTop = top; + scrollCalls.push({ + behavior: typeof options === 'number' ? undefined : options?.behavior, + top, + }); + }; + const virtualizer = new Virtualizer(); + + try { + virtualizer.setup(root, content); + await wait(0); + expect(virtualizer.getScrollTop()).toBe(1_200); + + virtualizer.scrollTo({ top: 2_500, behavior: 'instant' }); + expect(virtualizer.getScrollTop()).toBe(2_500); + expect(scrollCalls.at(-1)).toEqual({ + behavior: 'instant', + top: 2_500, + }); + await wait(0); + expect(virtualizer.getWindowSpecs().top).toBe(1_500); + + virtualizer.scrollTo({ top: 10_000 }); + expect(virtualizer.getScrollTop()).toBe(4_200); + expect(scrollCalls.at(-1)).toEqual({ behavior: 'auto', top: 4_200 }); + virtualizer.scrollTo({ top: -100 }); + expect(virtualizer.getScrollTop()).toBe(0); + } finally { + virtualizer.cleanUp(); + dom.cleanup(); + } + }); + + test('reads and restores a document root logical scroll position', async () => { + const dom = installDom(); + const originalInnerHeight = Object.getOwnPropertyDescriptor( + globalThis, + 'innerHeight' + ); + Object.defineProperty(globalThis, 'innerHeight', { + configurable: true, + value: 800, + }); + Object.defineProperty(document.documentElement, 'scrollHeight', { + configurable: true, + value: 5_000, + }); + let scrollY = 1_200; + Object.defineProperty(dom.window, 'scrollY', { + configurable: true, + get: () => scrollY, + }); + const scrollCalls: ScrollToOptions[] = []; + dom.window.scrollTo = (options?: ScrollToOptions | number, y?: number) => { + const top = + typeof options === 'number' + ? (y ?? scrollY) + : (options?.top ?? scrollY); + scrollY = top; + scrollCalls.push({ + behavior: typeof options === 'number' ? undefined : options?.behavior, + top, + }); + }; + const virtualizer = new Virtualizer(); + + try { + virtualizer.setup(document); + await wait(0); + expect(virtualizer.getScrollTop()).toBe(1_200); + + virtualizer.scrollTo({ top: 2_500, behavior: 'instant' }); + expect(virtualizer.getScrollTop()).toBe(2_500); + expect(scrollCalls.at(-1)).toEqual({ + behavior: 'instant', + top: 2_500, + }); + await wait(0); + expect(virtualizer.getWindowSpecs().top).toBe(1_500); + + virtualizer.scrollTo({ top: 10_000 }); + expect(virtualizer.getScrollTop()).toBe(4_200); + expect(scrollCalls.at(-1)).toEqual({ behavior: 'auto', top: 4_200 }); + virtualizer.scrollTo({ top: -100 }); + expect(virtualizer.getScrollTop()).toBe(0); + } finally { + virtualizer.cleanUp(); + if (originalInnerHeight === undefined) { + Reflect.deleteProperty(globalThis, 'innerHeight'); + } else { + Object.defineProperty(globalThis, 'innerHeight', originalInnerHeight); + } + dom.cleanup(); + } + }); +}); From aae6fcd6cfedf45d7cfc41269d7e7b8b74620d5c Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Wed, 22 Jul 2026 17:28:10 -0700 Subject: [PATCH 10/12] feat(diffs): Expose component scroll ownership Expose code scrollLeft through editable components and let virtualized variants return the Virtualizer or CodeView that owns vertical scrolling. Capture horizontal state before teardown clears the code elements. --- packages/diffs/src/components/File.ts | 17 +- packages/diffs/src/components/FileDiff.ts | 28 ++- .../diffs/src/components/VirtualizedFile.ts | 4 + .../src/components/VirtualizedFileDiff.ts | 4 + packages/diffs/src/types.ts | 9 + packages/diffs/test/editorClipboard.test.ts | 6 + packages/diffs/test/editorRecycle.test.ts | 6 + packages/diffs/test/editorState.test.ts | 42 ++-- .../test/editorVirtualizedReveal.test.ts | 6 + .../test/virtualizedEditorViewport.test.ts | 190 +++++++----------- 10 files changed, 176 insertions(+), 136 deletions(-) diff --git a/packages/diffs/src/components/File.ts b/packages/diffs/src/components/File.ts index 72bd16cee..1ff627508 100644 --- a/packages/diffs/src/components/File.ts +++ b/packages/diffs/src/components/File.ts @@ -300,6 +300,16 @@ export class File< }); } + public getCodeScrollLeft(): number { + return this.code?.scrollLeft ?? 0; + } + + public setCodeScrollLeft(position: number): void { + if (this.code != null) { + this.code.scrollLeft = position; + } + } + public flushManagers(): void { if (!this.managersDirty || this.pre == null) { this.managersDirty = false; @@ -323,6 +333,9 @@ export class File< public cleanUp(recycle = false): void { this.emitPostRender(true); + // Persist editor state while the code scroller still exists. + this.editor?.cleanUp(recycle); + this.editor = undefined; this.resizeManager.cleanUp(); this.interactionManager.cleanUp(); this.managersDirty = false; @@ -373,11 +386,7 @@ export class File< this.workerManager = undefined; this.file = undefined; } - this.enabled = false; - - this.editor?.cleanUp(recycle); - this.editor = undefined; } public virtualizedSetup(): void { diff --git a/packages/diffs/src/components/FileDiff.ts b/packages/diffs/src/components/FileDiff.ts index c11c5c003..fe07e8e1d 100644 --- a/packages/diffs/src/components/FileDiff.ts +++ b/packages/diffs/src/components/FileDiff.ts @@ -609,9 +609,32 @@ export class FileDiff< ); } + public getCodeScrollLeft(): number { + return Math.max( + this.codeUnified?.scrollLeft ?? 0, + this.codeDeletions?.scrollLeft ?? 0, + this.codeAdditions?.scrollLeft ?? 0 + ); + } + + public setCodeScrollLeft(position: number): void { + if (this.codeUnified != null) { + this.codeUnified.scrollLeft = position; + } + if (this.codeAdditions != null) { + this.codeAdditions.scrollLeft = position; + } + if (this.codeDeletions != null) { + this.codeDeletions.scrollLeft = position; + } + } + public cleanUp(recycle: boolean = false): void { dequeueRender(this.handleEditSessionRender); this.emitPostRender(true); + // Persist editor state while the code scrollers still exist. + this.editor?.cleanUp(recycle); + this.editor = undefined; this.resizeManager.cleanUp(); this.interactionManager.cleanUp(); this.scrollSyncManager.cleanUp(); @@ -672,10 +695,6 @@ export class FileDiff< this.additionFile = undefined; } - this.enabled = false; - - this.editor?.cleanUp(recycle); - this.editor = undefined; if (this.refreshViewTimeout != null) { clearTimeout(this.refreshViewTimeout); this.refreshViewTimeout = undefined; @@ -683,6 +702,7 @@ export class FileDiff< this.lineStateRefreshPending = false; this.deferredEditorActiveLine = undefined; this.deferredSelectedLines = undefined; + this.enabled = false; } public virtualizedSetup(): void { diff --git a/packages/diffs/src/components/VirtualizedFile.ts b/packages/diffs/src/components/VirtualizedFile.ts index 8df0cec20..22a915487 100644 --- a/packages/diffs/src/components/VirtualizedFile.ts +++ b/packages/diffs/src/components/VirtualizedFile.ts @@ -399,6 +399,10 @@ export class VirtualizedFile< return root instanceof HTMLElement ? root : root?.documentElement; } + public __getVirtualizer(): Virtualizer | CodeView { + return this.virtualizer; + } + public getEditorViewport(): HTMLElement | Document | undefined { return this.virtualizer.type === 'simple' ? this.virtualizer.getRoot() diff --git a/packages/diffs/src/components/VirtualizedFileDiff.ts b/packages/diffs/src/components/VirtualizedFileDiff.ts index bf865b39b..9411a328b 100644 --- a/packages/diffs/src/components/VirtualizedFileDiff.ts +++ b/packages/diffs/src/components/VirtualizedFileDiff.ts @@ -617,6 +617,10 @@ export class VirtualizedFileDiff< return root instanceof HTMLElement ? root : root?.documentElement; } + public __getVirtualizer(): Virtualizer | CodeView { + return this.virtualizer; + } + public getEditorViewport(): HTMLElement | Document | undefined { return this.virtualizer.type === 'simple' ? this.virtualizer.getRoot() diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index ee7324e4a..8b3b9f81b 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -13,6 +13,9 @@ import type { ThemeRegistrationResolved, } from 'shiki'; +import type { CodeView } from './components/CodeView'; +import type { Virtualizer } from './components/Virtualizer'; + export type { CreatePatchOptionsNonabortable }; export type CodeViewScrollBehavior = 'instant' | 'smooth' | 'smooth-auto'; @@ -1033,6 +1036,12 @@ export interface DiffsEditableComponent< lineNumber: number | null, options?: EditorActiveLineOptions ) => void; + /** Return the horizontal code scroll position (`scrollLeft`). */ + getCodeScrollLeft: () => number; + /** Set the horizontal code scroll position (`scrollLeft`). */ + setCodeScrollLeft: (position: number) => void; + /** @internal Return the owner of logical vertical scrolling, when present. */ + __getVirtualizer?: () => Virtualizer | CodeView; /** * Return the position and height of a one-based line relative to this component. * The host uses it to scroll to virtualized lines before their DOM nodes exist. diff --git a/packages/diffs/test/editorClipboard.test.ts b/packages/diffs/test/editorClipboard.test.ts index 06f267249..70cfabb83 100644 --- a/packages/diffs/test/editorClipboard.test.ts +++ b/packages/diffs/test/editorClipboard.test.ts @@ -143,6 +143,12 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} + getCodeScrollLeft(): number { + return 0; + } + + setCodeScrollLeft(): void {} + render({ file, lineAnnotations, diff --git a/packages/diffs/test/editorRecycle.test.ts b/packages/diffs/test/editorRecycle.test.ts index c9da4e7ed..3f23791d2 100644 --- a/packages/diffs/test/editorRecycle.test.ts +++ b/packages/diffs/test/editorRecycle.test.ts @@ -75,6 +75,12 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} + getCodeScrollLeft(): number { + return 0; + } + + setCodeScrollLeft(): void {} + render({ file, lineAnnotations, diff --git a/packages/diffs/test/editorState.test.ts b/packages/diffs/test/editorState.test.ts index 8593d7377..828e21dd2 100644 --- a/packages/diffs/test/editorState.test.ts +++ b/packages/diffs/test/editorState.test.ts @@ -1,12 +1,12 @@ import { describe, expect, test } from 'bun:test'; +import type { Virtualizer } from '../src/components/Virtualizer'; import { Editor } from '../src/editor/editor'; import type { DiffLineAnnotation, DiffsEditableComponent, DiffsEditor, DiffsHighlighter, - EditorState, FileContents, HighlightedToken, RenderRange, @@ -35,8 +35,17 @@ class TestEditableComponent implements DiffsEditableComponent { #editor?: DiffsEditor; #lineAnnotations?: DiffLineAnnotation[]; #renderRange?: RenderRange; - capturedView: EditorState['view']; - restoredViews: NonNullable[] = []; + codeScrollPosition = 0; + virtualizerScrollTop = 0; + restoredCodeScrollPositions: number[] = []; + restoredVirtualizerScrollPositions: unknown[] = []; + readonly virtualizer = { + type: 'simple', + getScrollTop: () => this.virtualizerScrollTop, + scrollTo: (target: unknown) => { + this.restoredVirtualizerScrollPositions.push(target); + }, + } as unknown as Virtualizer; constructor(private file: FileContents) { this.#renderShadowDom(); @@ -50,13 +59,17 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} - captureEditorViewState(): EditorState['view'] { - return this.capturedView == null ? undefined : { ...this.capturedView }; + getCodeScrollLeft(): number { + return this.codeScrollPosition; } - restoreEditorViewState(view: NonNullable): void { - this.capturedView = { ...view }; - this.restoredViews.push({ ...view }); + setCodeScrollLeft(position: number): void { + this.codeScrollPosition = position; + this.restoredCodeScrollPositions.push(position); + } + + __getVirtualizer(): Virtualizer { + return this.virtualizer; } render({ @@ -153,7 +166,8 @@ describe('Editor state', () => { try { editor.edit(component); - component.capturedView = { scrollLeft: 24, scrollTop: 128 }; + component.codeScrollPosition = 24; + component.virtualizerScrollTop = 128; expect(editor.getState().view).toEqual({ scrollLeft: 24, @@ -178,8 +192,9 @@ describe('Editor state', () => { editor.edit(component); editor.setState({ view: { scrollLeft: 24, scrollTop: 128 } }); - expect(component.restoredViews).toEqual([ - { scrollLeft: 24, scrollTop: 128 }, + expect(component.restoredCodeScrollPositions).toEqual([24]); + expect(component.restoredVirtualizerScrollPositions).toEqual([ + { top: 128, behavior: 'instant' }, ]); } finally { editor.cleanUp(); @@ -219,8 +234,9 @@ describe('Editor state', () => { view: { scrollLeft: 12, scrollTop: 40 }, }); - expect(component.restoredViews).toEqual([ - { scrollLeft: 12, scrollTop: 40 }, + expect(component.restoredCodeScrollPositions).toEqual([12]); + expect(component.restoredVirtualizerScrollPositions).toEqual([ + { top: 40, behavior: 'instant' }, ]); expect(scrollIntoViewCalls).toBe(0); expect(editor.getState().selections).toEqual([ diff --git a/packages/diffs/test/editorVirtualizedReveal.test.ts b/packages/diffs/test/editorVirtualizedReveal.test.ts index d9aa96290..69c9fa5a8 100644 --- a/packages/diffs/test/editorVirtualizedReveal.test.ts +++ b/packages/diffs/test/editorVirtualizedReveal.test.ts @@ -61,6 +61,12 @@ class VirtualizedEditableComponent implements DiffsEditableComponent setEditorActiveLine(_lineNumber: number | null): void {} + getCodeScrollLeft(): number { + return 0; + } + + setCodeScrollLeft(): void {} + render(): void { this.rerender(); } diff --git a/packages/diffs/test/virtualizedEditorViewport.test.ts b/packages/diffs/test/virtualizedEditorViewport.test.ts index a463d2e91..6fae6ab87 100644 --- a/packages/diffs/test/virtualizedEditorViewport.test.ts +++ b/packages/diffs/test/virtualizedEditorViewport.test.ts @@ -3,88 +3,71 @@ import { describe, expect, test } from 'bun:test'; import { VirtualizedFile } from '../src/components/VirtualizedFile'; import { VirtualizedFileDiff } from '../src/components/VirtualizedFileDiff'; import { DEFAULT_THEMES } from '../src/constants'; -import type { DiffsEditableComponent } from '../src/types'; +import type { DiffsEditor } from '../src/types'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; -import { installDom } from './domHarness'; +import { installDom, waitFor } from './domHarness'; -interface EditorViewState { - scrollLeft?: number; - scrollTop?: number; -} - -interface EditorViewStateOperations { - captureEditorViewState(): EditorViewState | undefined; - restoreEditorViewState(view: EditorViewState): void; -} - -function viewStateOperations( - component: DiffsEditableComponent -): EditorViewStateOperations { - return component as DiffsEditableComponent & - EditorViewStateOperations; -} - -function createSimpleVirtualizer(root: HTMLElement, scrollTop: number) { - const virtualizer = { +function createSimpleVirtualizer(root: HTMLElement) { + return { type: 'simple', config: {}, connect() {}, disconnect() {}, getRoot: () => root, - getScrollTop: () => scrollTop, + getScrollTop: () => 0, getWindowSpecs: () => ({ top: 0, bottom: 800 }), getOffsetInScrollContainer: () => 0, - instanceChanged() {}, + instanceChanged(instance: { onRender(dirty: boolean): boolean }) { + instance.onRender(true); + }, isInstanceVisible: () => true, markDOMDirty() {}, requestHeightReconcile() {}, + scrollTo() {}, } as never; - return virtualizer; -} - -function createAdvancedVirtualizer(root: HTMLElement, scrollTop: number) { - const scrollCalls: unknown[] = []; - const codeView = { - type: 'advanced', - getContainerElement: () => root, - getScrollTop: () => scrollTop, - scrollTo(target: unknown) { - scrollCalls.push(target); - }, - } as never; - return { codeView, scrollCalls }; } -function renderFile( +async function renderFile( file: VirtualizedFile, root: HTMLElement -): HTMLElement { +): Promise { const fileContainer = document.createElement('div'); root.appendChild(fileContainer); file.render({ - file: { name: 'state.ts', contents: 'alpha\nbravo\n' }, + file: { name: 'state.txt', contents: 'alpha\nbravo\n', lang: 'text' }, fileContainer, forceRender: true, }); + await waitFor( + () => + fileContainer.shadowRoot?.querySelector('[data-code]') instanceof + HTMLElement + ); const code = fileContainer.shadowRoot?.querySelector('[data-code]'); expect(code).toBeInstanceOf(HTMLElement); return code as HTMLElement; } -function renderFileDiff( +async function renderFileDiff( fileDiff: VirtualizedFileDiff, root: HTMLElement -): { additions: HTMLElement; deletions?: HTMLElement } { +): Promise<{ additions: HTMLElement; deletions?: HTMLElement }> { const fileContainer = document.createElement('div'); root.appendChild(fileContainer); fileDiff.render({ fileDiff: parseDiffFromFile( - { name: 'state.ts', contents: 'alpha\nbravo\n' }, - { name: 'state.ts', contents: 'alpha\ncharlie\n' } + { name: 'state.txt', contents: 'alpha\nbravo\n', lang: 'text' }, + { name: 'state.txt', contents: 'alpha\ncharlie\n', lang: 'text' } ), fileContainer, forceRender: true, }); + await waitFor( + () => + fileContainer.shadowRoot?.querySelector( + '[data-code]:not([data-deletions])' + ) instanceof HTMLElement + ); const additions = fileContainer.shadowRoot?.querySelector( '[data-code]:not([data-deletions])' ); @@ -112,6 +95,8 @@ describe('virtualized editor viewport', () => { try { expect(file.getEditorViewport()).toBe(root); expect(fileDiff.getEditorViewport()).toBe(root); + expect(file.__getVirtualizer()).toBe(virtualizer); + expect(fileDiff.__getVirtualizer()).toBe(virtualizer); } finally { file.cleanUp(); fileDiff.cleanUp(); @@ -132,6 +117,8 @@ describe('virtualized editor viewport', () => { try { expect(file.getEditorViewport()).toBe(root); expect(fileDiff.getEditorViewport()).toBe(root); + expect(file.__getVirtualizer()).toBe(codeView); + expect(fileDiff.__getVirtualizer()).toBe(codeView); } finally { file.cleanUp(); fileDiff.cleanUp(); @@ -139,26 +126,37 @@ describe('virtualized editor viewport', () => { } }); - test('captures File horizontal state and simple Virtualizer vertical state', () => { + test('reads and restores File horizontal state from the code scroller', async () => { const dom = installDom(); const root = document.createElement('div'); root.scrollLeft = 900; - root.scrollTop = 800; - const virtualizer = createSimpleVirtualizer(root, 700); + const virtualizer = createSimpleVirtualizer(root); const file = new VirtualizedFile( { disableFileHeader: true, theme: DEFAULT_THEMES }, virtualizer ); try { - const code = renderFile(file, root); + const code = await renderFile(file, root); code.scrollLeft = 60; - expect(viewStateOperations(file).captureEditorViewState()).toEqual({ - scrollLeft: 60, - scrollTop: 700, - }); - expect(file.getEditorViewport()).toBe(root); + expect(file.getCodeScrollLeft()).toBe(60); + file.setCodeScrollLeft(61); + expect(code.scrollLeft).toBe(61); + + let recyclePosition: number | undefined; + file.attachEditor({ + __captureFocusForDOMReplacement() {}, + __postponeBgTokenizeToNextFrame() {}, + __syncRenderView() {}, + edit: () => () => {}, + cleanUp() { + recyclePosition = file.getCodeScrollLeft(); + }, + } as DiffsEditor); + code.scrollLeft = 62; + file.cleanUp(true); + expect(recyclePosition).toBe(62); } finally { file.cleanUp(); dom.cleanup(); @@ -166,12 +164,11 @@ describe('virtualized editor viewport', () => { }); for (const diffStyle of ['unified', 'split'] as const) { - test(`captures ${diffStyle} FileDiff state from the additions code column`, () => { + test(`reads ${diffStyle} FileDiff horizontal state from active code columns`, async () => { const dom = installDom(); const root = document.createElement('div'); root.scrollLeft = 900; - root.scrollTop = 800; - const virtualizer = createSimpleVirtualizer(root, 700); + const virtualizer = createSimpleVirtualizer(root); const fileDiff = new VirtualizedFileDiff( { diffStyle, @@ -182,17 +179,15 @@ describe('virtualized editor viewport', () => { ); try { - const code = renderFileDiff(fileDiff, root); - code.additions.scrollLeft = 60; + const code = await renderFileDiff(fileDiff, root); + code.additions.scrollLeft = 40; if (code.deletions !== undefined) { - code.deletions.scrollLeft = 40; + code.deletions.scrollLeft = 60; } - expect(viewStateOperations(fileDiff).captureEditorViewState()).toEqual({ - scrollLeft: 60, - scrollTop: 700, - }); - expect(fileDiff.getEditorViewport()).toBe(root); + expect(fileDiff.getCodeScrollLeft()).toBe( + diffStyle === 'split' ? 60 : 40 + ); } finally { fileDiff.cleanUp(); dom.cleanup(); @@ -200,10 +195,10 @@ describe('virtualized editor viewport', () => { }); } - test('restores split FileDiff horizontal state to both code columns', () => { + test('restores split FileDiff horizontal state to both code columns', async () => { const dom = installDom(); const root = document.createElement('div'); - const virtualizer = createSimpleVirtualizer(root, 0); + const virtualizer = createSimpleVirtualizer(root); const fileDiff = new VirtualizedFileDiff( { diffStyle: 'split', @@ -214,63 +209,28 @@ describe('virtualized editor viewport', () => { ); try { - const code = renderFileDiff(fileDiff, root); + const code = await renderFileDiff(fileDiff, root); expect(code.deletions).toBeDefined(); - viewStateOperations(fileDiff).restoreEditorViewState({ - scrollLeft: 60, - }); + fileDiff.setCodeScrollLeft(60); expect(code.additions.scrollLeft).toBe(60); expect(code.deletions?.scrollLeft).toBe(60); - } finally { - fileDiff.cleanUp(); - dom.cleanup(); - } - }); - - test('uses CodeView logical scrolling without changing viewport geometry', () => { - const dom = installDom(); - const root = document.createElement('div'); - root.scrollTop = 200; - const { codeView, scrollCalls } = createAdvancedVirtualizer( - root, - 11_100_000 - ); - const file = new VirtualizedFile({}, codeView); - const fileDiff = new VirtualizedFileDiff({}, codeView); - try { - expect(viewStateOperations(file).captureEditorViewState()).toEqual({ - scrollTop: 11_100_000, - }); - expect(viewStateOperations(fileDiff).captureEditorViewState()).toEqual({ - scrollTop: 11_100_000, - }); - - viewStateOperations(file).restoreEditorViewState({ - scrollTop: 11_200_000, - }); - viewStateOperations(fileDiff).restoreEditorViewState({ - scrollTop: 11_300_000, - }); - - expect(scrollCalls).toEqual([ - { - type: 'position', - position: 11_200_000, - behavior: 'instant', + let recyclePosition: number | undefined; + fileDiff.attachEditor({ + __captureFocusForDOMReplacement() {}, + __postponeBgTokenizeToNextFrame() {}, + __syncRenderView() {}, + edit: () => () => {}, + cleanUp() { + recyclePosition = fileDiff.getCodeScrollLeft(); }, - { - type: 'position', - position: 11_300_000, - behavior: 'instant', - }, - ]); - expect(file.getEditorViewport()).toBe(root); - expect(fileDiff.getEditorViewport()).toBe(root); + } as DiffsEditor); + code.deletions!.scrollLeft = 64; + fileDiff.cleanUp(true); + expect(recyclePosition).toBe(64); } finally { - file.cleanUp(); fileDiff.cleanUp(); dom.cleanup(); } From be12fcd29c412caacd6bf75dd7834b20970e3945 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Wed, 22 Jul 2026 17:48:18 -0700 Subject: [PATCH 11/12] feat(diffs): Compose editor scroll state Capture horizontal scrolling from the editable code surface and logical vertical scrolling from its Virtualizer or CodeView. Restore each axis through its owner so rebased CodeView positions remain correct and non-virtualized state omits unsupported vertical state. --- packages/diffs/src/editor/editor.ts | 35 ++++++++---- packages/diffs/src/types.ts | 10 ++-- packages/diffs/test/editorState.test.ts | 73 ++++++++++++++++++++----- 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index 52a9ca321..c5ae91f81 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -592,14 +592,17 @@ export class Editor implements DiffsEditor { } getState(): EditorState { - const scrollContainer = this.#fileInstance?.getScrollContainer?.(); + const fileInstance = this.#fileInstance; + const virtualizer = fileInstance?.__getVirtualizer?.(); return { selections: this.#selections, view: - scrollContainer !== undefined + fileInstance != null ? { - scrollLeft: scrollContainer.scrollLeft, - scrollTop: scrollContainer.scrollTop, + scrollLeft: fileInstance.getCodeScrollLeft(), + ...(virtualizer != null + ? { scrollTop: virtualizer.getScrollTop() } + : null), } : undefined, }; @@ -614,14 +617,22 @@ export class Editor implements DiffsEditor { // When a saved view is present, honor its scroll offsets exactly. Scrolling // the caret into view afterward would overwrite them whenever the caret // sits outside that viewport (e.g. TreeApp remount restore). - if (view !== undefined) { - const scrollContainer = this.#fileInstance.getScrollContainer?.(); - if (scrollContainer !== undefined) { - scrollContainer.scrollTo({ - left: view.scrollLeft, - top: view.scrollTop, - behavior: 'instant', - }); + if (view != null) { + this.#fileInstance.setCodeScrollLeft(view.scrollLeft); + const virtualizer = this.#fileInstance.__getVirtualizer?.(); + if (virtualizer != null && view.scrollTop != null) { + if (virtualizer.type === 'simple') { + virtualizer.scrollTo({ + top: view.scrollTop, + behavior: 'instant', + }); + } else { + virtualizer.scrollTo({ + type: 'position', + position: view.scrollTop, + behavior: 'instant', + }); + } } return; } diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index 8b3b9f81b..0e4d2e3de 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -1226,12 +1226,14 @@ export interface EditorSelection extends Range { direction: SelectionDirection; } +export interface EditorViewState { + scrollLeft: number; + scrollTop?: number; +} + export interface EditorState { selections?: EditorSelection[]; - view?: { - scrollLeft: number; - scrollTop: number; - }; + view?: EditorViewState; } export interface DiffsTextDocument { diff --git a/packages/diffs/test/editorState.test.ts b/packages/diffs/test/editorState.test.ts index 828e21dd2..432ffbca4 100644 --- a/packages/diffs/test/editorState.test.ts +++ b/packages/diffs/test/editorState.test.ts @@ -35,9 +35,9 @@ class TestEditableComponent implements DiffsEditableComponent { #editor?: DiffsEditor; #lineAnnotations?: DiffLineAnnotation[]; #renderRange?: RenderRange; - codeScrollPosition = 0; + codeScrollLeft = 0; virtualizerScrollTop = 0; - restoredCodeScrollPositions: number[] = []; + restoredCodeScrollLefts: number[] = []; restoredVirtualizerScrollPositions: unknown[] = []; readonly virtualizer = { type: 'simple', @@ -47,7 +47,13 @@ class TestEditableComponent implements DiffsEditableComponent { }, } as unknown as Virtualizer; - constructor(private file: FileContents) { + readonly __getVirtualizer: (() => Virtualizer) | undefined; + + constructor( + private file: FileContents, + virtualized = true + ) { + this.__getVirtualizer = virtualized ? () => this.virtualizer : undefined; this.#renderShadowDom(); } @@ -60,16 +66,12 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} getCodeScrollLeft(): number { - return this.codeScrollPosition; + return this.codeScrollLeft; } setCodeScrollLeft(position: number): void { - this.codeScrollPosition = position; - this.restoredCodeScrollPositions.push(position); - } - - __getVirtualizer(): Virtualizer { - return this.virtualizer; + this.codeScrollLeft = position; + this.restoredCodeScrollLefts.push(position); } render({ @@ -156,7 +158,7 @@ class TestEditableComponent implements DiffsEditableComponent { } describe('Editor state', () => { - test('getState captures view state from the editable component', () => { + test('getState composes horizontal and vertical scroll owners', () => { const dom = installDom(); const editor = new Editor(); const component = new TestEditableComponent({ @@ -166,7 +168,7 @@ describe('Editor state', () => { try { editor.edit(component); - component.codeScrollPosition = 24; + component.codeScrollLeft = 24; component.virtualizerScrollTop = 128; expect(editor.getState().view).toEqual({ @@ -180,6 +182,26 @@ describe('Editor state', () => { } }); + test('getState omits vertical state for a non-virtualized component', () => { + const dom = installDom(); + const editor = new Editor(); + const component = new TestEditableComponent( + { name: 'state.ts', contents: 'alpha\nbravo' }, + false + ); + + try { + editor.edit(component); + component.codeScrollLeft = 24; + + expect(editor.getState().view).toEqual({ scrollLeft: 24 }); + } finally { + editor.cleanUp(); + component.cleanUp(); + dom.cleanup(); + } + }); + test('setState restores view state through the editable component', () => { const dom = installDom(); const editor = new Editor(); @@ -192,7 +214,7 @@ describe('Editor state', () => { editor.edit(component); editor.setState({ view: { scrollLeft: 24, scrollTop: 128 } }); - expect(component.restoredCodeScrollPositions).toEqual([24]); + expect(component.restoredCodeScrollLefts).toEqual([24]); expect(component.restoredVirtualizerScrollPositions).toEqual([ { top: 128, behavior: 'instant' }, ]); @@ -203,10 +225,31 @@ describe('Editor state', () => { } }); + test('setState restores horizontal state without an optional vertical position', () => { + const dom = installDom(); + const editor = new Editor(); + const component = new TestEditableComponent({ + name: 'state.ts', + contents: 'alpha\nbravo', + }); + + try { + editor.edit(component); + editor.setState({ view: { scrollLeft: 24 } }); + + expect(component.restoredCodeScrollLefts).toEqual([24]); + expect(component.restoredVirtualizerScrollPositions).toEqual([]); + } finally { + editor.cleanUp(); + component.cleanUp(); + dom.cleanup(); + } + }); + // A remount restore often carries both a viewport and a caret that sits // outside that viewport. The saved view must win; scrolling the caret into // view would overwrite it. jsdom's scrollIntoView is a no-op, so record any - // attempt to reveal the caret after restoring the component-owned view. + // attempt to reveal the caret after restoring the saved logical view. test('setState keeps the saved view when the caret is outside it', () => { const dom = installDom(); let scrollIntoViewCalls = 0; @@ -234,7 +277,7 @@ describe('Editor state', () => { view: { scrollLeft: 12, scrollTop: 40 }, }); - expect(component.restoredCodeScrollPositions).toEqual([12]); + expect(component.restoredCodeScrollLefts).toEqual([12]); expect(component.restoredVirtualizerScrollPositions).toEqual([ { top: 40, behavior: 'instant' }, ]); From 295d44e0b62409423d29931d6b69d4d2e003478e Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Wed, 22 Jul 2026 19:06:42 -0700 Subject: [PATCH 12/12] refactor(diffs): Limit editor state to horizontal scrolling Editor state should follow the editable code surface rather than a shared virtualized viewport. Capturing scrollTop ties per-file persistence to Virtualizer and CodeView and can move the viewer during state restoration. Persist only the code scroller's scrollLeft, remove the vertical owner bridge and obsolete scroll-container accessors, and leave vertical scroll APIs on their virtualizer owners. --- .../docs/app/(diffs)/docs/Editor/constants.ts | 8 +- apps/docs/app/(diffs)/docs/Editor/content.mdx | 10 +- .../diffs/src/components/VirtualizedFile.ts | 9 -- .../src/components/VirtualizedFileDiff.ts | 9 -- packages/diffs/src/editor/editor.ts | 25 +--- packages/diffs/src/types.ts | 11 +- packages/diffs/test/CodeView.edit.test.ts | 12 +- .../test/CodeView.scrollAnchoring.test.ts | 117 ------------------ packages/diffs/test/editorState.test.ts | 81 ++++-------- .../diffs/test/editorVirtualizedEdit.test.ts | 2 +- .../test/virtualizedEditorViewport.test.ts | 6 - 11 files changed, 47 insertions(+), 243 deletions(-) diff --git a/apps/docs/app/(diffs)/docs/Editor/constants.ts b/apps/docs/app/(diffs)/docs/Editor/constants.ts index e865891b5..28d58f7c1 100644 --- a/apps/docs/app/(diffs)/docs/Editor/constants.ts +++ b/apps/docs/app/(diffs)/docs/Editor/constants.ts @@ -943,7 +943,7 @@ interface EditorOptions { // Max undo stack entries historyMaxEntries?: number; - // Preserve each File's document and editor state between renders. + // Preserve each File's document and item-local editor state between renders. // Requires every editable file to provide a unique, stable cacheKey. // Default: false. persistState?: boolean; @@ -1065,14 +1065,14 @@ const file: FileContents | undefined = editor.getFile(); // Full document text, or '' when nothing is attached. const text: string = editor.getText(); -// Snapshot selections and scroll position for persistence or remount restore. +// Snapshot selections and horizontal code position for explicit restoration. const state: EditorState = editor.getState(); // EditorState = { // selections?: EditorSelection[]; -// view?: { scrollLeft: number; scrollTop: number }; +// view?: { scrollLeft: number }; // } -// Restore selections and scroll after re-rendering the underlying component. +// Restore selections and horizontal code position after re-rendering. editor.setState(state); // Replace all cursors and ranges programmatically. Positions are zero-based; diff --git a/apps/docs/app/(diffs)/docs/Editor/content.mdx b/apps/docs/app/(diffs)/docs/Editor/content.mdx index 6175e84de..fcf4a787d 100644 --- a/apps/docs/app/(diffs)/docs/Editor/content.mdx +++ b/apps/docs/app/(diffs)/docs/Editor/content.mdx @@ -319,10 +319,10 @@ Pass these when constructing `new Editor({ ... })`, or update them later with #### Persisting file state State persistence is disabled by default. Set `persistState: true` to preserve -an editable `File`'s text document, undo history, selections, and scroll -position when the attached file renders a different file and later returns. This -currently applies to `File` and `VirtualizedFile` surfaces; diff surfaces do not -use the persistence cache. +an editable `File`'s text document, undo history, selections, and horizontal +code scroll position when the attached file renders a different file and later +returns. This currently applies to `File` and `VirtualizedFile` surfaces; diff +surfaces do not use the persistence cache. Every editable file must provide an explicit, non-empty `file.cacheKey` while `persistState` is enabled. The editor throws if it encounters an unkeyed file; @@ -366,7 +366,7 @@ const editorWithCustomStorage = new Editor({ `persistStateStorage` accepts `'inMemory'`, `'indexedDB'`, or an `IStateStorage` implementation. It defaults to `'inMemory'`. Text documents and undo history remain scoped to the `Editor` instance; IndexedDB and custom storage persist -only the serializable editor state (`selections` and `view`). +only serializable item-local editor state. ### API Reference diff --git a/packages/diffs/src/components/VirtualizedFile.ts b/packages/diffs/src/components/VirtualizedFile.ts index 22a915487..3fecb6ac5 100644 --- a/packages/diffs/src/components/VirtualizedFile.ts +++ b/packages/diffs/src/components/VirtualizedFile.ts @@ -394,15 +394,6 @@ export class VirtualizedFile< }; } - public getScrollContainer(): HTMLElement | undefined { - const root = this.getSimpleVirtualizer()?.getRoot(); - return root instanceof HTMLElement ? root : root?.documentElement; - } - - public __getVirtualizer(): Virtualizer | CodeView { - return this.virtualizer; - } - public getEditorViewport(): HTMLElement | Document | undefined { return this.virtualizer.type === 'simple' ? this.virtualizer.getRoot() diff --git a/packages/diffs/src/components/VirtualizedFileDiff.ts b/packages/diffs/src/components/VirtualizedFileDiff.ts index 9411a328b..8704680dd 100644 --- a/packages/diffs/src/components/VirtualizedFileDiff.ts +++ b/packages/diffs/src/components/VirtualizedFileDiff.ts @@ -612,15 +612,6 @@ export class VirtualizedFileDiff< return position; } - public getScrollContainer(): HTMLElement | undefined { - const root = this.getSimpleVirtualizer()?.getRoot(); - return root instanceof HTMLElement ? root : root?.documentElement; - } - - public __getVirtualizer(): Virtualizer | CodeView { - return this.virtualizer; - } - public getEditorViewport(): HTMLElement | Document | undefined { return this.virtualizer.type === 'simple' ? this.virtualizer.getRoot() diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index c5ae91f81..35f5298d3 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -187,7 +187,7 @@ export interface EditorOptions { /** The maximum number of entries to keep in the undo stack. */ historyMaxEntries?: number; /** - * Preserve each file's document and editor state when switching files. + * Preserve each file's document and item-local editor state when switching files. * Every editable file must provide a unique, stable `cacheKey`. */ persistState?: boolean; @@ -593,16 +593,12 @@ export class Editor implements DiffsEditor { getState(): EditorState { const fileInstance = this.#fileInstance; - const virtualizer = fileInstance?.__getVirtualizer?.(); return { selections: this.#selections, view: fileInstance != null ? { scrollLeft: fileInstance.getCodeScrollLeft(), - ...(virtualizer != null - ? { scrollTop: virtualizer.getScrollTop() } - : null), } : undefined, }; @@ -619,21 +615,6 @@ export class Editor implements DiffsEditor { // sits outside that viewport (e.g. TreeApp remount restore). if (view != null) { this.#fileInstance.setCodeScrollLeft(view.scrollLeft); - const virtualizer = this.#fileInstance.__getVirtualizer?.(); - if (virtualizer != null && view.scrollTop != null) { - if (virtualizer.type === 'simple') { - virtualizer.scrollTo({ - top: view.scrollTop, - behavior: 'instant', - }); - } else { - virtualizer.scrollTo({ - type: 'position', - position: view.scrollTop, - behavior: 'instant', - }); - } - } return; } this.#scrollToPrimaryCaret(); @@ -1235,8 +1216,7 @@ export class Editor implements DiffsEditor { if ( textDocument.version === pendingRestore.documentVersion && this.#selections === pendingRestore.selections && - state.view?.scrollLeft === pendingRestore.view?.scrollLeft && - state.view?.scrollTop === pendingRestore.view?.scrollTop + state.view?.scrollLeft === pendingRestore.view?.scrollLeft ) { return; } @@ -1307,7 +1287,6 @@ export class Editor implements DiffsEditor { textDocument.version !== documentVersion || this.#selections !== selections || currentView?.scrollLeft !== view?.scrollLeft || - currentView?.scrollTop !== view?.scrollTop || this.#fileInfo === undefined || requirePersistedCacheKey(this.#fileInfo) !== cacheKey ) { diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index 0e4d2e3de..29320c28a 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -13,9 +13,6 @@ import type { ThemeRegistrationResolved, } from 'shiki'; -import type { CodeView } from './components/CodeView'; -import type { Virtualizer } from './components/Virtualizer'; - export type { CreatePatchOptionsNonabortable }; export type CodeViewScrollBehavior = 'instant' | 'smooth' | 'smooth-auto'; @@ -1040,8 +1037,6 @@ export interface DiffsEditableComponent< getCodeScrollLeft: () => number; /** Set the horizontal code scroll position (`scrollLeft`). */ setCodeScrollLeft: (position: number) => void; - /** @internal Return the owner of logical vertical scrolling, when present. */ - __getVirtualizer?: () => Virtualizer | CodeView; /** * Return the position and height of a one-based line relative to this component. * The host uses it to scroll to virtualized lines before their DOM nodes exist. @@ -1051,10 +1046,6 @@ export interface DiffsEditableComponent< getLinePosition?: ( lineNumber: number ) => { top: number; height: number } | undefined; - /** - * Return the scroll container element. - */ - getScrollContainer?: () => HTMLElement | undefined; /** * Return an explicit viewport that bounds visible editor rows. Components * without one fall back to their nearest scrollable ancestor or document. @@ -1227,8 +1218,8 @@ export interface EditorSelection extends Range { } export interface EditorViewState { + /** Horizontal position owned by the current editable code scroller. */ scrollLeft: number; - scrollTop?: number; } export interface EditorState { diff --git a/packages/diffs/test/CodeView.edit.test.ts b/packages/diffs/test/CodeView.edit.test.ts index f3112033f..2cf8b8dae 100644 --- a/packages/diffs/test/CodeView.edit.test.ts +++ b/packages/diffs/test/CodeView.edit.test.ts @@ -530,7 +530,7 @@ describe('CodeView item edit mode', () => { } }); - test('recycle restores item-local horizontal state without restoring the shared CodeView scroll position', async () => { + test('recycle restores persisted item-local state without moving CodeView', async () => { const { cleanup } = installDom(); const editors: Editor[] = []; const viewer = new CodeView({ @@ -606,13 +606,19 @@ describe('CodeView item edit mode', () => { HTMLElement ); }); - await wait(10); - const remounted = viewer .getRenderedItems() .find((item) => item.id === edited.id); const remountedCode = remounted?.element.shadowRoot?.querySelector('[data-code]'); + await waitFor(() => { + const selection = editor.getState().selections?.[0]; + return ( + selection?.start.line === 2 && + selection.start.character === 3 && + (remountedCode as HTMLElement | undefined)?.scrollLeft === 64 + ); + }); expect(editors).toHaveLength(1); expect(viewer.getEditor(edited.id)).toBe(editor); expect(editor.getState().selections).toEqual([ diff --git a/packages/diffs/test/CodeView.scrollAnchoring.test.ts b/packages/diffs/test/CodeView.scrollAnchoring.test.ts index 54ecaffca..4a7d8e5d3 100644 --- a/packages/diffs/test/CodeView.scrollAnchoring.test.ts +++ b/packages/diffs/test/CodeView.scrollAnchoring.test.ts @@ -2,7 +2,6 @@ import { describe, expect, test } from 'bun:test'; import { CodeView } from '../src/components/CodeView'; import { DEFAULT_CODE_VIEW_LAYOUT } from '../src/constants'; -import { Editor } from '../src/editor/editor'; import type { CodeViewItem, FileContents } from '../src/types'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { @@ -12,7 +11,6 @@ import { makeFileItem, renderItems, wait, - waitFor, } from './domHarness'; const ROOT_HEIGHT = 800; @@ -105,60 +103,6 @@ function makeReplacementDiffItem( }; } -function makeEditedFileItem(id: string): CodeViewItem { - return { - id, - type: 'file', - file: { - ...makeFile(`${id}.txt`, 1), - cacheKey: id, - lang: 'text', - }, - edit: true, - }; -} - -async function renderRebasedEditedItem( - viewer: CodeView -): Promise<{ code: HTMLElement; editor: Editor }> { - const editedIndex = 12; - const edited = makeEditedFileItem('edited'); - const items = Array.from({ length: 40 }, (_, index) => - index === editedIndex ? edited : makeFileItem(`file:${index}`, 1) - ); - await renderItems(viewer, items); - - viewer.scrollTo({ - type: 'item', - id: edited.id, - align: 'start', - behavior: 'instant', - }); - viewer.render(true); - await waitFor(() => { - const rendered = viewer - .getRenderedItems() - .find((item) => item.id === edited.id); - return ( - viewer.getEditor(edited.id) !== undefined && - rendered?.element.shadowRoot?.querySelector('[data-code]') instanceof - HTMLElement - ); - }); - - const rendered = viewer - .getRenderedItems() - .find((item) => item.id === edited.id); - const code = rendered?.element.shadowRoot?.querySelector('[data-code]'); - const editor = viewer.getEditor(edited.id); - expect(code).toBeInstanceOf(HTMLElement); - expect(editor).toBeInstanceOf(Editor); - return { - code: code as HTMLElement, - editor: editor as Editor, - }; -} - describe('CodeView scroll anchoring', () => { test('keeps an item anchor fixed when split to unified grows past the old scroll range', async () => { const { cleanup } = installDom(); @@ -262,67 +206,6 @@ describe('CodeView scroll anchoring', () => { } }); - test('captures logical editor state after the DOM scroll position rebases', async () => { - const { cleanup } = installDom(); - const viewer = new CodeView({ - createEditor: (options) => new Editor({ ...options }), - layout: { - ...DEFAULT_CODE_VIEW_LAYOUT, - gap: 1_000_000, - }, - }); - const root = createClampingRoot(); - - try { - viewer.setup(root); - const { code, editor } = await renderRebasedEditedItem(viewer); - const logicalScrollTop = viewer.getScrollTop(); - code.scrollLeft = 73; - - expect(logicalScrollTop).toBeGreaterThan(SCROLL_REBASE_THRESHOLD); - expect(root.scrollTop).not.toBe(logicalScrollTop); - expect(editor.getState().view).toEqual({ - scrollLeft: 73, - scrollTop: logicalScrollTop, - }); - } finally { - viewer.cleanUp(); - await wait(0); - cleanup(); - } - }); - - test('restores explicit editor state through CodeView logical scrolling', async () => { - const { cleanup } = installDom(); - const viewer = new CodeView({ - createEditor: (options) => new Editor({ ...options }), - layout: { - ...DEFAULT_CODE_VIEW_LAYOUT, - gap: 1_000_000, - }, - }); - const root = createClampingRoot(); - - try { - viewer.setup(root); - const { editor } = await renderRebasedEditedItem(viewer); - const initialScrollTop = viewer.getScrollTop(); - const restoredScrollTop = initialScrollTop - 100_000; - - editor.setState({ - view: { scrollLeft: 41, scrollTop: restoredScrollTop }, - }); - viewer.render(true); - - expect(viewer.getScrollTop()).toBe(restoredScrollTop); - expect(root.scrollTop).not.toBe(restoredScrollTop); - } finally { - viewer.cleanUp(); - await wait(0); - cleanup(); - } - }); - test('restores the paged scroll height after clearing and reusing the viewer', async () => { const { cleanup } = installDom(); const viewer = new CodeView({ diff --git a/packages/diffs/test/editorState.test.ts b/packages/diffs/test/editorState.test.ts index 432ffbca4..450baa2ce 100644 --- a/packages/diffs/test/editorState.test.ts +++ b/packages/diffs/test/editorState.test.ts @@ -1,12 +1,13 @@ import { describe, expect, test } from 'bun:test'; -import type { Virtualizer } from '../src/components/Virtualizer'; import { Editor } from '../src/editor/editor'; +import type { IStateStorage } from '../src/editor/stateStorage'; import type { DiffLineAnnotation, DiffsEditableComponent, DiffsEditor, DiffsHighlighter, + EditorState, FileContents, HighlightedToken, RenderRange, @@ -36,24 +37,9 @@ class TestEditableComponent implements DiffsEditableComponent { #lineAnnotations?: DiffLineAnnotation[]; #renderRange?: RenderRange; codeScrollLeft = 0; - virtualizerScrollTop = 0; restoredCodeScrollLefts: number[] = []; - restoredVirtualizerScrollPositions: unknown[] = []; - readonly virtualizer = { - type: 'simple', - getScrollTop: () => this.virtualizerScrollTop, - scrollTo: (target: unknown) => { - this.restoredVirtualizerScrollPositions.push(target); - }, - } as unknown as Virtualizer; - - readonly __getVirtualizer: (() => Virtualizer) | undefined; - - constructor( - private file: FileContents, - virtualized = true - ) { - this.__getVirtualizer = virtualized ? () => this.virtualizer : undefined; + + constructor(private file: FileContents) { this.#renderShadowDom(); } @@ -158,7 +144,7 @@ class TestEditableComponent implements DiffsEditableComponent { } describe('Editor state', () => { - test('getState composes horizontal and vertical scroll owners', () => { + test('getState captures horizontal state from the code scroller', () => { const dom = installDom(); const editor = new Editor(); const component = new TestEditableComponent({ @@ -166,30 +152,6 @@ describe('Editor state', () => { contents: 'alpha\nbravo', }); - try { - editor.edit(component); - component.codeScrollLeft = 24; - component.virtualizerScrollTop = 128; - - expect(editor.getState().view).toEqual({ - scrollLeft: 24, - scrollTop: 128, - }); - } finally { - editor.cleanUp(); - component.cleanUp(); - dom.cleanup(); - } - }); - - test('getState omits vertical state for a non-virtualized component', () => { - const dom = installDom(); - const editor = new Editor(); - const component = new TestEditableComponent( - { name: 'state.ts', contents: 'alpha\nbravo' }, - false - ); - try { editor.edit(component); component.codeScrollLeft = 24; @@ -212,12 +174,9 @@ describe('Editor state', () => { try { editor.edit(component); - editor.setState({ view: { scrollLeft: 24, scrollTop: 128 } }); + editor.setState({ view: { scrollLeft: 24 } }); expect(component.restoredCodeScrollLefts).toEqual([24]); - expect(component.restoredVirtualizerScrollPositions).toEqual([ - { top: 128, behavior: 'instant' }, - ]); } finally { editor.cleanUp(); component.cleanUp(); @@ -225,20 +184,33 @@ describe('Editor state', () => { } }); - test('setState restores horizontal state without an optional vertical position', () => { + test('automatic persistence stores horizontal state', () => { const dom = installDom(); - const editor = new Editor(); + let storedState: EditorState | undefined; + const storage: IStateStorage = { + get() { + return undefined; + }, + set(_cacheKey, state) { + storedState = state; + }, + }; + const editor = new Editor({ + persistState: true, + persistStateStorage: storage, + }); const component = new TestEditableComponent({ name: 'state.ts', contents: 'alpha\nbravo', + cacheKey: 'state.ts', }); try { editor.edit(component); - editor.setState({ view: { scrollLeft: 24 } }); + component.codeScrollLeft = 24; + editor.cleanUp(); - expect(component.restoredCodeScrollLefts).toEqual([24]); - expect(component.restoredVirtualizerScrollPositions).toEqual([]); + expect(storedState?.view).toEqual({ scrollLeft: 24 }); } finally { editor.cleanUp(); component.cleanUp(); @@ -274,13 +246,10 @@ describe('Editor state', () => { direction: 0, }, ], - view: { scrollLeft: 12, scrollTop: 40 }, + view: { scrollLeft: 12 }, }); expect(component.restoredCodeScrollLefts).toEqual([12]); - expect(component.restoredVirtualizerScrollPositions).toEqual([ - { top: 40, behavior: 'instant' }, - ]); expect(scrollIntoViewCalls).toBe(0); expect(editor.getState().selections).toEqual([ { diff --git a/packages/diffs/test/editorVirtualizedEdit.test.ts b/packages/diffs/test/editorVirtualizedEdit.test.ts index 15cd521c3..85dd0629a 100644 --- a/packages/diffs/test/editorVirtualizedEdit.test.ts +++ b/packages/diffs/test/editorVirtualizedEdit.test.ts @@ -387,7 +387,7 @@ describe('Editor selections in a virtualized window', () => { direction: DirectionForward, }, ], - view: { scrollLeft: 0, scrollTop: 0 }, + view: { scrollLeft: 0 }, }); expect( diff --git a/packages/diffs/test/virtualizedEditorViewport.test.ts b/packages/diffs/test/virtualizedEditorViewport.test.ts index 6fae6ab87..c89c6cec6 100644 --- a/packages/diffs/test/virtualizedEditorViewport.test.ts +++ b/packages/diffs/test/virtualizedEditorViewport.test.ts @@ -14,7 +14,6 @@ function createSimpleVirtualizer(root: HTMLElement) { connect() {}, disconnect() {}, getRoot: () => root, - getScrollTop: () => 0, getWindowSpecs: () => ({ top: 0, bottom: 800 }), getOffsetInScrollContainer: () => 0, instanceChanged(instance: { onRender(dirty: boolean): boolean }) { @@ -23,7 +22,6 @@ function createSimpleVirtualizer(root: HTMLElement) { isInstanceVisible: () => true, markDOMDirty() {}, requestHeightReconcile() {}, - scrollTo() {}, } as never; } @@ -95,8 +93,6 @@ describe('virtualized editor viewport', () => { try { expect(file.getEditorViewport()).toBe(root); expect(fileDiff.getEditorViewport()).toBe(root); - expect(file.__getVirtualizer()).toBe(virtualizer); - expect(fileDiff.__getVirtualizer()).toBe(virtualizer); } finally { file.cleanUp(); fileDiff.cleanUp(); @@ -117,8 +113,6 @@ describe('virtualized editor viewport', () => { try { expect(file.getEditorViewport()).toBe(root); expect(fileDiff.getEditorViewport()).toBe(root); - expect(file.__getVirtualizer()).toBe(codeView); - expect(fileDiff.__getVirtualizer()).toBe(codeView); } finally { file.cleanUp(); fileDiff.cleanUp();