Merge #5253 (franksong2702/fix-live-worklog-manual-scroll) into stage-w2
This commit is contained in:
commit
4a7159a2ee
4 changed files with 211 additions and 6 deletions
28
static/ui.js
28
static/ui.js
|
|
@ -4038,6 +4038,7 @@ const MESSAGE_TOUCH_SCROLL_SUPPRESS_MS=1200;
|
|||
// post-render artifact suppression would swallow it for the whole window.
|
||||
const MESSAGE_WHEEL_INTENT_SUPPRESS_MS=1200;
|
||||
let _lastMessageWheelIntentMs=-Infinity;
|
||||
let _lastMessageScrollIntentMs=-Infinity;
|
||||
// #4970 review (greptile P1): keyboard scrolling of the message pane (PageUp/Down,
|
||||
// arrows, Space, Home/End) fires a native `scroll` event with NO wheel/touch/
|
||||
// scrollbar/non-message intent. Without recording it, a keyboard scroll-up inside
|
||||
|
|
@ -4065,6 +4066,13 @@ function _recentMessageTouchScrollIntent(){
|
|||
function _recentMessageWheelIntent(){
|
||||
return performance.now()-_lastMessageWheelIntentMs<MESSAGE_WHEEL_INTENT_SUPPRESS_MS;
|
||||
}
|
||||
function _recentMessageScrollIntent(){
|
||||
// This manual-reader snapshot signal intentionally excludes the raw
|
||||
// touch/key recency helpers: those also record near-tail events for render
|
||||
// artifact suppression. Only this timestamp is guarded by bottom distance.
|
||||
return performance.now()-_lastMessageScrollIntentMs<MESSAGE_WHEEL_INTENT_SUPPRESS_MS
|
||||
|| (typeof _scrollbarDragActive!=='undefined'&&!!_scrollbarDragActive);
|
||||
}
|
||||
// #4970 review (greptile P1): true when the reader recently used the keyboard to
|
||||
// scroll the message pane. Keyboard scrolls fire a native scroll event with no
|
||||
// wheel/touch intent, so the post-render artifact suppression must skip them.
|
||||
|
|
@ -4102,6 +4110,10 @@ function _recordNonMessageScrollIntent(e){
|
|||
// suppression consults _recentMessageWheelIntent() so it cannot swallow a real
|
||||
// gentle scroll-up. This does NOT unpin on its own — only the <-30 branch and
|
||||
// the scroll listener's movedUp branch flip _messageUserUnpinned.
|
||||
if(e.type==='touchmove'||(typeof e.deltaY==='number'&&e.deltaY!==0)){
|
||||
const bottomDistance=el.scrollHeight-el.scrollTop-el.clientHeight;
|
||||
if(bottomDistance>120) _lastMessageScrollIntentMs=performance.now();
|
||||
}
|
||||
if(typeof e.deltaY==='number'&&e.deltaY<0) _lastMessageWheelIntentMs=performance.now();
|
||||
if(e.type==='touchmove'||(typeof e.deltaY==='number'&&e.deltaY< -30)){
|
||||
_cancelBottomSettle();
|
||||
|
|
@ -4197,6 +4209,7 @@ function _resetScrollDirectionTracker(){
|
|||
// into the new chat's first post-render window — the artifact then isn't
|
||||
// suppressed, falls into movedUp, and falsely unpins live-follow.
|
||||
_lastMessageWheelIntentMs=-Infinity;
|
||||
_lastMessageScrollIntentMs=-Infinity;
|
||||
// #4970 review (greptile P1): same hygiene for keyboard scroll intent.
|
||||
_lastMessageKeyScrollIntentMs=-Infinity;
|
||||
clearTimeout(_deferredOlderMessagesTimer);
|
||||
|
|
@ -4212,6 +4225,7 @@ function _resetStreamScrollFollow(){
|
|||
// gentle upward wheel within the prior 1200ms can under-suppress a genuine
|
||||
// no-intent render artifact and silently disable live follow for the new stream.
|
||||
_lastMessageWheelIntentMs=-Infinity;
|
||||
_lastMessageScrollIntentMs=-Infinity;
|
||||
// #4970 review (greptile P1): same hygiene for keyboard scroll intent.
|
||||
_lastMessageKeyScrollIntentMs=-Infinity;
|
||||
_cancelBottomSettle();
|
||||
|
|
@ -4338,7 +4352,10 @@ if(typeof window!=='undefined'){
|
|||
// Count only when the message pane itself is the scroll target: it is focused,
|
||||
// contains the focus, or the pointer is over it (keyboard scroll w/o focus).
|
||||
if(a===el||el.contains(a)||el.matches(':hover')){
|
||||
_lastMessageKeyScrollIntentMs=performance.now();
|
||||
const now=performance.now();
|
||||
_lastMessageKeyScrollIntentMs=now;
|
||||
const bottomDistance=el.scrollHeight-el.scrollTop-el.clientHeight;
|
||||
if(bottomDistance>120) _lastMessageScrollIntentMs=now;
|
||||
}
|
||||
},{capture:true,passive:true});
|
||||
let _scrollRaf=0;
|
||||
|
|
@ -11685,13 +11702,18 @@ function _captureMessageScrollSnapshot(){
|
|||
const el=$('messages');
|
||||
if(!el) return null;
|
||||
const bottom=Math.max(0,el.scrollHeight-el.scrollTop-el.clientHeight);
|
||||
const readerAwayFromBottom=bottom>250&&(
|
||||
_messageUserUnpinned ||
|
||||
_scrollPinned===false ||
|
||||
(typeof _recentMessageScrollIntent==='function'&&_recentMessageScrollIntent())
|
||||
);
|
||||
return {
|
||||
anchor:(typeof _captureMessageViewportAnchor==='function')?_captureMessageViewportAnchor():null,
|
||||
top:el.scrollTop,
|
||||
bottom,
|
||||
scrollHeight:el.scrollHeight,
|
||||
pinned:_shouldFollowMessagesOnDomReplace(),
|
||||
userUnpinned:_messageUserUnpinned,
|
||||
pinned:readerAwayFromBottom?false:_shouldFollowMessagesOnDomReplace(),
|
||||
userUnpinned:readerAwayFromBottom?true:_messageUserUnpinned,
|
||||
};
|
||||
}
|
||||
function _restorePinnedMessageScrollSnapshot(snapshot){
|
||||
|
|
|
|||
|
|
@ -101,8 +101,8 @@ def test_same_frame_snapshot_preserves_bottom_distance_and_unpinned_state():
|
|||
wrapper = _function_body(UI_JS, "_renderMessagesWithScrollSnapshot")
|
||||
|
||||
assert "bottom" in capture
|
||||
assert "pinned:_shouldFollowMessagesOnDomReplace()" in _compact(capture)
|
||||
assert "userUnpinned:_messageUserUnpinned" in _compact(capture)
|
||||
assert "readerAwayFromBottom?false:_shouldFollowMessagesOnDomReplace()" in _compact(capture)
|
||||
assert "readerAwayFromBottom?true:_messageUserUnpinned" in _compact(capture)
|
||||
assert "maxTop-Math.max(0,bottom)" in restore
|
||||
assert "_messageUserUnpinned=true" in restore
|
||||
assert "_scrollPinned=false" in restore
|
||||
|
|
|
|||
|
|
@ -255,6 +255,185 @@ assert.strictEqual(_programmaticScroll, false);
|
|||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_capture_snapshot_treats_recent_scroll_away_as_manual_reader():
|
||||
"""Manual live-Worklog browsing must not reuse a stale pinned snapshot.
|
||||
|
||||
A reader can be far from the live tail while scrolling down through a long
|
||||
Compact Worklog. If a live activity rebuild captures that state while the
|
||||
sticky globals still say pinned, restoring as a pinned follower preserves a
|
||||
huge bottom gap and can keep yanking the viewport back toward the turn start.
|
||||
Recent message-pane scroll intent makes the snapshot explicitly unpinned.
|
||||
"""
|
||||
|
||||
script = f"""
|
||||
const assert = require('assert');
|
||||
let _messageUserUnpinned = false;
|
||||
let _scrollPinned = true;
|
||||
let _lastMessageScrollIntentMs = 1000;
|
||||
const MESSAGE_WHEEL_INTENT_SUPPRESS_MS = 1200;
|
||||
const container = {{
|
||||
scrollTop: 900,
|
||||
scrollHeight: 5000,
|
||||
clientHeight: 600,
|
||||
getBoundingClientRect() {{ return {{ top: 0, bottom: 600 }}; }},
|
||||
querySelectorAll() {{ return []; }},
|
||||
}};
|
||||
function $(id) {{ return id === 'messages' ? container : null; }}
|
||||
const performance = {{ now() {{ return 1500; }} }};
|
||||
function _captureMessageViewportAnchor() {{ return null; }}
|
||||
function _recentMessageTouchScrollIntent() {{ return false; }}
|
||||
function _recentMessageKeyScrollIntent() {{ return false; }}
|
||||
function _shouldFollowMessagesOnDomReplace() {{ return true; }}
|
||||
{_function_body(UI_JS, "_recentMessageScrollIntent")}
|
||||
{_function_body(UI_JS, "_captureMessageScrollSnapshot")}
|
||||
const snapshot = _captureMessageScrollSnapshot();
|
||||
assert.strictEqual(snapshot.bottom, 3500);
|
||||
assert.strictEqual(snapshot.pinned, false);
|
||||
assert.strictEqual(snapshot.userUnpinned, true);
|
||||
"""
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_capture_snapshot_keeps_true_pinned_follower_pinned_despite_large_gap():
|
||||
"""Large bottom distance alone is not enough to unpin a follower.
|
||||
|
||||
During fast streaming, content can grow under a followed viewport before the
|
||||
follow write lands. Without a recent user scroll intent or explicit unpin
|
||||
state, the snapshot must stay pinned so the existing tail-relative restore
|
||||
continues to protect pinned live followers. Raw touch/key recency is not
|
||||
enough here because those helpers also track near-tail artifact-suppression
|
||||
windows; only the guarded away-from-bottom intent stamp can classify a
|
||||
snapshot as manual-reader state.
|
||||
"""
|
||||
|
||||
script = f"""
|
||||
const assert = require('assert');
|
||||
let _messageUserUnpinned = false;
|
||||
let _scrollPinned = true;
|
||||
let _lastMessageScrollIntentMs = -Infinity;
|
||||
const MESSAGE_WHEEL_INTENT_SUPPRESS_MS = 1200;
|
||||
const container = {{
|
||||
scrollTop: 900,
|
||||
scrollHeight: 5000,
|
||||
clientHeight: 600,
|
||||
getBoundingClientRect() {{ return {{ top: 0, bottom: 600 }}; }},
|
||||
querySelectorAll() {{ return []; }},
|
||||
}};
|
||||
function $(id) {{ return id === 'messages' ? container : null; }}
|
||||
const performance = {{ now() {{ return 1500; }} }};
|
||||
function _captureMessageViewportAnchor() {{ return null; }}
|
||||
function _recentMessageTouchScrollIntent() {{ return true; }}
|
||||
function _recentMessageKeyScrollIntent() {{ return true; }}
|
||||
function _shouldFollowMessagesOnDomReplace() {{ return true; }}
|
||||
{_function_body(UI_JS, "_recentMessageScrollIntent")}
|
||||
{_function_body(UI_JS, "_captureMessageScrollSnapshot")}
|
||||
const snapshot = _captureMessageScrollSnapshot();
|
||||
assert.strictEqual(snapshot.bottom, 3500);
|
||||
assert.strictEqual(snapshot.pinned, true);
|
||||
assert.strictEqual(snapshot.userUnpinned, false);
|
||||
"""
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_wheel_scroll_intent_only_records_when_reader_is_away_from_bottom():
|
||||
"""Downward wheel intent should protect manual reading, not bottom following."""
|
||||
|
||||
script = f"""
|
||||
const assert = require('assert');
|
||||
let _lastNonMessageScrollIntentMs = -Infinity;
|
||||
let _lastMessageWheelIntentMs = -Infinity;
|
||||
let _lastMessageScrollIntentMs = -Infinity;
|
||||
let _messageUserUnpinned = false;
|
||||
let _nearBottomCount = 2;
|
||||
let _scrollPinned = true;
|
||||
let _messageTouchScrollActive = false;
|
||||
let _lastMessageTouchScrollIntentMs = -Infinity;
|
||||
let _touchStartY = null;
|
||||
const child = {{}};
|
||||
const el = {{
|
||||
scrollTop: 900,
|
||||
scrollHeight: 5000,
|
||||
clientHeight: 600,
|
||||
contains(target) {{ return target === child; }},
|
||||
}};
|
||||
function _cancelBottomSettle() {{}}
|
||||
function _markMessageTouchScrollIntent(active) {{
|
||||
_messageTouchScrollActive = !!active;
|
||||
_lastMessageTouchScrollIntentMs = performance.now();
|
||||
}}
|
||||
const document = {{ getElementById(id) {{ return id === 'messages' ? el : null; }} }};
|
||||
const performance = {{ now() {{ return 1234; }} }};
|
||||
{_function_body(UI_JS, "_recordNonMessageScrollIntent")}
|
||||
_recordNonMessageScrollIntent({{ target: child, type: 'wheel', deltaY: 24 }});
|
||||
assert.strictEqual(_lastMessageScrollIntentMs, 1234);
|
||||
assert.strictEqual(_messageUserUnpinned, false);
|
||||
|
||||
_lastMessageScrollIntentMs = -Infinity;
|
||||
el.scrollTop = 4400; // bottomDistance = 0
|
||||
_recordNonMessageScrollIntent({{ target: child, type: 'wheel', deltaY: 24 }});
|
||||
assert.strictEqual(_lastMessageScrollIntentMs, -Infinity);
|
||||
assert.strictEqual(_scrollPinned, true);
|
||||
"""
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_manual_scroll_snapshot_intent_excludes_raw_touch_and_key_recency():
|
||||
"""Only bottom-guarded message scroll intent can drive snapshot unpinning."""
|
||||
|
||||
compact = _compact(_function_body(UI_JS, "_recentMessageScrollIntent"))
|
||||
|
||||
assert "_lastMessageScrollIntentMs" in compact
|
||||
assert "_scrollbarDragActive" in compact
|
||||
assert "_recentMessageTouchScrollIntent" not in compact
|
||||
assert "_recentMessageKeyScrollIntent" not in compact
|
||||
assert "_lastMessageKeyScrollIntentMs=now;" in _compact(UI_JS)
|
||||
assert "if(bottomDistance>120)_lastMessageScrollIntentMs=now;" in _compact(UI_JS)
|
||||
|
||||
|
||||
def test_touch_scroll_intent_only_records_when_reader_is_away_from_bottom():
|
||||
"""Touch recency is broad, but snapshot intent is bottom-distance guarded."""
|
||||
|
||||
script = f"""
|
||||
const assert = require('assert');
|
||||
let _lastNonMessageScrollIntentMs = -Infinity;
|
||||
let _lastMessageWheelIntentMs = -Infinity;
|
||||
let _lastMessageScrollIntentMs = -Infinity;
|
||||
let _messageUserUnpinned = false;
|
||||
let _nearBottomCount = 2;
|
||||
let _scrollPinned = true;
|
||||
let _messageTouchScrollActive = false;
|
||||
let _lastMessageTouchScrollIntentMs = -Infinity;
|
||||
let _touchStartY = null;
|
||||
const child = {{}};
|
||||
const el = {{
|
||||
scrollTop: 900,
|
||||
scrollHeight: 5000,
|
||||
clientHeight: 600,
|
||||
contains(target) {{ return target === child; }},
|
||||
}};
|
||||
function _cancelBottomSettle() {{}}
|
||||
function _markMessageTouchScrollIntent(active) {{
|
||||
_messageTouchScrollActive = !!active;
|
||||
_lastMessageTouchScrollIntentMs = performance.now();
|
||||
}}
|
||||
const document = {{ getElementById(id) {{ return id === 'messages' ? el : null; }} }};
|
||||
const performance = {{ now() {{ return 1234; }} }};
|
||||
{_function_body(UI_JS, "_recordNonMessageScrollIntent")}
|
||||
_recordNonMessageScrollIntent({{ target: child, type: 'touchmove' }});
|
||||
assert.strictEqual(_lastMessageScrollIntentMs, 1234);
|
||||
assert.strictEqual(_lastMessageTouchScrollIntentMs, 1234);
|
||||
assert.strictEqual(_messageUserUnpinned, false);
|
||||
|
||||
_lastMessageScrollIntentMs = -Infinity;
|
||||
el.scrollTop = 4400; // bottomDistance = 0
|
||||
_recordNonMessageScrollIntent({{ target: child, type: 'touchmove' }});
|
||||
assert.strictEqual(_lastMessageScrollIntentMs, -Infinity);
|
||||
assert.strictEqual(_lastMessageTouchScrollIntentMs, 1234);
|
||||
assert.strictEqual(_scrollPinned, true);
|
||||
"""
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_restore_message_scroll_snapshot_prefers_semantic_anchor_with_virtual_fallback():
|
||||
body = _function_body(UI_JS, "_restoreMessageScrollSnapshot")
|
||||
helper = _function_body(UI_JS, "_remountMessageViewportAnchor")
|
||||
|
|
|
|||
|
|
@ -420,10 +420,14 @@ def test_keyboard_scroll_intent_tracked_and_gated():
|
|||
assert "let _lastMessageKeyScrollIntentMs=-Infinity" in UI_JS
|
||||
assert "function _recentMessageKeyScrollIntent" in UI_JS
|
||||
# A keydown listener must stamp the intent for the pane scroll keys.
|
||||
assert "_lastMessageKeyScrollIntentMs=performance.now()" in UI_JS, (
|
||||
assert "_lastMessageKeyScrollIntentMs=now;" in UI_JS, (
|
||||
"a keydown handler must stamp _lastMessageKeyScrollIntentMs when the "
|
||||
"reader uses the keyboard to scroll the message pane."
|
||||
)
|
||||
assert "if(bottomDistance>120) _lastMessageScrollIntentMs=now;" in UI_JS, (
|
||||
"keyboard-driven manual-reader snapshot intent must be guarded by "
|
||||
"distance from the live tail."
|
||||
)
|
||||
assert "'PageUp'" in UI_JS and "'PageDown'" in UI_JS
|
||||
# The suppression branch must consult it.
|
||||
listener_idx = UI_JS.find("el.addEventListener('scroll'")
|
||||
|
|
|
|||
Loading…
Reference in a new issue