From 6fc83abbb2c8def450d1abf7f96dc18e4aa7b5c0 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Wed, 9 Sep 2026 23:57:25 +0300 Subject: [PATCH] fix: restore file preview scroll positions after virtualizer height reconciliation Restore scroll position after Pierre reconciles measured heights, before paint Only remember line anchors when the anchor line is visible in the viewport --- .../useFilePreviewScrollPosition.test.tsx | 52 +++++++++++++++++-- .../views/useFilePreviewScrollPosition.ts | 33 ++++++++---- 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/packages/ui/src/components/views/useFilePreviewScrollPosition.test.tsx b/packages/ui/src/components/views/useFilePreviewScrollPosition.test.tsx index 47539a63..00859be0 100644 --- a/packages/ui/src/components/views/useFilePreviewScrollPosition.test.tsx +++ b/packages/ui/src/components/views/useFilePreviewScrollPosition.test.tsx @@ -2,13 +2,28 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import { Window } from 'happy-dom'; import React, { act, useLayoutEffect } from 'react'; import { createRoot, type Root } from 'react-dom/client'; +import { VirtualizedFile, Virtualizer } from '@pierre/diffs'; import { useFilePreviewScrollPosition } from './useFilePreviewScrollPosition'; +type RestorePreview = ReturnType['restore']; + +class MeasuredPreviewFile extends VirtualizedFile { + lineTop = 1000; + + override getLinePosition() { + return { top: this.lineTop, height: 20 }; + } + + override getNumericScrollAnchor() { + return { lineNumber: 51, top: this.lineTop }; + } +} + function Preview({ positionKey, element, onReady }: { positionKey: string | null; element: HTMLElement; - onReady: (restore: () => void) => void; + onReady: (restore: RestorePreview) => void; }) { const { setScroller, restore } = useFilePreviewScrollPosition(positionKey); useLayoutEffect(() => { @@ -27,10 +42,10 @@ describe('file preview scroll positions', () => { let height: number; let top: number; let left: number; - let restore: () => void; + let restore: RestorePreview; let prefix: string; let sequence = 0; - const onReady = (callback: () => void) => { restore = callback; }; + const onReady = (callback: RestorePreview) => { restore = callback; }; beforeEach(() => { windowInstance = new Window(); @@ -39,6 +54,7 @@ describe('file preview scroll positions', () => { document: windowInstance.document, HTMLElement: windowInstance.HTMLElement, Event: windowInstance.Event, + DOMRect: windowInstance.DOMRect, MutationObserver: windowInstance.MutationObserver, ResizeObserver: windowInstance.ResizeObserver, IS_REACT_ACT_ENVIRONMENT: true, @@ -54,6 +70,7 @@ describe('file preview scroll positions', () => { left = 0; prefix = `preview-test-${sequence++}`; Object.defineProperties(scroller, { + clientHeight: { value: 100 }, scrollTop: { get: () => top, set: (value: number) => { top = Math.max(0, Math.min(value, height - 100)); }, @@ -168,4 +185,33 @@ describe('file preview scroll positions', () => { restore(); expect(top).toBe(50); }); + + test('restores the same virtualized line after Pierre reconciles different height estimates', async () => { + const file = new MeasuredPreviewFile({}, new Virtualizer()); + const node = document.createElement('div'); + const line = document.createElement('div'); + line.dataset.line = ''; + line.dataset.lineIndex = '50'; + node.attachShadow({ mode: 'open' }).append(line); + content.append(node); + line.getBoundingClientRect = () => new DOMRect(0, file.lineTop - top + 8, 100, 20); + + await render('virtual'); + scroll(1000); + restore(node, file); + await Promise.resolve(); + expect(line.getBoundingClientRect().top).toBe(8); + + await render(null); + scroll(0); + await render('virtual'); + restore(node, file); + // onPostRender runs before Pierre's synchronous height reconciliation. + file.lineTop = 1500; + scroll(800); + await Promise.resolve(); + expect(top).toBe(1500); + expect(line.getBoundingClientRect().top).toBe(8); + file.cleanUp(); + }); }); diff --git a/packages/ui/src/components/views/useFilePreviewScrollPosition.ts b/packages/ui/src/components/views/useFilePreviewScrollPosition.ts index 7b0609bc..bd0cbd3b 100644 --- a/packages/ui/src/components/views/useFilePreviewScrollPosition.ts +++ b/packages/ui/src/components/views/useFilePreviewScrollPosition.ts @@ -21,10 +21,18 @@ export function useFilePreviewScrollPosition(positionKey: string | null) { if (node && instance instanceof VirtualizedFile) { virtualFileRef.current = { key: positionKey, file: instance, node }; } - restoreRef.current?.(); - // The scroll event can precede the virtualizer mounting the target lines. - // Capture again after rendering, when a numeric line anchor is available. - rememberRef.current?.(); + const restorePosition = restoreRef.current; + const rememberPosition = rememberRef.current; + const finishRender = () => { + if (restoreRef.current !== restorePosition) return; + restorePosition?.(); + rememberPosition?.(); + }; + // Pierre calls onPostRender before reconciling measured heights and applying + // its own scroll correction. Finish after that synchronous render pass, + // before paint, rather than saving estimates or having our restore undone. + if (instance) queueMicrotask(finishRender); + else finishRender(); }, [positionKey]); useLayoutEffect(() => { @@ -48,11 +56,13 @@ export function useFilePreviewScrollPosition(positionKey: string | null) { const lineElement = target.line && getLineElement(target.line.number); // Estimated heights locate the virtual window; the mounted row supplies // the final visual offset, including wrapping and Pierre's padding. - const targetTop = lineElement && target.line - ? scroller.scrollTop + lineElement.getBoundingClientRect().top - scroller.getBoundingClientRect().top - target.line.offset - : linePosition && target.line - ? (file?.top ?? 0) + linePosition.top - target.line.offset - : target.top; + let targetTop = target.top; + if (target.line && linePosition) { + targetTop = (file?.top ?? 0) + linePosition.top - target.line.offset; + } + if (target.line && lineElement) { + targetTop = scroller.scrollTop + lineElement.getBoundingClientRect().top - scroller.getBoundingClientRect().top - target.line.offset; + } scroller.scrollTop = targetTop; scroller.scrollLeft = target.left; if ((!target.line || lineElement) && Math.abs(scroller.scrollTop - targetTop) < 1 && Math.abs(scroller.scrollLeft - target.left) < 1) { @@ -77,12 +87,13 @@ export function useFilePreviewScrollPosition(positionKey: string | null) { const file = getVirtualFile(); const anchor = file?.getNumericScrollAnchor(scroller.scrollTop - (file.top ?? 0)); const lineElement = anchor && getLineElement(anchor.lineNumber); + const offset = lineElement ? lineElement.getBoundingClientRect().top - scroller.getBoundingClientRect().top : null; positions.delete(positionKey); positions.set(positionKey, { top: scroller.scrollTop, left: scroller.scrollLeft, - line: anchor && lineElement - ? { number: anchor.lineNumber, offset: lineElement.getBoundingClientRect().top - scroller.getBoundingClientRect().top } + line: anchor && offset !== null && offset >= 0 && offset < scroller.clientHeight + ? { number: anchor.lineNumber, offset } : undefined, }); if (positions.size > MAX_POSITIONS) {