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..28d58f7c1 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() { ( @@ -931,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; @@ -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(). @@ -1053,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; @@ -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..fcf4a787d 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 @@ -265,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; @@ -312,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/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/components/File.ts b/packages/diffs/src/components/File.ts index 22bf92f01..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 { @@ -1476,6 +1485,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 +1567,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..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 { @@ -1862,6 +1882,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 +1966,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 +2236,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/components/VirtualizedFile.ts b/packages/diffs/src/components/VirtualizedFile.ts index e214dfb09..3fecb6ac5 100644 --- a/packages/diffs/src/components/VirtualizedFile.ts +++ b/packages/diffs/src/components/VirtualizedFile.ts @@ -394,9 +394,10 @@ export class VirtualizedFile< }; } - public getScrollContainer(): HTMLElement | undefined { - const root = this.getSimpleVirtualizer()?.getRoot(); - 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( diff --git a/packages/diffs/src/components/VirtualizedFileDiff.ts b/packages/diffs/src/components/VirtualizedFileDiff.ts index 05282ed9f..8704680dd 100644 --- a/packages/diffs/src/components/VirtualizedFileDiff.ts +++ b/packages/diffs/src/components/VirtualizedFileDiff.ts @@ -612,9 +612,10 @@ export class VirtualizedFileDiff< return position; } - public getScrollContainer(): HTMLElement | undefined { - const root = this.getSimpleVirtualizer()?.getRoot(); - 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( 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/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index 64c2b4105..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; @@ -244,6 +244,15 @@ export interface EditorOptions { __debug?: boolean; } +export interface EditorFocusOptions extends FocusOptions { + /** 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 // 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; @@ -353,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; @@ -579,14 +592,13 @@ export class Editor implements DiffsEditor { } getState(): EditorState { - const scrollContainer = this.#fileInstance?.getScrollContainer?.(); + const fileInstance = this.#fileInstance; return { selections: this.#selections, view: - scrollContainer !== undefined + fileInstance != null ? { - scrollLeft: scrollContainer.scrollLeft, - scrollTop: scrollContainer.scrollTop, + scrollLeft: fileInstance.getCodeScrollLeft(), } : undefined, }; @@ -601,15 +613,8 @@ 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); return; } this.#scrollToPrimaryCaret(); @@ -681,8 +686,29 @@ 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 (lineNumber === 'first-visible' || typeof lineNumber === 'number') { + const textDocument = this.#textDocument; + 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: targetLineNumber - 1, + character: + lineNumber === 'first-visible' ? 0 : (options?.character ?? 0), + }); + this.#focusAtPosition(position, preventScroll); + return; + } const primarySelection = this.#selections?.at(-1); if (primarySelection !== undefined) { const pos = @@ -696,6 +722,9 @@ export class Editor implements DiffsEditor { } blur(): void { + this.#replacementFocusRequest = undefined; + this.#contentHasFocus = false; + this.#shouldIgnoreSelectionChange = false; this.#contentElement?.blur(); } @@ -763,6 +792,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; @@ -819,6 +849,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, @@ -856,6 +901,7 @@ export class Editor implements DiffsEditor { } } if (codeElement === undefined || contentEl === undefined) { + this.#replacementFocusRequest = undefined; return; } @@ -1001,6 +1047,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 @@ -1071,9 +1120,12 @@ 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 && + this.#contentHasFocus && !this.#retainSearchPanelFocus ) { this.focus({ preventScroll: true }); @@ -1164,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; } @@ -1236,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 ) { @@ -1402,6 +1452,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); }; @@ -1437,7 +1498,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', @@ -1679,6 +1779,10 @@ export class Editor implements DiffsEditor { contentEl, 'blur', () => { + if (this.#replacementFocusRequest?.target === contentEl) { + this.#replacementFocusRequest = undefined; + this.#shouldIgnoreSelectionChange = false; + } this.#contentHasFocus = false; onBlur?.(); }, @@ -2935,7 +3039,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(); } } @@ -3235,7 +3341,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. @@ -3249,16 +3359,207 @@ 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 { + const contentElement = this.#contentElement; + const fileContainer = this.#fileContainer; + if ( + this.#textDocument == null || + contentElement == null || + fileContainer == null + ) { + return undefined; + } + const viewport = + this.#fileInstance?.getEditorViewport?.() ?? + this.#getDefaultEditorViewport(fileContainer); + + 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; + } + + // 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, + end: position, + direction: DirectionNone, + }; + this.#canMountSelectionAction = false; + this.#updateSelections([selection]); + this.#revealLineIfCollapsed(position.line); + if (preventScroll) { + this.#focus(position, true); + } else { + this.#scrollToPrimaryCaret(); } } @@ -3320,11 +3621,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); @@ -3366,7 +3671,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 47cb5d959..29320c28a 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -1033,6 +1033,10 @@ 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; /** * 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. @@ -1043,9 +1047,10 @@ export interface DiffsEditableComponent< lineNumber: number ) => { top: number; height: number } | undefined; /** - * Return the scroll container element. + * Return an explicit viewport that bounds visible editor rows. Components + * without one fall back to their nearest scrollable ancestor or document. */ - getScrollContainer?: () => HTMLElement | undefined; + 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 @@ -1109,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, @@ -1210,12 +1217,14 @@ export interface EditorSelection extends Range { direction: SelectionDirection; } +export interface EditorViewState { + /** Horizontal position owned by the current editable code scroller. */ + scrollLeft: number; +} + export interface EditorState { selections?: EditorSelection[]; - view?: { - scrollLeft: number; - scrollTop: number; - }; + view?: EditorViewState; } export interface DiffsTextDocument { diff --git a/packages/diffs/test/CodeView.edit.test.ts b/packages/diffs/test/CodeView.edit.test.ts index 96ab540c7..2cf8b8dae 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 { @@ -73,6 +74,7 @@ function createEditorHarness({ detach?.(recycle); detach = undefined; }, + __captureFocusForDOMReplacement() {}, __postponeBgTokenizeToNextFrame() {}, __syncRenderView() {}, }; @@ -528,6 +530,114 @@ describe('CodeView item edit mode', () => { } }); + test('recycle restores persisted item-local state without moving CodeView', 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 + ); + }); + 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([ + { + 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/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(); + } + }); +}); 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/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/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/editorPublicApi.test.ts b/packages/diffs/test/editorPublicApi.test.ts index 16ec45e0b..4e36ce7db 100644 --- a/packages/diffs/test/editorPublicApi.test.ts +++ b/packages/diffs/test/editorPublicApi.test.ts @@ -1,8 +1,13 @@ 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, 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'; @@ -16,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); @@ -37,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( @@ -71,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, @@ -275,6 +360,306 @@ 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(); + } + }); + + 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 falls back to document viewport bounds', async () => { + const { cleanup, content, editor } = await createEditorFixture( + 'alpha\nbravo\ncharlie' + ); + try { + 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 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' + ); + 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/editorRecycle.test.ts b/packages/diffs/test/editorRecycle.test.ts index 9c7ec661f..3f23791d2 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(); } @@ -62,6 +75,12 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} + getCodeScrollLeft(): number { + return 0; + } + + setCodeScrollLeft(): void {} + render({ file, lineAnnotations, @@ -81,6 +100,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 +155,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 +167,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 +218,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 +439,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 { diff --git a/packages/diffs/test/editorState.test.ts b/packages/diffs/test/editorState.test.ts index df5e50a5f..450baa2ce 100644 --- a/packages/diffs/test/editorState.test.ts +++ b/packages/diffs/test/editorState.test.ts @@ -1,11 +1,13 @@ import { describe, expect, test } from 'bun:test'; import { Editor } from '../src/editor/editor'; +import type { IStateStorage } from '../src/editor/stateStorage'; import type { DiffLineAnnotation, DiffsEditableComponent, DiffsEditor, DiffsHighlighter, + EditorState, FileContents, HighlightedToken, RenderRange, @@ -34,11 +36,10 @@ class TestEditableComponent implements DiffsEditableComponent { #editor?: DiffsEditor; #lineAnnotations?: DiffLineAnnotation[]; #renderRange?: RenderRange; + codeScrollLeft = 0; + restoredCodeScrollLefts: number[] = []; - constructor( - readonly scrollContainer: HTMLElement, - private file: FileContents - ) { + constructor(private file: FileContents) { this.#renderShadowDom(); } @@ -50,8 +51,13 @@ class TestEditableComponent implements DiffsEditableComponent { setEditorActiveLine(_lineNumber: number | null): void {} - getScrollContainer(): HTMLElement { - return this.scrollContainer; + getCodeScrollLeft(): number { + return this.codeScrollLeft; + } + + setCodeScrollLeft(position: number): void { + this.codeScrollLeft = position; + this.restoredCodeScrollLefts.push(position); } render({ @@ -138,48 +144,73 @@ class TestEditableComponent implements DiffsEditableComponent { } describe('Editor state', () => { - test('setState restores the saved scroll position', () => { + test('getState captures horizontal state from the code scroller', () => { 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({ + name: 'state.ts', + contents: 'alpha\nbravo', + }); + + 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(); - 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(); + editor.setState({ view: { scrollLeft: 24 } }); - scrollContainer.scrollLeft = 0; - scrollContainer.scrollTop = 0; - editor.setState(state); + expect(component.restoredCodeScrollLefts).toEqual([24]); + } finally { + editor.cleanUp(); + component.cleanUp(); + dom.cleanup(); + } + }); + + test('automatic persistence stores horizontal state', () => { + const dom = installDom(); + 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', + }); - expect(scrollContainer.scrollLeft).toBe(24); - expect(scrollContainer.scrollTop).toBe(128); - expect(scrollCalls).toEqual([{ left: 24, top: 128 }]); + try { + editor.edit(component); + component.codeScrollLeft = 24; + editor.cleanUp(); + + expect(storedState?.view).toEqual({ scrollLeft: 24 }); } finally { editor.cleanUp(); component.cleanUp(); @@ -189,38 +220,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 saved logical 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', }); @@ -235,11 +246,11 @@ describe('Editor state', () => { direction: 0, }, ], - view: { scrollLeft: 12, scrollTop: 40 }, + view: { scrollLeft: 12 }, }); - expect(scrollContainer.scrollLeft).toBe(12); - expect(scrollContainer.scrollTop).toBe(40); + expect(component.restoredCodeScrollLefts).toEqual([12]); + expect(scrollIntoViewCalls).toBe(0); expect(editor.getState().selections).toEqual([ { start: { line: 5, character: 0 }, 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/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/reactCodeViewEditOptions.test.ts b/packages/diffs/test/reactCodeViewEditOptions.test.ts index 1a4a0836b..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() {}, }; @@ -337,6 +338,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 +346,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 +371,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); diff --git a/packages/diffs/test/virtualizedEditorViewport.test.ts b/packages/diffs/test/virtualizedEditorViewport.test.ts new file mode 100644 index 000000000..c89c6cec6 --- /dev/null +++ b/packages/diffs/test/virtualizedEditorViewport.test.ts @@ -0,0 +1,232 @@ +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 { DiffsEditor } from '../src/types'; +import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; +import { installDom, waitFor } from './domHarness'; + +function createSimpleVirtualizer(root: HTMLElement) { + return { + type: 'simple', + config: {}, + connect() {}, + disconnect() {}, + getRoot: () => root, + getWindowSpecs: () => ({ top: 0, bottom: 800 }), + getOffsetInScrollContainer: () => 0, + instanceChanged(instance: { onRender(dirty: boolean): boolean }) { + instance.onRender(true); + }, + isInstanceVisible: () => true, + markDOMDirty() {}, + requestHeightReconcile() {}, + } as never; +} + +async function renderFile( + file: VirtualizedFile, + root: HTMLElement +): Promise { + const fileContainer = document.createElement('div'); + root.appendChild(fileContainer); + file.render({ + 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; +} + +async function renderFileDiff( + fileDiff: VirtualizedFileDiff, + root: HTMLElement +): Promise<{ additions: HTMLElement; deletions?: HTMLElement }> { + const fileContainer = document.createElement('div'); + root.appendChild(fileContainer); + fileDiff.render({ + fileDiff: parseDiffFromFile( + { 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])' + ); + 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(); + 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(); + } + }); + + test('reads and restores File horizontal state from the code scroller', async () => { + const dom = installDom(); + const root = document.createElement('div'); + root.scrollLeft = 900; + const virtualizer = createSimpleVirtualizer(root); + const file = new VirtualizedFile( + { disableFileHeader: true, theme: DEFAULT_THEMES }, + virtualizer + ); + + try { + const code = await renderFile(file, root); + code.scrollLeft = 60; + + 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(); + } + }); + + for (const diffStyle of ['unified', 'split'] as const) { + test(`reads ${diffStyle} FileDiff horizontal state from active code columns`, async () => { + const dom = installDom(); + const root = document.createElement('div'); + root.scrollLeft = 900; + const virtualizer = createSimpleVirtualizer(root); + const fileDiff = new VirtualizedFileDiff( + { + diffStyle, + disableFileHeader: true, + theme: DEFAULT_THEMES, + }, + virtualizer + ); + + try { + const code = await renderFileDiff(fileDiff, root); + code.additions.scrollLeft = 40; + if (code.deletions !== undefined) { + code.deletions.scrollLeft = 60; + } + + expect(fileDiff.getCodeScrollLeft()).toBe( + diffStyle === 'split' ? 60 : 40 + ); + } finally { + fileDiff.cleanUp(); + dom.cleanup(); + } + }); + } + + test('restores split FileDiff horizontal state to both code columns', async () => { + const dom = installDom(); + const root = document.createElement('div'); + const virtualizer = createSimpleVirtualizer(root); + const fileDiff = new VirtualizedFileDiff( + { + diffStyle: 'split', + disableFileHeader: true, + theme: DEFAULT_THEMES, + }, + virtualizer + ); + + try { + const code = await renderFileDiff(fileDiff, root); + expect(code.deletions).toBeDefined(); + + fileDiff.setCodeScrollLeft(60); + + expect(code.additions.scrollLeft).toBe(60); + expect(code.deletions?.scrollLeft).toBe(60); + + let recyclePosition: number | undefined; + fileDiff.attachEditor({ + __captureFocusForDOMReplacement() {}, + __postponeBgTokenizeToNextFrame() {}, + __syncRenderView() {}, + edit: () => () => {}, + cleanUp() { + recyclePosition = fileDiff.getCodeScrollLeft(); + }, + } as DiffsEditor); + code.deletions!.scrollLeft = 64; + fileDiff.cleanUp(true); + expect(recyclePosition).toBe(64); + } finally { + fileDiff.cleanUp(); + dom.cleanup(); + } + }); +}); 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() {