mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 11:10:18 +00:00
fix: _loadOlderMessages scrolls to bottom instead of preserving position
When _loadOlderMessages prepends older messages, the viewport snaps
to the bottom instead of staying where the user was.
Two bugs compounding:
1. Wrong scrollable container. Code used `$("msgInner")` for scrollHeight
and scrollTop, but #msgInner has no overflow-y — it is a flex column.
The actual scrollable container is #messages (`.messages{overflow-y:auto}`).
Setting msgInner.scrollTop was silently ignored.
2. renderMessages calls scrollToBottom at the end (ui.js:2552),
which unconditionally scrolls #messages to the bottom and sets
_scrollPinned=true. Since bug #1 made the scroll-restore a no-op,
the page landed at the bottom every time.
Fix:
- Changed scroll restore target from `$("msgInner")` to `$("messages")`.
- Reset _scrollPinned = false after restoring the user position,
so scrollToBottom does not re-fire on next tick.
This commit is contained in:
+12
-6
@@ -385,17 +385,23 @@ async function _loadOlderMessages() {
|
||||
const olderMsgs = (data.session.messages || []).filter(m => m && m.role);
|
||||
if (!olderMsgs.length) { _messagesTruncated = false; return; }
|
||||
// Prepend older messages
|
||||
const inner = $('msgInner');
|
||||
const prevScrollH = inner ? inner.scrollHeight : 0;
|
||||
// Use $('messages') — the scrollable container (#msgInner is not scrollable).
|
||||
const container = $('messages');
|
||||
const prevScrollH = container ? container.scrollHeight : 0;
|
||||
S.messages = [...olderMsgs, ...S.messages];
|
||||
_messagesTruncated = !!data.session._messages_truncated;
|
||||
_oldestIdx = data.session._messages_offset || 0;
|
||||
renderMessages();
|
||||
// Restore scroll position so the user stays at the same message
|
||||
if (inner) {
|
||||
const newScrollH = inner.scrollHeight;
|
||||
inner.scrollTop = newScrollH - prevScrollH;
|
||||
// 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.
|
||||
if (container) {
|
||||
const newScrollH = container.scrollHeight;
|
||||
container.scrollTop = newScrollH - prevScrollH;
|
||||
}
|
||||
// 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);
|
||||
} finally {
|
||||
|
||||
@@ -557,3 +557,54 @@ class TestSessionSwitchCancellation:
|
||||
assert active_check_idx >= 0 and mutation_idx >= 0 and active_check_idx < mutation_idx, (
|
||||
"Active-session guard must run before S.messages mutation."
|
||||
)
|
||||
|
||||
|
||||
# ── 6. Scroll position preservation ──────────────────────────────────────────
|
||||
|
||||
|
||||
class TestScrollPositionPreservation:
|
||||
"""When _loadOlderMessages prepends messages, the user's scroll position
|
||||
must be preserved — not snapped to the bottom.
|
||||
|
||||
The scrollable container is #messages (overflow-y:auto), not #msgInner
|
||||
(which is a flex column with no overflow). Also, renderMessages() calls
|
||||
scrollToBottom() at the end, so _scrollPinned must be reset."""
|
||||
|
||||
def test_uses_correct_scrollable_container(self):
|
||||
"""_loadOlderMessages must use $('messages') not $('msgInner')."""
|
||||
SESSIONS_JS = pathlib.Path(__file__).parent.parent / "static" / "sessions.js"
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
|
||||
fn_start = src.find("async function _loadOlderMessages")
|
||||
fn_end = src.find("\n}", fn_start) + 2
|
||||
fn_body = src[fn_start:fn_end]
|
||||
|
||||
assert "$('messages')" in fn_body, (
|
||||
"_loadOlderMessages should use $('messages') as the scrollable container "
|
||||
"(#messages has overflow-y:auto). #msgInner has no overflow and is not scrollable."
|
||||
)
|
||||
assert "$('msgInner')" not in fn_body, (
|
||||
"_loadOlderMessages must NOT use $('msgInner') for scroll position — "
|
||||
"#msgInner is a flex column with no overflow-y."
|
||||
)
|
||||
|
||||
def test_resets_scroll_pinned_after_restore(self):
|
||||
"""_scrollPinned must be set to false after restoring scroll position."""
|
||||
SESSIONS_JS = pathlib.Path(__file__).parent.parent / "static" / "sessions.js"
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
|
||||
fn_start = src.find("async function _loadOlderMessages")
|
||||
fn_end = src.find("\n}", fn_start) + 2
|
||||
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."
|
||||
)
|
||||
# _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."
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user