fix: preserve sidebar scrolling while streaming
This commit is contained in:
committed by
nesquena-hermes
parent
b62f9dbbf8
commit
eeedccec58
BIN
docs/pr-media/1784/sidebar-scroll-fixture.png
Normal file
BIN
docs/pr-media/1784/sidebar-scroll-fixture.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 58 KiB |
25
docs/pr-media/1784/sidebar-scroll-qa.json
Normal file
25
docs/pr-media/1784/sidebar-scroll-qa.json
Normal file
@@ -0,0 +1,25 @@
|
||||
{
|
||||
"issue": 1784,
|
||||
"commit_under_test": "9875967",
|
||||
"fixture": "Synthetic 180-row session sidebar with active sid_0 streaming and long chat pane content.",
|
||||
"pre_fix_observation": {
|
||||
"steps": [
|
||||
"Set _scrollPinned=true with #messages at scrollTop 0 in a long chat fixture.",
|
||||
"Dispatch a wheel gesture on the active sidebar session row.",
|
||||
"Call scrollIfPinned() to mimic the next streaming token render."
|
||||
],
|
||||
"result": "#messages jumped from scrollTop 0 to 3073 immediately after the sidebar wheel gesture, showing the chat auto-scroll path fought non-chat scroll intent."
|
||||
},
|
||||
"post_fix_observation": {
|
||||
"steps": [
|
||||
"Repeat the same fixture and sidebar wheel gesture after the fix.",
|
||||
"Call scrollIfPinned() immediately, then again after the 350ms non-chat intent guard expires."
|
||||
],
|
||||
"result": {
|
||||
"afterSidebarWheel": 0,
|
||||
"afterIntentExpires": 2992,
|
||||
"sessionListCss": "overscroll-behavior-y: contain; touch-action: pan-y"
|
||||
},
|
||||
"meaning": "A sidebar wheel/touch scroll intent now suppresses only the immediate chat-pane auto-scroll write, leaving the sidebar gesture free while streaming continues."
|
||||
}
|
||||
}
|
||||
@@ -321,7 +321,7 @@
|
||||
.sidebar-section{padding:14px 14px 8px;}
|
||||
.new-chat-btn{width:100%;padding:9px 12px;border-radius:9px;background:var(--accent-bg);border:1px solid var(--accent-bg-strong);color:var(--accent-text);font-size:13px;cursor:pointer;display:flex;align-items:center;gap:8px;transition:all .15s;margin-bottom:8px;font-weight:500;}
|
||||
.new-chat-btn:hover{background:var(--accent-bg-strong);border-color:var(--accent);}
|
||||
.session-list{flex:1;overflow-y:auto;padding:0 8px 8px;min-height:0;}
|
||||
.session-list{flex:1;overflow-y:auto;padding:0 8px 8px;min-height:0;overscroll-behavior-y:contain;touch-action:pan-y;}
|
||||
.sidebar-search{position:relative;padding:8px 12px;flex-shrink:0;}
|
||||
.sidebar-search input{width:100%;background:var(--bg);border:1px solid var(--border);border-radius:8px;color:var(--text);padding:7px 10px 7px 32px;font-size:13px;outline:none;transition:border-color .15s,box-shadow .15s,background .15s;box-sizing:border-box;}
|
||||
.sidebar-search input:focus{border-color:var(--accent);box-shadow:0 0 0 3px var(--accent-bg);}
|
||||
|
||||
20
static/ui.js
20
static/ui.js
@@ -1472,6 +1472,25 @@ let _scrollPinned=true;
|
||||
let _programmaticScroll=false;
|
||||
let _nearBottomCount=0;
|
||||
let _lastScrollTop=null;
|
||||
let _lastNonMessageScrollIntentMs=0;
|
||||
const NON_MESSAGE_SCROLL_INTENT_SUPPRESS_MS=350;
|
||||
function _recordNonMessageScrollIntent(e){
|
||||
const el=document.getElementById('messages');
|
||||
const target=e&&e.target;
|
||||
if(!el||!target) return;
|
||||
// Streaming token renders should keep pinning the chat only while the user is
|
||||
// actually interacting with the chat pane. A wheel/touch gesture over the
|
||||
// session sidebar (or another independent pane) must not be immediately fought
|
||||
// by scrollIfPinned() writing #messages.scrollTop on the next token (#1784).
|
||||
if(!el.contains(target)) _lastNonMessageScrollIntentMs=performance.now();
|
||||
}
|
||||
function _recentNonMessageScrollIntent(){
|
||||
return performance.now()-_lastNonMessageScrollIntentMs<NON_MESSAGE_SCROLL_INTENT_SUPPRESS_MS;
|
||||
}
|
||||
if(typeof document!=='undefined'){
|
||||
document.addEventListener('wheel',_recordNonMessageScrollIntent,{capture:true,passive:true});
|
||||
document.addEventListener('touchmove',_recordNonMessageScrollIntent,{capture:true,passive:true});
|
||||
}
|
||||
// Reset hook for session-switch — called from sessions.js loadSession() to
|
||||
// prevent the new chat's first scroll comparing against the previous chat's
|
||||
// scrollTop (Opus stage-302 SHOULD-FIX, #1731 follow-up).
|
||||
@@ -1778,6 +1797,7 @@ document.addEventListener('DOMContentLoaded',function(){
|
||||
|
||||
function scrollIfPinned(){
|
||||
if(!_scrollPinned) return;
|
||||
if(_recentNonMessageScrollIntent()) return;
|
||||
const el=$('messages');
|
||||
if(el){_programmaticScroll=true;el.scrollTop=el.scrollHeight;setTimeout(()=>{_programmaticScroll=false;},0);}
|
||||
}
|
||||
|
||||
47
tests/test_streaming_sidebar_scroll.py
Normal file
47
tests/test_streaming_sidebar_scroll.py
Normal file
@@ -0,0 +1,47 @@
|
||||
"""Regression tests for #1784: sidebar scroll remains independent while streaming."""
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _extract_fn(src: str, name: str) -> str:
|
||||
marker = f"function {name}"
|
||||
start = src.find(marker)
|
||||
assert start >= 0, f"{name} not found"
|
||||
brace = src.find("{", start)
|
||||
assert brace >= 0, f"{name} body not found"
|
||||
depth = 0
|
||||
for i in range(brace, len(src)):
|
||||
ch = src[i]
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[start : i + 1]
|
||||
raise AssertionError(f"{name} body did not close")
|
||||
|
||||
|
||||
def test_sidebar_wheel_intent_is_recorded_passively():
|
||||
"""A sidebar wheel gesture must not be swallowed or ignored during streaming."""
|
||||
assert "_recordNonMessageScrollIntent" in UI_JS
|
||||
assert "document.addEventListener('wheel',_recordNonMessageScrollIntent" in UI_JS
|
||||
assert "{capture:true,passive:true}" in UI_JS
|
||||
assert "!el.contains(target)" in UI_JS
|
||||
assert "_lastNonMessageScrollIntentMs=performance.now()" in UI_JS
|
||||
|
||||
|
||||
def test_scroll_if_pinned_skips_during_recent_non_message_scroll():
|
||||
"""Token rendering must not force-scroll #messages while the sidebar is being scrolled."""
|
||||
fn = _extract_fn(UI_JS, "scrollIfPinned")
|
||||
assert "_recentNonMessageScrollIntent()" in fn
|
||||
guard_index = fn.find("_recentNonMessageScrollIntent()")
|
||||
write_index = fn.find("scrollTop=el.scrollHeight")
|
||||
assert guard_index >= 0 and write_index >= 0 and guard_index < write_index
|
||||
|
||||
|
||||
def test_session_list_has_its_own_scroll_boundary():
|
||||
"""The session list is its own scroll surface, not chained to the chat/body scroller."""
|
||||
assert ".session-list{flex:1;overflow-y:auto;padding:0 8px 8px;min-height:0;overscroll-behavior-y:contain;touch-action:pan-y;}" in STYLE_CSS
|
||||
Reference in New Issue
Block a user