From e29a0223a9342261b68c543ea15cfd2d46cf43b7 Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Sun, 11 Oct 2026 17:17:18 +0200 Subject: [PATCH 1/2] fix(virtual-core): let a user scroll end a non-smooth scrollToIndex before it lands The scroll handler only treated a scroll event as the user leaving an index scroll once reconcile had seen the target reached. A target that keeps moving while rows are measured (or, on iOS, a write ignored under a finger) never "reaches" it, so the state stayed alive after the user scrolled away and the next measurement that moved the target wrote them back to it. A non-smooth write lands at once, so a scroll event that isn't the read-back of our own write is the user's even before the target is reached. A write the browser reads back short of what we wrote because the scroll max shrank reports the max, where the fresh target also clamps, so it keeps reconciling as before (#1294); clamped compensation writes stay covered by `_clampedAdjustment`. Smooth scrolls are unchanged: their animation events look like a user's, so they still yield only after reaching the target. That is decided by a new `startedSmooth` flag rather than `behavior`, which reconcile switches to 'auto' for the final approach while WebKit can still deliver animation events. Removes `.fails` from the two #1308 pull-back tests, and makes the #1294 "keeps an index scroll active before reaching its target" test model the max shrinking before the read-back. Refs #1307 Co-Authored-By: Claude Opus 5.5 --- .changeset/index-scroll-user-interrupt.md | 5 ++ packages/virtual-core/src/index.ts | 25 +++++- packages/virtual-core/tests/index.test.ts | 103 ++++++++++++++-------- 3 files changed, 91 insertions(+), 42 deletions(-) create mode 100644 .changeset/index-scroll-user-interrupt.md diff --git a/.changeset/index-scroll-user-interrupt.md b/.changeset/index-scroll-user-interrupt.md new file mode 100644 index 00000000..219eca40 --- /dev/null +++ b/.changeset/index-scroll-user-interrupt.md @@ -0,0 +1,5 @@ +--- +'@tanstack/virtual-core': patch +--- + +fix(virtual-core): let a user scroll end a non-smooth `scrollToIndex` before reconcile has seen it land, so a later measurement no longer pulls the user back to the old target diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index e42eff8f..935e10e4 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -406,6 +406,10 @@ type ScrollState = { stableFrames: number // Keep arrival across measurement-driven retargets so external scrolls can cancel. hasReachedTarget: boolean + // The command asked for a smooth scroll. Unlike `behavior`, which reconcile + // switches to 'auto' for the final approach, this stays set: the smooth + // animation can still deliver scroll events after that switch. + startedSmooth: boolean // Where our last non-smooth write for this command put the element, until // its scroll event arrives. Until then `scrollOffset` still holds the old @@ -969,15 +973,24 @@ export class Virtualizer< offset = this._intendedScrollOffset } else if ( this.scrollState?.index != null && - this.scrollState.hasReachedTarget && + // A non-smooth write lands at once, so any other scroll event is + // the user's, even before reconcile saw the target reached (the + // target may keep moving, or iOS ignored the write under a + // finger). A write the browser clamped because the sizer hasn't + // grown yet reads back at the max, which is also where the fresh + // target clamps, so the check below keeps it reconciling (#1294). + // A smooth scroll's own animation events look like a user's, so + // it only yields once it has reached the target. + (this.scrollState.hasReachedTarget || + !this.scrollState.startedSmooth) && !approxEqual(offset, this.scrollState.lastTargetOffset) && !approxEqual(offset, this.getScrollStateTarget(this.scrollState)) && (this._clampedAdjustment === null || Math.abs(offset - this._clampedAdjustment.maxAtWrite) >= 1.5) ) { - // The index scroll reached its target, but an external scroll moved - // away while we waited for measurements. Our own measurement updates - // (including clamped writes) still need to reconcile. + // An external scroll moved away from the index scroll's target. + // Our own measurement updates (including clamped writes) still + // need to reconcile. this.scrollState = null } this._intendedScrollOffset = null @@ -2063,6 +2076,7 @@ export class Virtualizer< lastTargetOffset: offset, stableFrames: 0, hasReachedTarget: false, + startedSmooth: behavior === 'smooth', } this._scrollToOffset(offset, { adjustments: undefined, behavior }) @@ -2098,6 +2112,7 @@ export class Virtualizer< lastTargetOffset: offset, stableFrames: 0, hasReachedTarget: approxEqual(offset, this.getScrollOffset()), + startedSmooth: behavior === 'smooth', } this._scrollToOffset(offset, { adjustments: undefined, behavior }) @@ -2126,6 +2141,7 @@ export class Virtualizer< lastTargetOffset: offset, stableFrames: 0, hasReachedTarget: false, + startedSmooth: behavior === 'smooth', } this._scrollToOffset(offset, { adjustments: undefined, behavior }) @@ -2150,6 +2166,7 @@ export class Virtualizer< lastTargetOffset: offset, stableFrames: 0, hasReachedTarget: approxEqual(offset, this.getScrollOffset()), + startedSmooth: behavior === 'smooth', } this._scrollToOffset(offset, { adjustments: undefined, behavior }) diff --git a/packages/virtual-core/tests/index.test.ts b/packages/virtual-core/tests/index.test.ts index 7fdcad52..101348b7 100644 --- a/packages/virtual-core/tests/index.test.ts +++ b/packages/virtual-core/tests/index.test.ts @@ -543,6 +543,13 @@ test.each(['auto', 'smooth'] as const)( virtualizer._willUpdate() virtualizer.getVirtualItems() virtualizer.scrollToIndex(99, { align: 'end', behavior }) + // The scroll max shrinks to 2000 before the read-back, so the browser + // reports 2000 instead of the 4400 we wrote. That is where the fresh + // target clamps too, so it isn't mistaken for the user scrolling away. + Object.defineProperty(mockScrollElement, 'scrollHeight', { + value: 2600, + configurable: true, + }) onScroll(2000, true) expect(virtualizer['scrollState']).not.toBeNull() @@ -2551,46 +2558,40 @@ test('scrollToIndex (#1307): a resize classified against the stale offset does n expect(h.el.scrollTop).toBe(500) }) -test.fails( - 'scrollToIndex (#1307): a drag that never reached the target is not pulled back when the target moves', - () => { - // Seen in iOS Safari: a scrollToIndex issued while a finger is dragging is - // ignored by the browser, so the offset never gets near the target. - const h = makeDeferralHarness() - h.run(() => { - h.v.scrollToIndex(50, { align: 'start' }) - h.scroll(true, 20) - h.frame() - h.scroll(true, 60) - h.frame() - // A row measures and moves the target; the user is still at 60. - h.v.resizeItem(30, 100) - h.frame() - expect(h.el.scrollTop).toBe(60) - }) - }, -) - -test.fails( - 'scrollToIndex (#1307): a scroll away from a target that moved before landing is not pulled back, non-iOS', - () => { - // Not specific to iOS ignoring writes under a finger: a target that moves - // before reconcile sees the scroll land leaves the state alive too. - const h = makeDeferralHarness({ ios: false }) +test('scrollToIndex (#1307): a drag that never reached the target is not pulled back when the target moves', () => { + // Seen in iOS Safari: a scrollToIndex issued while a finger is dragging is + // ignored by the browser, so the offset never gets near the target. + const h = makeDeferralHarness() + h.run(() => { h.v.scrollToIndex(50, { align: 'start' }) - h.scroll(true) - h.v.resizeItem(10, 100) + h.scroll(true, 20) h.frame() - h.scroll(true) - expect(h.el.scrollTop).toBe(2550) - // The user scrolls away, then a row above them measures. - h.scroll(true, 1000) + h.scroll(true, 60) h.frame() - h.v.resizeItem(5, 100) + // A row measures and moves the target; the user is still at 60. + h.v.resizeItem(30, 100) h.frame() - expect(h.el.scrollTop).toBe(1050) - }, -) + expect(h.el.scrollTop).toBe(60) + }) +}) + +test('scrollToIndex (#1307): a scroll away from a target that moved before landing is not pulled back, non-iOS', () => { + // Not specific to iOS ignoring writes under a finger: a target that moves + // before reconcile sees the scroll land leaves the state alive too. + const h = makeDeferralHarness({ ios: false }) + h.v.scrollToIndex(50, { align: 'start' }) + h.scroll(true) + h.v.resizeItem(10, 100) + h.frame() + h.scroll(true) + expect(h.el.scrollTop).toBe(2550) + // The user scrolls away, then a row above them measures. + h.scroll(true, 1000) + h.frame() + h.v.resizeItem(5, 100) + h.frame() + expect(h.el.scrollTop).toBe(1050) +}) test('iOS deferral: a deferred correction follows the viewport through momentum', () => { const h = makeDeferralHarness({ start: 1000 }) @@ -5474,9 +5475,10 @@ function setupHandDrivenScroll( v['getMeasurements']() const scroll = (offset: number, isScrolling: boolean) => scrollCallback!(offset, isScrolling) + const frame = () => rafCallbacks.splice(0).forEach((cb) => cb(0)) const settle = () => { for (let i = 0; i < 10 && rafCallbacks.length > 0; i++) { - rafCallbacks.splice(0).forEach((cb) => cb(0)) + frame() scroll(el.scrollTop, true) } scroll(el.scrollTop, false) @@ -5488,7 +5490,7 @@ function setupHandDrivenScroll( rafCallbacks.splice(0).forEach((cb) => cb(0)) } } - return { v, el, scroll, settle, frames, browser } + return { v, el, scroll, frame, settle, frames, browser } } test('a resize before the read-back of a scrollToIndex write is left to reconcile', () => { @@ -5649,3 +5651,28 @@ test('scrollBy after cancelScroll dropped an ignored write builds on the real po expect(el.scrollTop).toBe(2100) }) + +test('a smooth scrollToIndex switched to auto near the end is not cancelled by a late animation event', () => { + // Reconcile switches a smooth scroll to 'auto' for the final approach when + // the target moves within a viewport. WebKit can still deliver a scroll + // event from the smooth animation after that instant write; it is not the + // user and must not end the scroll. + const { v, el, scroll, frame, settle } = setupHandDrivenScroll() + + v.scrollToIndex(50, { align: 'start', behavior: 'smooth' }) + el.scrollTop = 2450 // the animation is almost there + scroll(2450, true) + v.resizeItem(10, 100) // the target moves to 2550, within a viewport + frame() // reconcile switches to 'auto' and writes 2550 + expect(el.scrollTop).toBe(2550) + + el.scrollTop = 2470 // a late event from the smooth animation + scroll(2470, true) + expect(v.scrollState).not.toBeNull() + + el.scrollTop = 2550 + scroll(2550, true) + settle() + expect(el.scrollTop).toBe(2550) + expect(v.scrollState).toBeNull() +}) From f596cb0bfc0633a2d236134b715300372f1073a3 Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Mon, 12 Oct 2026 12:36:10 +0200 Subject: [PATCH 2/2] test(virtual-core): pin a clamped index read-back below a shrunk target The user-interrupt check keeps a clamped read-back reconciling because it lands where the fresh target clamps. Rows above the target that shrink before the read-back can pull the target below the new max, and the read-back then reads as a user scroll. It needs the sizer re-rendered before the scroll event, which has not been seen in a browser, so pin it with a `.fails` test and correct the comment instead of special-casing it. Co-Authored-By: Claude Opus 5.5 --- packages/virtual-core/src/index.ts | 5 +++++ packages/virtual-core/tests/index.test.ts | 24 +++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index 935e10e4..4720c705 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -979,6 +979,11 @@ export class Virtualizer< // finger). A write the browser clamped because the sizer hasn't // grown yet reads back at the max, which is also where the fresh // target clamps, so the check below keeps it reconciling (#1294). + // Not when rows above the target shrink before the read-back and + // pull the target below the new max: the read-back then looks + // like the user and the scroll stops at the max. That needs the + // sizer re-rendered before the scroll event, which browsers have + // not been seen to do (pinned by a `.fails` test). // A smooth scroll's own animation events look like a user's, so // it only yields once it has reached the target. (this.scrollState.hasReachedTarget || diff --git a/packages/virtual-core/tests/index.test.ts b/packages/virtual-core/tests/index.test.ts index 101348b7..17066043 100644 --- a/packages/virtual-core/tests/index.test.ts +++ b/packages/virtual-core/tests/index.test.ts @@ -5676,3 +5676,27 @@ test('a smooth scrollToIndex switched to auto near the end is not cancelled by a expect(el.scrollTop).toBe(2550) expect(v.scrollState).toBeNull() }) + +test.fails( + 'a scrollToIndex write clamped by a sizer that shrank below it still reaches the fresh target', + () => { + // Not seen in a real browser: the sizer has to re-render before the + // scroll event reports the write. Rows above the target shrink by 600px, + // so the max drops from 4800 to 4200 and the target from 4500 to 3900. + // The read-back at the new max is neither our write nor the target, so it + // reads as a user scroll and the index scroll stops at 4200. + const { v, el, scroll, settle } = setupHandDrivenScroll() + + v.scrollToIndex(90, { align: 'start' }) + expect(el.scrollTop).toBe(4500) + for (let i = 0; i < 20; i++) v.resizeItem(i, 20) + // The re-render that shrinks the sizer also rebuilds the measurements. + v.getVirtualItems() + el.scrollHeight = 4400 + el.scrollTop = 4200 + scroll(4200, true) + settle() + + expect(el.scrollTop).toBe(3900) + }, +)