mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 19:20:16 +00:00
Stage 326: PR #1941 — fix: preserve chat scroll across final render by @ai-ag2026
This commit is contained in:
+22
-5
@@ -4674,12 +4674,28 @@ function clearMessageRenderCache(){
|
||||
_sessionHtmlCacheSid=null;
|
||||
}
|
||||
|
||||
function _scrollAfterMessageRender(preserveScroll){
|
||||
function _captureMessageScrollSnapshot(){
|
||||
const el=$('messages');
|
||||
if(!el) return null;
|
||||
return {top:el.scrollTop};
|
||||
}
|
||||
function _restoreMessageScrollSnapshot(snapshot){
|
||||
const el=$('messages');
|
||||
if(!el||!snapshot) return;
|
||||
const maxTop=Math.max(0,el.scrollHeight-el.clientHeight);
|
||||
_programmaticScroll=true;
|
||||
el.scrollTop=Math.max(0,Math.min(Number(snapshot.top)||0,maxTop));
|
||||
_lastScrollTop=el.scrollTop;
|
||||
requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); });
|
||||
}
|
||||
function _scrollAfterMessageRender(preserveScroll, scrollSnapshot){
|
||||
// Terminal stream renders can happen after S.activeStreamId is cleared.
|
||||
// In that case, preserveScroll asks the normal pin-state helper to decide:
|
||||
// pinned users stay at bottom; users who manually scrolled up stay put.
|
||||
// pinned users stay at bottom; users who manually scrolled up get their
|
||||
// pre-render scrollTop restored after the DOM replacement.
|
||||
if(preserveScroll){
|
||||
scrollIfPinned();
|
||||
if(_scrollPinned) scrollIfPinned();
|
||||
else _restoreMessageScrollSnapshot(scrollSnapshot);
|
||||
return;
|
||||
}
|
||||
if(S.activeStreamId){
|
||||
@@ -4691,6 +4707,7 @@ function _scrollAfterMessageRender(preserveScroll){
|
||||
|
||||
function renderMessages(options){
|
||||
const preserveScroll=!!(options&&options.preserveScroll);
|
||||
const scrollSnapshot=preserveScroll?_captureMessageScrollSnapshot():null;
|
||||
const inner=$('msgInner');
|
||||
const sid=S.session?S.session.session_id:null;
|
||||
const msgCount=S.messages.length;
|
||||
@@ -4716,7 +4733,7 @@ function renderMessages(options){
|
||||
_sessionHtmlCacheSid=sid;
|
||||
_wireMessageWindowLoadEarlierButton();
|
||||
if(typeof _applySessionNavigationPrefs==='function') _applySessionNavigationPrefs();
|
||||
_scrollAfterMessageRender(preserveScroll);
|
||||
_scrollAfterMessageRender(preserveScroll, scrollSnapshot);
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();loadDiffInline();loadCsvInline();loadExcalidrawInline();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();initTreeViews();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
if(typeof _initMediaPlaybackObserver==='function') _initMediaPlaybackObserver();
|
||||
@@ -5256,7 +5273,7 @@ function renderMessages(options){
|
||||
// Only force-scroll when not actively streaming — mid-stream re-renders
|
||||
// (tool completion, session switch) must not override the user's scroll position.
|
||||
// scrollIfPinned() respects _scrollPinned, so it's a no-op if user scrolled up.
|
||||
_scrollAfterMessageRender(preserveScroll);
|
||||
_scrollAfterMessageRender(preserveScroll, scrollSnapshot);
|
||||
// Apply syntax highlighting after DOM is built
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();loadDiffInline();loadCsvInline();loadExcalidrawInline();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
requestAnimationFrame(()=>{highlightCode();addCopyButtons();initTreeViews();loadPdfInline();loadHtmlInline();renderMermaidBlocks();renderKatexBlocks();});
|
||||
|
||||
@@ -54,8 +54,9 @@ def test_render_messages_preserve_scroll_option_uses_user_pin_state_not_stream_l
|
||||
|
||||
assert "function renderMessages(options)" in render_body
|
||||
assert "const preserveScroll=!!(options&&options.preserveScroll);" in render_body
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in render_body
|
||||
assert "if(preserveScroll){\n scrollIfPinned();\n return;\n }" in scroll_helper
|
||||
assert "_scrollAfterMessageRender(preserveScroll, scrollSnapshot);" in render_body
|
||||
assert "const scrollSnapshot=preserveScroll?_captureMessageScrollSnapshot():null" in render_body
|
||||
assert "if(preserveScroll){\n if(_scrollPinned) scrollIfPinned();\n else _restoreMessageScrollSnapshot(scrollSnapshot);\n return;\n }" in scroll_helper
|
||||
assert "if(S.activeStreamId){\n scrollIfPinned();\n return;\n }" in scroll_helper
|
||||
|
||||
|
||||
@@ -63,7 +64,7 @@ def test_cached_render_path_uses_same_scroll_policy_as_fresh_render():
|
||||
render_body = _function_body(UI_JS, "renderMessages")
|
||||
cached_branch = render_body[render_body.index("if(sid&&sid!==_sessionHtmlCacheSid") : render_body.index("const compressionState=")]
|
||||
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in cached_branch
|
||||
assert "_scrollAfterMessageRender(preserveScroll, scrollSnapshot);" in cached_branch
|
||||
assert "if(S.activeStreamId){scrollIfPinned();}else{scrollToBottom();}" not in cached_branch
|
||||
|
||||
|
||||
|
||||
@@ -40,7 +40,7 @@ class TestScrollPinningFix:
|
||||
"unconditional scrollToBottom() overrides user scroll position (#677)"
|
||||
)
|
||||
# scrollIfPinned must be called through the renderMessages scroll policy (stream path)
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in rm_body
|
||||
assert "_scrollAfterMessageRender(preserveScroll, scrollSnapshot);" in rm_body
|
||||
assert "scrollIfPinned()" in helper_body, (
|
||||
"renderMessages() must call scrollIfPinned() during streaming (#677)"
|
||||
)
|
||||
|
||||
@@ -24,7 +24,7 @@ def test_load_earlier_expands_local_window_before_server_pagination_and_preserve
|
||||
|
||||
|
||||
def test_windowed_render_keeps_streaming_and_tool_activity_anchored_to_rendered_messages():
|
||||
assert "_scrollAfterMessageRender(preserveScroll);" in UI_JS
|
||||
assert "_scrollAfterMessageRender(preserveScroll, scrollSnapshot);" in UI_JS
|
||||
assert "const assistantIdxs=[...assistantSegments.keys()].sort((a,b)=>a-b);" in UI_JS
|
||||
assert "if(aIdx<assistantIdxs[0]) continue;" in UI_JS
|
||||
assert "const renderedAssistantIdxs=[...assistantSegments.keys()].sort((a,b)=>a-b);" in UI_JS
|
||||
|
||||
@@ -84,3 +84,22 @@ def test_user_scroll_cancels_delayed_bottom_settling():
|
||||
assert "e.deltaY<0" in record
|
||||
assert "_cancelBottomSettle();" in record
|
||||
assert "_scrollPinned=false" in record
|
||||
|
||||
|
||||
def test_preserve_scroll_restores_unpinned_viewport_after_dom_rebuild():
|
||||
render = _function_body(UI_JS, "function renderMessages")
|
||||
after_render = _function_body(UI_JS, "function _scrollAfterMessageRender")
|
||||
restore = _function_body(UI_JS, "function _restoreMessageScrollSnapshot")
|
||||
|
||||
snapshot_idx = render.index("const scrollSnapshot=preserveScroll?_captureMessageScrollSnapshot():null")
|
||||
inner_idx = render.index("const inner=$('msgInner')")
|
||||
final_scroll_idx = render.rindex("_scrollAfterMessageRender(preserveScroll, scrollSnapshot)")
|
||||
|
||||
assert snapshot_idx < inner_idx < final_scroll_idx, (
|
||||
"renderMessages({preserveScroll:true}) must capture #messages.scrollTop before "
|
||||
"replacing transcript DOM, then pass that snapshot to the post-render scroll helper"
|
||||
)
|
||||
assert "if(_scrollPinned) scrollIfPinned()" in after_render
|
||||
assert "else _restoreMessageScrollSnapshot(scrollSnapshot)" in after_render
|
||||
assert "el.scrollTop=Math.max(0,Math.min(Number(snapshot.top)||0,maxTop))" in restore
|
||||
assert "_programmaticScroll=true" in restore
|
||||
|
||||
Reference in New Issue
Block a user