diff --git a/packages/diffs/src/components/CodeView.ts b/packages/diffs/src/components/CodeView.ts index 61dd6ba6e..596062a82 100644 --- a/packages/diffs/src/components/CodeView.ts +++ b/packages/diffs/src/components/CodeView.ts @@ -651,6 +651,11 @@ type PendingScrollTarget = | PendingRangeTarget | PendingItemTarget; +type CodeViewItemMap = Map< + string, + CodeViewContextItem +>; + export class CodeView { static __STOP = false; static __lastScrollPosition = 0; @@ -662,7 +667,7 @@ export class CodeView { resizeDebugging: false, }; private items: CodeViewContextItem[] = []; - private idToItem: Map> = new Map(); + private idToItem: CodeViewItemMap = new Map(); private selectedLines: CodeViewLineSelection | null = null; // One editor per edit-mode item, created lazily via options.createEditor. // Entries survive virtualization unmounts so a remounted item re-attaches @@ -1706,9 +1711,10 @@ export class CodeView { } } - public capturePendingLayoutAnchor(): void { + public capturePendingLayoutAnchor( + nextItems: Readonly> = this.idToItem + ): void { if ( - this.pendingLayoutAnchor != null || this.root == null || this.items.length === 0 || this.pendingScrollTarget != null @@ -1716,7 +1722,10 @@ export class CodeView { return; } - this.pendingLayoutAnchor = this.getScrollAnchor(this.getScrollTop()); + this.pendingLayoutAnchor = this.getScrollAnchor( + this.getScrollTop(), + nextItems + ); } public render(immediate = false): void { @@ -2521,6 +2530,15 @@ export class CodeView { nextInstanceToItem.set(item.instance, item); } + if (firstDirtyIndex == null) { + if (removedItems.size === 0) { + return; + } + firstDirtyIndex = Math.max(nextItems.length - 1, 0); + } + + this.capturePendingLayoutAnchor(nextIdToItem); + for (let index = 0; index < previousItems.length; index++) { const removedItem = previousItems[index]; if (removedItem == null || !removedItems.has(removedItem)) { @@ -2531,10 +2549,6 @@ export class CodeView { firstDirtyIndex = Math.min(firstDirtyIndex ?? dirtyIndex, dirtyIndex); } - if (firstDirtyIndex == null) { - return; - } - this.items = nextItems; this.idToItem = nextIdToItem; this.instanceToItem = nextInstanceToItem; @@ -3629,10 +3643,25 @@ export class CodeView { * A scroll anchor represents the first fully visible element (in other * words, the first file or first line who's top is fully in the viewport). */ - private getScrollAnchor(scrollTop: number): ScrollAnchor | undefined { - // If we already have a pendingLayoutAnchor, let's use that. - if (this.pendingLayoutAnchor != null) { - return this.pendingLayoutAnchor; + private getScrollAnchor( + scrollTop: number, + availableItems: ReadonlyMap> = this + .idToItem + ): ScrollAnchor | undefined { + let skippedItem: CodeViewContextItem | undefined; + + const { pendingLayoutAnchor } = this; + if (pendingLayoutAnchor != null) { + const pendingItem = this.idToItem.get(pendingLayoutAnchor.id); + if ( + pendingItem != null && + availableItems.get(pendingLayoutAnchor.id) === pendingItem + ) { + return pendingLayoutAnchor; + } + if (pendingItem != null) { + skippedItem = pendingItem; + } } // We shouldn't scroll anchor when at the top, this way if a custom header @@ -3671,6 +3700,12 @@ export class CodeView { break; } + const itemIsAvailable = availableItems.get(item.item.id) === item; + if (!itemIsAvailable) { + skippedItem ??= item; + continue; + } + if (absoluteItemTop >= scrollTop) { return { type: 'item', @@ -3698,6 +3733,28 @@ export class CodeView { } } + // If we couldn't find an anchored item and we have a skipped item, lets + // attempt to anchor to the top of the item that came after it + if (skippedItem != null) { + for ( + let index = skippedItem.index + 1; + index < this.items.length; + index++ + ) { + const candidate = this.items[index]; + if ( + candidate != null && + availableItems.get(candidate.item.id) === candidate + ) { + return { + type: 'item', + id: candidate.item.id, + viewportOffset: 0, + }; + } + } + } + // I don't think we'll ever make it this far... return undefined; } diff --git a/packages/diffs/test/CodeView.scrollAnchoring.test.ts b/packages/diffs/test/CodeView.scrollAnchoring.test.ts index 4a7d8e5d3..8b0542fe1 100644 --- a/packages/diffs/test/CodeView.scrollAnchoring.test.ts +++ b/packages/diffs/test/CodeView.scrollAnchoring.test.ts @@ -5,6 +5,7 @@ import { DEFAULT_CODE_VIEW_LAYOUT } from '../src/constants'; import type { CodeViewItem, FileContents } from '../src/types'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { + createRoot, dispatchScroll, installDom, makeFile, @@ -251,6 +252,181 @@ describe('CodeView scroll anchoring', () => { } }); + test('keeps a rendered item anchored when reconcileItems removes earlier items', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView(); + const root = createRoot({ height: ROOT_HEIGHT }); + const items = Array.from({ length: 60 }, (_, index) => + makeFileItem(`file:${index}`, 5 + ((index * 7) % 15)) + ); + const anchorId = 'file:30'; + + try { + viewer.setup(root); + await renderItems(viewer, items); + + viewer.scrollTo({ + type: 'item', + id: anchorId, + align: 'start', + behavior: 'instant', + }); + viewer.render(true); + await wait(0); + + const anchorTopBeforeRemoval = viewer.getTopForItem(anchorId) ?? 0; + const scrollTopBeforeRemoval = viewer.getScrollTop(); + + // Remove several earlier items in one call, well outside the current + // render window but still shifting the index (and therefore the top + // offset) of every item at or after them, including the anchor. A + // single removal isn't enough here, even giving the removed item a + // very different height from its neighbors: the stale scan is off by + // exactly one slot, which almost always still resolves to the very + // next real item, and that item's own recomputed shift is identical + // regardless of what was removed, so the error cancels out. Only a + // shift big enough to carry the stale, off-by-N read past the old + // viewport/overscan bounds makes the search fail outright, which is + // what removing 10 items in one call reliably does. + const removedIds = new Set( + Array.from({ length: 10 }, (_, index) => `file:${index}`) + ); + viewer.setItems(items.filter((item) => !removedIds.has(item.id))); + viewer.render(true); + await wait(0); + + const anchorTopAfterRemoval = viewer.getTopForItem(anchorId) ?? 0; + const scrollTopAfterRemoval = viewer.getScrollTop(); + + // The anchor shifted up by the removed items' combined height. A + // correct anchor keeps it fixed in the viewport, so the scroll + // position must shift by the same delta. Without capturing the anchor + // before the reindex, the anchor scan instead finds nothing (it reads + // stale, not-yet-relaid-out positions through the new index mapping) + // and scrollTop is left stuck at its old value. + const anchorShift = anchorTopBeforeRemoval - anchorTopAfterRemoval; + expect(anchorShift).toBeGreaterThan(0); + expect(scrollTopBeforeRemoval - scrollTopAfterRemoval).toBe(anchorShift); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + + test('anchors the next item at the top when reconcileItems removes the only visible anchor', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView(); + const root = createRoot({ height: ROOT_HEIGHT }); + const removedId = 'diff:removed-anchor'; + const nextId = 'file:after-anchor'; + const items: CodeViewItem[] = [ + makeFileItem('file:before-anchor', 120), + makeReplacementDiffItem(removedId, 240), + makeFileItem(nextId, 80), + ...Array.from({ length: 8 }, (_, index) => + makeFileItem(`file:trailing-${index}`, 80) + ), + ]; + + try { + viewer.setup(root); + await renderItems(viewer, items); + + viewer.scrollTo({ + type: 'line', + id: removedId, + lineNumber: 80, + side: 'additions', + align: 'start', + behavior: 'instant', + }); + viewer.render(true); + await wait(0); + + // Keep the successor outside the old render window so reconciliation + // has to anchor its item header at the top instead of finding a visible + // survivor. + expect(viewer.getRenderedItems().some((item) => item.id === nextId)).toBe( + false + ); + + viewer.setItems(items.filter((item) => item.id !== removedId)); + viewer.render(true); + await wait(0); + + const nextTop = viewer.getTopForItem(nextId); + if (nextTop == null) { + throw new Error('expected the promoted successor'); + } + expect(nextTop - viewer.getScrollTop()).toBe(0); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + + test('skips a removed line anchor for a visible surviving item', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView(); + const root = createRoot({ height: ROOT_HEIGHT }); + const removedId = 'diff:visible-removed-anchor'; + const nextId = 'file:visible-after-anchor'; + const items: CodeViewItem[] = [ + makeFileItem('file:visible-before-anchor', 100), + makeReplacementDiffItem(removedId, 12), + makeFileItem(nextId, 40), + makeFileItem('file:visible-trailing', 100), + ]; + + try { + viewer.setup(root); + await renderItems(viewer, items); + + viewer.scrollTo({ + type: 'line', + id: removedId, + lineNumber: 4, + side: 'additions', + align: 'start', + behavior: 'instant', + }); + viewer.render(true); + await wait(0); + + expect(viewer.getRenderedItems().some((item) => item.id === nextId)).toBe( + true + ); + const nextTopBeforeRemoval = viewer.getTopForItem(nextId); + if (nextTopBeforeRemoval == null) { + throw new Error('expected the visible successor'); + } + const nextViewportOffset = + DEFAULT_CODE_VIEW_LAYOUT.paddingTop + + nextTopBeforeRemoval - + viewer.getScrollTop(); + + viewer.setItems(items.filter((item) => item.id !== removedId)); + viewer.render(true); + await wait(0); + + const nextTopAfterRemoval = viewer.getTopForItem(nextId); + if (nextTopAfterRemoval == null) { + throw new Error('expected the surviving item'); + } + expect( + DEFAULT_CODE_VIEW_LAYOUT.paddingTop + + nextTopAfterRemoval - + viewer.getScrollTop() + ).toBe(nextViewportOffset); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + test('moves the physical spacer before applying a programmatic rebase jump', async () => { const { cleanup } = installDom(); const viewer = new CodeView({