fix: preserve viewport when loading older messages
This commit is contained in:
parent
dec2d25fcc
commit
018d491570
3 changed files with 84 additions and 16 deletions
|
|
@ -1020,18 +1020,32 @@ async function _loadOlderMessages() {
|
|||
const container = $('messages');
|
||||
const prevScrollH = container ? container.scrollHeight : 0;
|
||||
S.messages = [...olderMsgs, ...S.messages];
|
||||
// renderMessages() windows long transcripts from the end. If we do not
|
||||
// expand that window before rendering, the newly prepended page stays
|
||||
// hidden and the "hidden" counter rises while the viewport appears stuck.
|
||||
// Count roughly by the same visible-message rules used by renderMessages().
|
||||
const addedRenderable = olderMsgs.filter(m=>{
|
||||
if(!m||!m.role||m.role==='tool') return false;
|
||||
if(typeof _isContextCompactionMessage==='function'&&_isContextCompactionMessage(m)) return false;
|
||||
if(typeof _isPreservedCompressionTaskListMessage==='function'&&_isPreservedCompressionTaskListMessage(m)) return false;
|
||||
const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0;
|
||||
const hasTu=Array.isArray(m.content)&&m.content.some(p=>p&&p.type==='tool_use');
|
||||
return !!(msgContent(m)||m._statusCard||m.attachments?.length||(m.role==='assistant'&&(hasTc||hasTu||(typeof _messageHasReasoningPayload==='function'&&_messageHasReasoningPayload(m)))));
|
||||
}).length;
|
||||
_messageRenderWindowSize=_currentMessageRenderWindowSize()+Math.max(addedRenderable, MESSAGE_RENDER_WINDOW_DEFAULT);
|
||||
_messagesTruncated = !!data.session._messages_truncated;
|
||||
_oldestIdx = data.session._messages_offset || 0;
|
||||
renderMessages();
|
||||
// Restore scroll position so the user stays at the same message.
|
||||
// renderMessages() calls scrollToBottom() at the end, so we must
|
||||
// counter-scroll to where the user was before loading older messages.
|
||||
renderMessages({ preserveScroll: true });
|
||||
if (container) {
|
||||
// Prepending older messages must not teleport the reader. Preserve the
|
||||
// currently visible viewport by adding the inserted height to scrollTop.
|
||||
const oldTop = container.scrollTop;
|
||||
const newScrollH = container.scrollHeight;
|
||||
container.scrollTop = newScrollH - prevScrollH;
|
||||
const addedHeight = Math.max(0, newScrollH - prevScrollH);
|
||||
_programmaticScroll = true;
|
||||
container.scrollTop = oldTop + addedHeight;
|
||||
requestAnimationFrame(()=>{ _programmaticScroll = false; });
|
||||
}
|
||||
// renderMessages() called scrollToBottom() which set _scrollPinned=true.
|
||||
// We just restored the user's scroll position, so mark as not pinned.
|
||||
_scrollPinned = false;
|
||||
} catch(e) {
|
||||
console.warn('_loadOlderMessages failed:', e);
|
||||
|
|
|
|||
55
tests/test_older_history_viewport_preservation.py
Normal file
55
tests/test_older_history_viewport_preservation.py
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _function_body(src: str, signature: str) -> str:
|
||||
start = src.index(signature)
|
||||
brace = src.index("{", start)
|
||||
depth = 0
|
||||
for i in range(brace, len(src)):
|
||||
if src[i] == "{":
|
||||
depth += 1
|
||||
elif src[i] == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[start : i + 1]
|
||||
raise AssertionError(f"function body not found: {signature}")
|
||||
|
||||
|
||||
def test_loading_older_messages_expands_render_window_before_rendering():
|
||||
body = _function_body(SESSIONS_JS, "async function _loadOlderMessages")
|
||||
|
||||
prepend_idx = body.index("S.messages = [...olderMsgs, ...S.messages]")
|
||||
expand_idx = body.index("_messageRenderWindowSize=_currentMessageRenderWindowSize()")
|
||||
render_idx = body.index("renderMessages({ preserveScroll: true });")
|
||||
|
||||
assert prepend_idx < expand_idx < render_idx, (
|
||||
"scroll-to-top paging must expand the DOM render window before renderMessages(); "
|
||||
"otherwise fetched older messages stay hidden and only the hidden counter changes"
|
||||
)
|
||||
assert "Math.max(addedRenderable, MESSAGE_RENDER_WINDOW_DEFAULT)" in body
|
||||
|
||||
|
||||
def test_loading_older_messages_preserves_viewport_without_bottom_snap():
|
||||
body = _function_body(SESSIONS_JS, "async function _loadOlderMessages")
|
||||
|
||||
assert "renderMessages({ preserveScroll: true });" in body
|
||||
assert "const oldTop = container.scrollTop" in body
|
||||
assert "const addedHeight = Math.max(0, newScrollH - prevScrollH)" in body
|
||||
assert "container.scrollTop = oldTop + addedHeight" in body
|
||||
assert "container.scrollTop = newScrollH - prevScrollH" not in body
|
||||
|
||||
restore_idx = body.index("container.scrollTop = oldTop + addedHeight")
|
||||
unpin_idx = body.rindex("_scrollPinned = false")
|
||||
assert restore_idx < unpin_idx
|
||||
|
||||
|
||||
def test_loading_older_messages_marks_scroll_programmatic_while_anchoring():
|
||||
body = _function_body(SESSIONS_JS, "async function _loadOlderMessages")
|
||||
|
||||
set_idx = body.index("_programmaticScroll = true;")
|
||||
restore_idx = body.index("container.scrollTop = oldTop + addedHeight")
|
||||
clear_idx = body.index("requestAnimationFrame(()=>{ _programmaticScroll = false; })")
|
||||
assert set_idx < restore_idx < clear_idx
|
||||
|
|
@ -589,7 +589,7 @@ class TestScrollPositionPreservation:
|
|||
)
|
||||
|
||||
def test_resets_scroll_pinned_after_restore(self):
|
||||
"""_scrollPinned must be set to false after restoring scroll position."""
|
||||
"""_scrollPinned must be false after older-history scroll anchoring."""
|
||||
SESSIONS_JS = pathlib.Path(__file__).parent.parent / "static" / "sessions.js"
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
|
||||
|
|
@ -598,13 +598,12 @@ class TestScrollPositionPreservation:
|
|||
fn_body = src[fn_start:fn_end]
|
||||
|
||||
assert "_scrollPinned = false" in fn_body, (
|
||||
"renderMessages() calls scrollToBottom() which sets _scrollPinned=true. "
|
||||
"After restoring the user's scroll position we must set _scrollPinned=false "
|
||||
"to prevent the next render from snapping back to the bottom."
|
||||
"Older-history paging must leave the transcript unpinned so the next "
|
||||
"render does not snap back to the newest output."
|
||||
)
|
||||
# _scrollPinned must appear after the scrollTop restore
|
||||
restore_idx = fn_body.find("container.scrollTop = newScrollH - prevScrollH")
|
||||
pinned_idx = fn_body.find("_scrollPinned = false")
|
||||
assert restore_idx >= 0 and pinned_idx >= 0 and restore_idx < pinned_idx, (
|
||||
"_scrollPinned = false must appear AFTER the scrollTop restore."
|
||||
target_idx = fn_body.find("container.scrollTop = oldTop + addedHeight")
|
||||
scroll_idx = fn_body.find("requestAnimationFrame(()=>{ _programmaticScroll = false; })")
|
||||
pinned_idx = fn_body.rfind("_scrollPinned = false")
|
||||
assert target_idx >= 0 and scroll_idx >= 0 and pinned_idx >= 0 and target_idx < scroll_idx < pinned_idx, (
|
||||
"_scrollPinned = false must appear AFTER the older-history viewport-preserve scroll."
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue