Merge pull request #4128 from nesquena/stage-4124
Release NC (v0.51.390): don't snap long final answers to bottom on PWA refresh (#4123)
This commit is contained in:
commit
5838f8c770
7 changed files with 68 additions and 8 deletions
|
|
@ -3,6 +3,12 @@
|
||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
|
## [v0.51.390] — 2026-06-13 — Release NC (don't snap long final answers to bottom on PWA refresh, #4123)
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- **Long final answers no longer snap back to the bottom while you are reading them in PWA/mobile mode (#4123).** Same-session background refreshes now treat a preserved scroll snapshot whose viewport is clearly away from the bottom (>250px) as an active reading position and restore it before any follow-to-bottom path runs, re-mark restored mid-answer positions as unpinned so later refreshes keep respecting the reader, and no longer reset the scroll-direction tracker on same-session force refreshes (that reset stays scoped to real session switches). (#4123)
|
||||||
|
|
||||||
## [v0.51.389] — 2026-06-13 — Release NB (surface dirty-install state in update check, #4085)
|
## [v0.51.389] — 2026-06-13 — Release NB (surface dirty-install state in update check, #4085)
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
|
||||||
|
|
@ -1038,10 +1038,13 @@ async function loadSession(sid){
|
||||||
if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session);
|
if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session);
|
||||||
S.session._modelResolutionDeferred=true;
|
S.session._modelResolutionDeferred=true;
|
||||||
S.lastUsage={...(data.session.last_usage||{})};
|
S.lastUsage={...(data.session.last_usage||{})};
|
||||||
// Reset scroll-direction tracker on session switch so the new chat's
|
// Reset scroll-direction tracker only on real session switches so the new
|
||||||
// first scroll doesn't compare against the previous chat's scrollTop
|
// chat's first scroll doesn't compare against the previous chat's scrollTop
|
||||||
// and false-trigger an unpin (#1731 follow-up — Opus stage-302 SHOULD-FIX).
|
// and false-trigger an unpin (#1731 follow-up — Opus stage-302 SHOULD-FIX).
|
||||||
if (typeof window !== 'undefined' && typeof window._resetScrollDirectionTracker === 'function') {
|
// Same-session force refreshes reuse the current transcript viewport; clearing
|
||||||
|
// the sticky-unpin state here makes preserveScroll treat a reader mid-answer
|
||||||
|
// as pinned and snap them back to the bottom on the next render.
|
||||||
|
if (currentSid !== sid && typeof window !== 'undefined' && typeof window._resetScrollDirectionTracker === 'function') {
|
||||||
try { window._resetScrollDirectionTracker(); } catch (_) {}
|
try { window._resetScrollDirectionTracker(); } catch (_) {}
|
||||||
}
|
}
|
||||||
if(typeof _applyPendingSessionModelForSession==='function') _applyPendingSessionModelForSession(sid);
|
if(typeof _applyPendingSessionModelForSession==='function') _applyPendingSessionModelForSession(sid);
|
||||||
|
|
|
||||||
17
static/ui.js
17
static/ui.js
|
|
@ -8718,6 +8718,16 @@ function _restoreMessageScrollSnapshot(snapshot){
|
||||||
el.scrollTop=Math.max(0,Math.min(Number(snapshot.top)||0,maxTop));
|
el.scrollTop=Math.max(0,Math.min(Number(snapshot.top)||0,maxTop));
|
||||||
// Sync _lastScrollTop after programmatic restore so sticky-unpin does not false-trigger (#1731).
|
// Sync _lastScrollTop after programmatic restore so sticky-unpin does not false-trigger (#1731).
|
||||||
_lastScrollTop=el.scrollTop;
|
_lastScrollTop=el.scrollTop;
|
||||||
|
const bottomDistance=el.scrollHeight-el.scrollTop-el.clientHeight;
|
||||||
|
if(bottomDistance>250){
|
||||||
|
_messageUserUnpinned=true;
|
||||||
|
_scrollPinned=false;
|
||||||
|
_nearBottomCount=0;
|
||||||
|
}else if(bottomDistance<=120){
|
||||||
|
_messageUserUnpinned=false;
|
||||||
|
_scrollPinned=true;
|
||||||
|
_nearBottomCount=2;
|
||||||
|
}
|
||||||
requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); });
|
requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); });
|
||||||
}
|
}
|
||||||
function _restoreMessageScrollSnapshotSameFrame(snapshot){
|
function _restoreMessageScrollSnapshotSameFrame(snapshot){
|
||||||
|
|
@ -8773,6 +8783,11 @@ function _scrollAfterMessageRender(preserveScroll, scrollSnapshot){
|
||||||
// pinned users stay at bottom; users who manually scrolled up get their
|
// pinned users stay at bottom; users who manually scrolled up get their
|
||||||
// pre-render scrollTop restored after the DOM replacement.
|
// pre-render scrollTop restored after the DOM replacement.
|
||||||
if(preserveScroll){
|
if(preserveScroll){
|
||||||
|
const readerAwayFromBottom=!!(
|
||||||
|
scrollSnapshot &&
|
||||||
|
Number.isFinite(Number(scrollSnapshot.bottom)) &&
|
||||||
|
Number(scrollSnapshot.bottom)>250
|
||||||
|
);
|
||||||
// Keep master's follow heuristic for pinned / still-near-bottom users:
|
// Keep master's follow heuristic for pinned / still-near-bottom users:
|
||||||
// _followMessagesAfterDomReplace() does a FORCED scrollToBottom() (synchronous
|
// _followMessagesAfterDomReplace() does a FORCED scrollToBottom() (synchronous
|
||||||
// bottom write + forced settle), so the final settled response can't leave a
|
// bottom write + forced settle), so the final settled response can't leave a
|
||||||
|
|
@ -8781,7 +8796,7 @@ function _scrollAfterMessageRender(preserveScroll, scrollSnapshot){
|
||||||
// new-message cue. (Using scrollIfPinned() here instead would skip the forced
|
// new-message cue. (Using scrollIfPinned() here instead would skip the forced
|
||||||
// write unless distance>500 and let the DOM-rebuild scroll event cancel the
|
// write unless distance>500 and let the DOM-rebuild scroll event cancel the
|
||||||
// delayed settles — Codex CORE catch on #3631.)
|
// delayed settles — Codex CORE catch on #3631.)
|
||||||
if(!_messageUserUnpinned && _followMessagesAfterDomReplace()) return;
|
if(!readerAwayFromBottom && !_messageUserUnpinned && _followMessagesAfterDomReplace()) return;
|
||||||
_restoreMessageScrollSnapshot(scrollSnapshot);
|
_restoreMessageScrollSnapshot(scrollSnapshot);
|
||||||
_maybeShowNewMessageScrollCue(scrollSnapshot);
|
_maybeShowNewMessageScrollCue(scrollSnapshot);
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
|
|
@ -57,8 +57,13 @@ def test_render_messages_preserve_scroll_option_uses_user_pin_state_not_stream_l
|
||||||
assert "const preserveScroll=!!(options&&options.preserveScroll);" in render_body
|
assert "const preserveScroll=!!(options&&options.preserveScroll);" in render_body
|
||||||
assert "_scrollAfterMessageRender(preserveScroll, scrollSnapshot);" in render_body
|
assert "_scrollAfterMessageRender(preserveScroll, scrollSnapshot);" in render_body
|
||||||
assert "const scrollSnapshot=(preserveScroll||(!_autoScrollFollow&&_messageUserUnpinned))?_captureMessageScrollSnapshot():null" in render_body
|
assert "const scrollSnapshot=(preserveScroll||(!_autoScrollFollow&&_messageUserUnpinned))?_captureMessageScrollSnapshot():null" in render_body
|
||||||
assert "if(preserveScroll){\n // Keep master's follow heuristic" in scroll_helper
|
assert "if(preserveScroll){" in scroll_helper
|
||||||
assert "if(!_messageUserUnpinned && _followMessagesAfterDomReplace()) return;\n _restoreMessageScrollSnapshot(scrollSnapshot);\n _maybeShowNewMessageScrollCue(scrollSnapshot);\n return;\n }" in scroll_helper
|
# #4124: a reader clearly away from the bottom (>250px) is treated as an active
|
||||||
|
# reading position, so the forced follow-to-bottom is gated behind it.
|
||||||
|
assert "const readerAwayFromBottom=" in scroll_helper
|
||||||
|
assert "Number(scrollSnapshot.bottom)>250" in scroll_helper
|
||||||
|
assert "// Keep master's follow heuristic" in scroll_helper
|
||||||
|
assert "if(!readerAwayFromBottom && !_messageUserUnpinned && _followMessagesAfterDomReplace()) return;\n _restoreMessageScrollSnapshot(scrollSnapshot);\n _maybeShowNewMessageScrollCue(scrollSnapshot);\n return;\n }" in scroll_helper
|
||||||
assert "_shouldFollowMessagesOnDomReplace()" in follow_helper
|
assert "_shouldFollowMessagesOnDomReplace()" in follow_helper
|
||||||
assert "scrollToBottom();" in follow_helper
|
assert "scrollToBottom();" in follow_helper
|
||||||
assert "if(S.activeStreamId){\n scrollIfPinned();\n return;\n }" in scroll_helper
|
assert "if(S.activeStreamId){\n scrollIfPinned();\n return;\n }" in scroll_helper
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ from pathlib import Path
|
||||||
|
|
||||||
ROOT = Path(__file__).resolve().parents[1]
|
ROOT = Path(__file__).resolve().parents[1]
|
||||||
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||||
|
SESSIONS_JS = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||||
STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8")
|
STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8")
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -78,6 +79,36 @@ def test_same_frame_snapshot_preserves_bottom_distance_and_unpinned_state():
|
||||||
assert "_restoreMessageScrollSnapshotSameFrame(scrollSnapshot);" in wrapper
|
assert "_restoreMessageScrollSnapshotSameFrame(scrollSnapshot);" in wrapper
|
||||||
|
|
||||||
|
|
||||||
|
def test_preserve_scroll_restores_reader_away_from_bottom_before_following():
|
||||||
|
body = _function_body(UI_JS, "_scrollAfterMessageRender")
|
||||||
|
compact = _compact(body)
|
||||||
|
|
||||||
|
reader_idx = compact.index("constreaderAwayFromBottom=")
|
||||||
|
follow_idx = compact.index("if(!readerAwayFromBottom&&!_messageUserUnpinned&&_followMessagesAfterDomReplace())return;")
|
||||||
|
restore_idx = compact.index("_restoreMessageScrollSnapshot(scrollSnapshot);")
|
||||||
|
|
||||||
|
assert "Number(scrollSnapshot.bottom)>250" in compact
|
||||||
|
assert reader_idx < follow_idx < restore_idx
|
||||||
|
|
||||||
|
|
||||||
|
def test_scroll_snapshot_restore_reinstates_unpinned_state_when_reader_is_mid_answer():
|
||||||
|
restore = _function_body(UI_JS, "_restoreMessageScrollSnapshot")
|
||||||
|
compact = _compact(restore)
|
||||||
|
|
||||||
|
assert "constbottomDistance=el.scrollHeight-el.scrollTop-el.clientHeight;" in compact
|
||||||
|
assert "if(bottomDistance>250)" in compact
|
||||||
|
assert "_messageUserUnpinned=true" in compact
|
||||||
|
assert "_scrollPinned=false" in compact
|
||||||
|
|
||||||
|
|
||||||
|
def test_same_session_force_refresh_does_not_reset_scroll_direction_tracker():
|
||||||
|
body = _function_body(SESSIONS_JS, "loadSession")
|
||||||
|
compact = _compact(body)
|
||||||
|
|
||||||
|
assert "constcurrentSid=S.session?S.session.session_id:null;" in compact
|
||||||
|
assert "if(currentSid!==sid&&typeofwindow!=='undefined'&&typeofwindow._resetScrollDirectionTracker==='function')" in compact
|
||||||
|
|
||||||
|
|
||||||
def test_clarify_card_is_height_clamped_and_scrollable_on_mobile_viewports():
|
def test_clarify_card_is_height_clamped_and_scrollable_on_mobile_viewports():
|
||||||
compact = _compact(STYLE_CSS)
|
compact = _compact(STYLE_CSS)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -34,7 +34,7 @@ def test_preserve_scroll_unpinned_branch_shows_new_message_cue_after_restore():
|
||||||
# only the genuinely-scrolled-up cohort restores their viewport + gets the
|
# only the genuinely-scrolled-up cohort restores their viewport + gets the
|
||||||
# new-message cue. (Codex CORE catch: scrollIfPinned() in the pinned branch
|
# new-message cue. (Codex CORE catch: scrollIfPinned() in the pinned branch
|
||||||
# could leave a pinned reader short of the settled response.)
|
# could leave a pinned reader short of the settled response.)
|
||||||
assert "if(!_messageUserUnpinned && _followMessagesAfterDomReplace()) return;" in helper
|
assert "if(!readerAwayFromBottom && !_messageUserUnpinned && _followMessagesAfterDomReplace()) return;" in helper
|
||||||
assert "_restoreMessageScrollSnapshot(scrollSnapshot);" in helper
|
assert "_restoreMessageScrollSnapshot(scrollSnapshot);" in helper
|
||||||
assert helper.index("_restoreMessageScrollSnapshot(scrollSnapshot)") < helper.index(
|
assert helper.index("_restoreMessageScrollSnapshot(scrollSnapshot)") < helper.index(
|
||||||
"_maybeShowNewMessageScrollCue(scrollSnapshot)"
|
"_maybeShowNewMessageScrollCue(scrollSnapshot)"
|
||||||
|
|
|
||||||
|
|
@ -116,7 +116,7 @@ def test_preserve_scroll_restores_unpinned_viewport_after_dom_rebuild():
|
||||||
"renderMessages({preserveScroll:true}) must capture #messages.scrollTop before "
|
"renderMessages({preserveScroll:true}) must capture #messages.scrollTop before "
|
||||||
"replacing transcript DOM, then pass that snapshot to the post-render scroll helper"
|
"replacing transcript DOM, then pass that snapshot to the post-render scroll helper"
|
||||||
)
|
)
|
||||||
assert "if(!_messageUserUnpinned && _followMessagesAfterDomReplace()) return;" in after_render
|
assert "if(!readerAwayFromBottom && !_messageUserUnpinned && _followMessagesAfterDomReplace()) return;" in after_render
|
||||||
assert "_restoreMessageScrollSnapshot(scrollSnapshot);\n _maybeShowNewMessageScrollCue(scrollSnapshot);" in after_render
|
assert "_restoreMessageScrollSnapshot(scrollSnapshot);\n _maybeShowNewMessageScrollCue(scrollSnapshot);" in after_render
|
||||||
assert "_shouldFollowMessagesOnDomReplace()" in follow
|
assert "_shouldFollowMessagesOnDomReplace()" in follow
|
||||||
assert "scrollToBottom();" in follow
|
assert "scrollToBottom();" in follow
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue