diff --git a/static/sessions.js b/static/sessions.js index e0e07bf7..1fdf6ff2 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -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) diff --git a/tests/test_issue1669_sidebar_scroll_jump_fix.py b/tests/test_issue1669_sidebar_scroll_jump_fix.py new file mode 100644 index 00000000..848656cf --- /dev/null +++ b/tests/test_issue1669_sidebar_scroll_jump_fix.py @@ -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