fix(sidebar): scroll jumps back to 0 on small lists (≤80 sessions) — #1669 follow-up

PR #1669 added DOM virtualization to renderSessionListFromCache() with two issues
for lists below the virtualization threshold (≤80 rows):

1. The unconditional scroll listener triggered renderSessionListFromCache() on
   every rAF, rebuilding the entire list DOM on every scroll event.
2. After each rebuild, scrollTop was only restored when virtualWindow.virtualized
   was true (i.e. total > 80). For lists ≤ 80 rows, scrollTop dropped to 0 on
   every scroll event, producing a 'scroll keeps jumping back' feel.

Fix:
- Always restore scrollTop after re-render when listScrollTopBeforeRender > 0
  (regardless of virtualized flag).
- Short-circuit _scheduleSessionVirtualizedRender when total <=
  SESSION_VIRTUAL_THRESHOLD_ROWS (saves wasteful rebuild on small lists).

Live verified on a 56-session sidebar: scrollTop holds across animation frames.
3 regression tests pin the fix shape.
This commit is contained in:
Nathan Esquenazi
2026-05-05 02:02:54 +00:00
parent 136d858963
commit 4e9ec6f191
2 changed files with 104 additions and 1 deletions
+17 -1
View File
@@ -1936,6 +1936,16 @@ function _sessionVirtualSpacer(height, where){
function _scheduleSessionVirtualizedRender(){
if(_renamingSid||_sessionVirtualScrollRaf) return;
// Skip the re-render if the list is below the virtualization threshold —
// there's no virtual window to recompute, and re-rendering would just
// rebuild the whole DOM on every scroll tick. Without this guard, the
// unconditional scroll listener (attached for any list) caused
// user-facing scroll jumps on small lists. (#1669 follow-up)
const list=_sessionVirtualScrollList;
if(list){
const total=Number(list.dataset.sessionVirtualTotal||0);
if(total>0&&total<=SESSION_VIRTUAL_THRESHOLD_ROWS) return;
}
_sessionVirtualScrollRaf=requestAnimationFrame(()=>{_sessionVirtualScrollRaf=0;renderSessionListFromCache();});
}
@@ -2202,7 +2212,13 @@ function renderSessionListFromCache(){
}
if(virtualAnchorScrollTop!==null){
list.scrollTop=virtualAnchorScrollTop;
}else if(virtualWindow.virtualized){
}else if(listScrollTopBeforeRender>0){
// Always restore the user's scroll position after re-render, regardless
// of whether the virtualization window applies. Lists below the
// virtualization threshold (≤80 rows) still have their DOM rebuilt by
// every renderSessionListFromCache() call, and without this restore the
// scrollTop drops to 0 — producing a "scroll keeps jumping back" feel
// when the list scrolls naturally. Fixed for #1669 follow-up.
list.scrollTop=listScrollTopBeforeRender;
}
// Select mode toggle button (only when NOT in select mode)
@@ -0,0 +1,87 @@
"""Regression test for #1669 follow-up — sidebar scroll jump fix.
The original PR #1669 added DOM virtualization to renderSessionListFromCache,
which:
1. Attached an unconditional scroll listener to the session list
2. The scroll listener triggers renderSessionListFromCache() on every rAF
3. The render rebuilds the list DOM via list.innerHTML='' / appendChild loop
4. After the rebuild, scrollTop was only restored when virtualWindow.virtualized
was true (i.e. total > 80 rows)
5. For lists 80 rows, the scrollTop reset to 0 on every scroll event,
producing a "scroll keeps jumping back" feel.
This test pins:
- The non-virtualized branch always restores scrollTop after a rebuild
- The scroll handler short-circuits when total <= threshold (prevents the
rebuild churn entirely on small lists)
"""
from pathlib import Path
SESSIONS_JS = Path(__file__).parent.parent / "static" / "sessions.js"
def _read_source():
return SESSIONS_JS.read_text()
def test_render_restores_scroll_top_for_non_virtualized_lists():
"""The bug: virtualWindow.virtualized=false skipped the scrollTop restore.
The fix: restore scrollTop whenever listScrollTopBeforeRender > 0,
regardless of virtualized flag. Otherwise small lists (80 rows) reset
to scrollTop=0 on every render.
"""
src = _read_source()
# The new branch must include listScrollTopBeforeRender>0 as the guard
# rather than virtualWindow.virtualized
assert "}else if(listScrollTopBeforeRender>0){" in src, (
"Expected the scrollTop-restore guard to use listScrollTopBeforeRender>0, "
"not virtualWindow.virtualized — without this fix, small lists drop "
"scrollTop to 0 on every scroll event."
)
def test_scroll_handler_short_circuits_below_virtualization_threshold():
"""The bug: the rAF re-render fired on every scroll event regardless of
whether virtualization was actually needed. For 80-row lists this caused
full DOM rebuild on every scroll tick.
The fix: _scheduleSessionVirtualizedRender skips the rebuild when
total <= SESSION_VIRTUAL_THRESHOLD_ROWS there's no virtual window to
recompute on small lists, and the rebuild was wasteful (and bug-prone).
"""
src = _read_source()
# Locate the function body
start = src.find("function _scheduleSessionVirtualizedRender()")
end = src.find("function _ensureSessionVirtualScrollHandler", start)
body = src[start:end]
# The fix introduces an early-return when total <= SESSION_VIRTUAL_THRESHOLD_ROWS
assert "SESSION_VIRTUAL_THRESHOLD_ROWS" in body, (
"Expected _scheduleSessionVirtualizedRender to read the threshold; "
"without this guard, the rAF re-render fires on every scroll event "
"even when there's nothing to virtualize."
)
assert "total<=SESSION_VIRTUAL_THRESHOLD_ROWS" in body or "total <= SESSION_VIRTUAL_THRESHOLD_ROWS" in body, (
"Expected explicit total<=THRESHOLD comparison to short-circuit the re-render."
)
# The early return must be BEFORE the rAF schedule (else it's dead code)
early_return_idx = body.find("return")
raf_idx = body.find("requestAnimationFrame")
assert early_return_idx > 0 and early_return_idx < raf_idx, (
"The total<=THRESHOLD short-circuit must return BEFORE scheduling the rAF."
)
def test_virtualization_still_active_for_large_lists():
"""Regression: ensure the threshold + virtualWindow logic is still in place
for large lists. The fix must not break the original virtualization path.
"""
src = _read_source()
assert "SESSION_VIRTUAL_THRESHOLD_ROWS = 80" in src, (
"Threshold constant must remain at 80 rows."
)
# _sessionVirtualWindow function still defined
assert "function _sessionVirtualWindow" in src
# virtualWindow.virtualized branch still drives spacer rendering
assert "virtualWindow.virtualized" in src