Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/adjust-scroll-first-measurement-backward.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/virtual-core': patch
---

Fix "items jump while scrolling up": the default scroll-adjustment predicate now compensates scrollTop on the first measurement of an above-viewport item even while scrolling backward (the estimate→actual delta must be absorbed), and only skips compensation for re-measurements during backward scroll to avoid the cascading jank
1 change: 1 addition & 0 deletions examples/react/chat/src/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ button:hover {
.Messages {
min-height: 0;
overflow: auto;
overflow-anchor: none;
width: 100%;
}

Expand Down
10 changes: 10 additions & 0 deletions packages/react-virtual/e2e/app/scroll-anchor/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
</head>
<body>
<div id="root"></div>
<script type="module" src="./main.tsx"></script>
</body>
</html>
59 changes: 59 additions & 0 deletions packages/react-virtual/e2e/app/scroll-anchor/main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import React from 'react'
import ReactDOM from 'react-dom/client'
import { useVirtualizer } from '@tanstack/react-virtual'

// Deterministic sizing: every row is ITEM_SIZE tall while the virtualizer
// estimates ESTIMATE. The gap means every above-viewport item that gets
// measured for the first time produces a predictable estimate→actual delta,
// which is exactly the scenario where scrolling up must compensate scrollTop
// to keep the anchored content visually stable.
const ITEM_SIZE = 90
const ESTIMATE = 50

const App = () => {
const parentRef = React.useRef<HTMLDivElement>(null)
const initialOffset = Number(
new URLSearchParams(window.location.search).get('initialOffset') ?? 0,
)
const rowVirtualizer = useVirtualizer({
count: 1000,
getScrollElement: () => parentRef.current,
estimateSize: () => ESTIMATE,
initialOffset,
})

return (
<div
ref={parentRef}
id="scroll-container"
style={{ height: 400, overflow: 'auto' }}
>
<div
style={{
height: rowVirtualizer.getTotalSize(),
position: 'relative',
}}
>
{rowVirtualizer.getVirtualItems().map((v) => (
<div
key={v.key}
data-testid={`item-${v.index}`}
ref={rowVirtualizer.measureElement}
data-index={v.index}
style={{
position: 'absolute',
top: 0,
left: 0,
transform: `translateY(${v.start}px)`,
width: '100%',
}}
>
<div style={{ height: ITEM_SIZE }}>Row {v.index}</div>
</div>
))}
</div>
</div>
)
}

ReactDOM.createRoot(document.getElementById('root')!).render(<App />)
98 changes: 98 additions & 0 deletions packages/react-virtual/e2e/app/test/scroll-anchor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { expect, test } from '@playwright/test'

// Records the data-index and viewport-relative top of a rendered item that
// sits clearly inside the viewport (so it is already measured) with rows above
// it inside the viewport and room to stay visible after a moderate upward
// scroll moves it down. The window is wider than a row (rows are 90px) so it
// always contains at least one candidate; we pick the one nearest the target.
const pickAnchor = () => {
const container = document.querySelector('#scroll-container')
if (!container) throw new Error('Container not found')
const containerTop = container.getBoundingClientRect().top

const TARGET = 100
const candidates = Array.from(container.querySelectorAll('[data-index]'))
.map((el) => ({
index: Number(el.getAttribute('data-index')),
top: el.getBoundingClientRect().top - containerTop,
}))
// Away from both edges: rows above it, and room to move down ~200px.
.filter((c) => c.top > 30 && c.top < 170)
.sort((a, b) => Math.abs(a.top - TARGET) - Math.abs(b.top - TARGET))

if (candidates.length === 0) throw new Error('No anchor candidate found')
return candidates[0]
}

const topOfIndex = (index: number) => {
const container = document.querySelector('#scroll-container')
if (!container) throw new Error('Container not found')
const el = container.querySelector(`[data-index="${index}"]`)
if (!el) return null
return el.getBoundingClientRect().top - container.getBoundingClientRect().top
}

// Largest gap between consecutive rendered rows. Before measurement, rows are
// positioned from the 50px estimate while their real height is 90px, so they
// overlap (gap ~40px); once measured, positions are contiguous (gap ~0). This
// is a direct, pollable signal that measurement has settled. Returns a large
// finite sentinel (not Infinity — page.evaluate serializes that to null) when
// the DOM isn't ready yet so the poll keeps waiting.
const NOT_READY = 1e9
const maxRowGap = () => {
const container = document.querySelector('#scroll-container')
if (!container) return NOT_READY
const containerTop = container.getBoundingClientRect().top
const rows = Array.from(container.querySelectorAll('[data-index]'))
.map((el) => {
const rect = el.getBoundingClientRect()
return {
top: rect.top - containerTop,
bottom: rect.bottom - containerTop,
}
})
.sort((a, b) => a.top - b.top)
if (rows.length < 2) return NOT_READY
let gap = 0
for (let i = 1; i < rows.length; i++) {
gap = Math.max(gap, Math.abs(rows[i].top - rows[i - 1].bottom))
}
return gap
}

// Regression test for the "items jump while scrolling up" bug.
//
// When scrolling backward into never-measured items, their estimate→actual
// size delta lives above the viewport, so scrollTop MUST be compensated or the
// anchored content visibly jumps. The fix adjusts on first measurement even
// during backward scroll (it only skips compensation for RE-measurements while
// scrolling up). This test anchors on a visible item and asserts it moves down
// by exactly the scroll delta — no extra jump from uncompensated measurement.
test('anchored item stays stable when scrolling up into unmeasured items', async ({
page,
}) => {
await page.goto('/scroll-anchor/?initialOffset=10000')

// Wait for the initial measurement to settle (rows become contiguous).
await expect.poll(() => page.evaluate(maxRowGap)).toBeLessThan(1)

const anchor = await page.evaluate(pickAnchor)

const SCROLL_UP = 200
await page.evaluate((delta) => {
const container = document.querySelector('#scroll-container')
if (!container) throw new Error('Container not found')
container.scrollTop -= delta
}, SCROLL_UP)

// Poll until the anchor settles at its compensated position: it should have
// moved down by exactly the scroll delta. With the fix this converges; any
// extra shift is the uncompensated estimate→actual delta of the items
// measured above it (~40px per item) — the regression — and times out here.
await expect
.poll(async () => {
const top = await page.evaluate(topOfIndex, anchor.index)
return top === null ? Infinity : Math.abs(top - (anchor.top + SCROLL_UP))
})
.toBeLessThan(8)
})
1 change: 1 addition & 0 deletions packages/react-virtual/e2e/app/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default defineConfig({
rollupOptions: {
input: {
scroll: path.resolve(__dirname, 'scroll/index.html'),
'scroll-anchor': path.resolve(__dirname, 'scroll-anchor/index.html'),
chat: path.resolve(__dirname, 'chat/index.html'),
'measure-element': path.resolve(
__dirname,
Expand Down
15 changes: 8 additions & 7 deletions packages/virtual-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1519,14 +1519,15 @@ export class Virtualizer<
delta,
this,
)
: // Default: adjust scrollTop only when the resize is an above-
// viewport item AND we're not actively scrolling backward.
// Adjusting during backward scroll fights the user's scroll
// direction and produces the "items jump while scrolling up"
// jank reported across many issues. Users who want the old
// behavior can pass shouldAdjustScrollPositionOnItemSizeChange.
: // Default: adjust when the resize is an above-viewport item.
// First measurement (!has(key)): always adjust — the item
// has never been sized, so the estimate→actual delta must
// be compensated regardless of scroll direction.
// Re-measurement (has(key)): skip during backward scroll
// to avoid the "items jump while scrolling up" cascade.
itemStart < this.getScrollOffset() + this.scrollAdjustments &&
this.scrollDirection !== 'backward')
(!this.itemSizeCache.has(key) ||
this.scrollDirection !== 'backward'))

if (this.pendingMin === null || index < this.pendingMin) {
this.pendingMin = index
Expand Down
56 changes: 49 additions & 7 deletions packages/virtual-core/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2066,10 +2066,10 @@ test('non-iOS: adjustment is applied immediately during scroll (no regression)',
expect(v['_iosDeferredAdjustment']).toBe(0)
})

test('scroll-up jank: backward-scroll skips scroll-position adjustment by default', () => {
// Default behavior change: when an above-viewport item resizes while the
// user is scrolling BACKWARD, we no longer write to scrollTop. This avoids
// the well-known "items jump while scrolling up" jank.
test('scroll-up jank: backward-scroll skips adjustment on re-measurement by default', () => {
// Default behavior: when an already-measured above-viewport item resizes
// AGAIN while the user is scrolling BACKWARD, we no longer write to
// scrollTop. This avoids the well-known "items jump while scrolling up" jank.
const scrollToFn = vi.fn()
let scrollCb: ((o: number, s: boolean) => void) | null = null
const v = new Virtualizer({
Expand All @@ -2094,18 +2094,60 @@ test('scroll-up jank: backward-scroll skips scroll-position adjustment by defaul
})
v._willUpdate()
v['getMeasurements']()
// Seed item 0's size so the backward resize below is a RE-measurement.
v.resizeItem(0, 80)
// Now simulate backward scroll: from 200 to 100 (offset decreases).
scrollCb!(100, true)
expect(v.scrollDirection).toBe('backward')
scrollToFn.mockClear()

// Resize an above-viewport item while scrolling backward.
v.resizeItem(0, 100) // item 0 grows by 50px
// Re-measure an above-viewport item while scrolling backward.
v.resizeItem(0, 100) // item 0 grows by 20px

// Default behavior: no scroll-position adjustment fires.
// Default behavior: no scroll-position adjustment fires for re-measurements.
expect(scrollToFn).not.toHaveBeenCalled()
})

test('scroll-up jank: backward-scroll still applies adjustment on first measurement', () => {
// First measurement is special: the item was rendered at its estimate and is
// now reporting its actual size. That estimate→actual delta lives above the
// viewport and MUST be compensated, or the anchored content jumps when
// scrolling up into never-measured rows.
const scrollToFn = vi.fn()
let scrollCb: ((o: number, s: boolean) => void) | null = null
const v = new Virtualizer({
count: 10,
estimateSize: () => 50,
getScrollElement: () =>
({
scrollTop: 200,
scrollLeft: 0,
scrollHeight: 500,
clientHeight: 200,
offsetHeight: 200,
}) as any,
scrollToFn,
observeElementRect: () => {},
observeElementOffset: (_inst, cb) => {
scrollCb = cb
cb(200, false)
return () => {}
},
})
v._willUpdate()
v['getMeasurements']()
// Backward scroll: 200 → 100.
scrollCb!(100, true)
expect(v.scrollDirection).toBe('backward')
scrollToFn.mockClear()

// First measurement of an above-viewport item while scrolling backward.
v.resizeItem(0, 100) // never measured before → estimate(50)→actual(100)

// Adjustment still fires for the first measurement.
expect(scrollToFn).toHaveBeenCalled()
})

test('scroll-up jank: forward-scroll still applies adjustment (no regression)', () => {
const scrollToFn = vi.fn()
let scrollCb: ((o: number, s: boolean) => void) | null = null
Expand Down
Loading