Fix scroll position stability when items resize#12264
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR improves scroll position stability in the UI’s virtualized scrolling components, focusing on preventing scroll “jumps” and lag when list items resize (e.g., expanding diffs) and when scrolling quickly.
Changes:
- Removes the scrollbar thumb “top” transition to reduce lag during fast scrolling.
- Updates
VirtualListresize handling to (a) stick to bottom when the last visible item expands near the bottom and (b) only compensate scroll position when the topmost visible element resizes while scrolling up.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/ui/src/lib/components/scroll/Scrollbar.svelte | Removes top transition on the scrollbar thumb to avoid lag/jitter during rapid scroll updates. |
| packages/ui/src/lib/components/VirtualList.svelte | Refines resize compensation logic to reduce jitter when scrolling up and improve bottom-sticking behavior when near bottom. |
| // When scrolling up, compensate for the height change of the topmost | ||
| // visible element to prevent content from jumping downward. | ||
| viewport.scrollBy({ top: heightMap[index] - oldHeight }); | ||
| } else if ( | ||
| (lastJumpToIndex !== undefined || startIndex) && | ||
| lastScrollDirection === undefined | ||
| ) { | ||
| const newScrollTop = calculateHeightSum(0, lastJumpToIndex || startIndex || 0); | ||
| viewport.scrollTop = newScrollTop; | ||
| // After jumping to an index, maintain position as off-viewport elements | ||
| // resize. Scroll direction is undefined during jumps. | ||
| viewport.scrollTop = calculateHeightSum(0, lastJumpToIndex || startIndex || 0); | ||
| skipNextScrollEvent = true; | ||
| } else if (stickToBottom && previousDistance < STICKY_DISTANCE) { | ||
| } else if ( | ||
| (stickToBottom || index === visibleRange.end - 1) && | ||
| previousDistance < STICKY_DISTANCE | ||
| ) { | ||
| // Maintain bottom position when near bottom and either stickToBottom | ||
| // is enabled or the last visible item resizes. | ||
| skipNextScrollEvent = true; | ||
| scrollToBottom(); | ||
| } |
There was a problem hiding this comment.
The new scroll-stability behavior (auto-sticking to bottom when the last visible item resizes, and only compensating for the topmost visible item while scrolling up) isn’t covered by the existing VirtualList Playwright CT tests. Since packages/ui/tests/VirtualList.spec.ts exists, please add/adjust tests to exercise these resize scenarios (e.g., expand the last visible row via the wrapper’s “Expand Last” control and assert the scroll position remains stable both with and without stickToBottom).
- Stick to bottom when the last visible item expands (e.g. expanding a collapsed diff), not just when stickToBottom is enabled - Compensate scroll position only when the topmost visible element resizes, fixing jitter when scrolling up - Remove scroll thumb position transition that caused lag during fast scrolling
1 participant
collapsed diff), not just when stickToBottom is enabled
resizes, fixing jitter when scrolling up
fast scrolling