* fix(stream): preserve fade nodes while prose is live * fix(stream): preserve transparent fade nodes like assistant prose The transparent-activity prose path (transparent_stream mode) kept the pre-PR fade cleanup: _bindTransparentFadeCleanup() in static/ui.js still replaced every animated stream-fade-word span with a fresh text node on animationend, invalidating the native scroll anchor while the row is still growing. Mirror the messages.js fix so both live prose paths share one node-preserving cleanup: remove only the transient is-new class and the --stream-fade-ms inline property, keeping span identity until final settlement rebuilds the persisted DOM. Extend tests/test_smooth_text_fade.py with a sibling regression that extracts the production _bindTransparentFadeCleanup() from ui.js and fires animationend, asserting no replacement, is-new removal, stream-fade-word retention, and timing-override cleanup (plus source-string guards against replaceWith in the cleanup block). The new test is red-before-green against the previous ui.js implementation. * docs(changelog): stamp #6257 (stream fade scroll-bounce fix) --------- Co-authored-by: Anthony Ruiz <ruizanthony@users.noreply.github.com> Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
4 files changed
+115
-3
No files matched your search
@@ -5,6 +5,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **The chat view no longer bounces vertically while a reply streams in.** As each word faded in during streaming, the browser replaced the finished word's DOM node with a fresh text node — and every replacement made the browser's native scroll anchoring re-pick an anchor while the transcript was still growing, producing a visible vertical bounce. The two fade-cleanup handlers now keep each faded-in word's node stable (they just clear the transient animation class and timing variable) instead of replacing it, so the scroll position stays put. The settled reply is still rebuilt as plain text exactly as before, and text selection during streaming is steadier as a bonus. Thanks @ruizanthony. (#6257)
|
||||
|
||||
- **Links ending in Chinese/Japanese full-width punctuation no longer swallow the punctuation into the URL.** When an assistant wrote a URL followed immediately by a CJK full-width mark (`)。,;:!?、`) — common in Chinese/Japanese prose — the autolink pass treated the mark as part of the link, so the rendered href included the trailing punctuation (and often 404'd). Both autolink passes now strip a single trailing CJK full-width mark the same way they already strip ASCII `.,;:!?)`, and a full-width close-paren also terminates the URL. ASCII behavior is unchanged. Thanks @happy5318. (#6792)
|
||||
|
||||
- **Mobile: create forms opened from the sidebar drawer are no longer hidden behind it.** On phones the sidebar is a full-screen drawer over the main view; opening a new cron/skill/workspace/profile form from inside it left the drawer covering the form, so the tap looked like a no-op. The panel re-open on a rail tap now happens synchronously (before the panel's async data load, so a form that closes the drawer isn't re-covered), and the four create-form openers now close the drawer the same way their matching detail views already did. Desktop is unaffected. Thanks @silent-reader-cn. (#6921)
|
||||
|
||||
+6
-1
@@ -4586,7 +4586,12 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
el.addEventListener('animationend',e=>{
|
||||
const span=e.target;
|
||||
if(!span||!span.classList||!span.classList.contains('stream-fade-word')) return;
|
||||
span.replaceWith(document.createTextNode(span.textContent||''));
|
||||
// Keep the animated inline node stable for the lifetime of the live turn.
|
||||
// Replacing each word with a fresh text node makes native scroll anchoring
|
||||
// choose a new anchor while the transcript is still growing, producing a
|
||||
// visible vertical bounce. Final settlement rebuilds plain persisted DOM.
|
||||
span.classList.remove('is-new');
|
||||
if(span.style) span.style.removeProperty('--stream-fade-ms');
|
||||
});
|
||||
}
|
||||
function _streamFadeRenderer(el){
|
||||
|
||||
+7
-1
@@ -13474,7 +13474,13 @@ function _bindTransparentFadeCleanup(body){
|
||||
body.addEventListener('animationend', e=>{
|
||||
const span = e.target;
|
||||
if(!span || !span.classList || !span.classList.contains('stream-fade-word')) return;
|
||||
span.replaceWith(document.createTextNode(span.textContent || ''));
|
||||
// Keep the animated inline node stable for the lifetime of the live turn,
|
||||
// exactly like _streamFadeBindCleanup() in messages.js. Replacing each word
|
||||
// with a fresh text node makes native scroll anchoring choose a new anchor
|
||||
// while the transparent prose row is still growing, producing a visible
|
||||
// vertical bounce. Final settlement rebuilds plain persisted DOM.
|
||||
span.classList.remove('is-new');
|
||||
if(span.style) span.style.removeProperty('--stream-fade-ms');
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ CONFIG_PY = (REPO / "api" / "config.py").read_text(encoding="utf-8")
|
||||
INDEX_HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8")
|
||||
PANELS_JS = (REPO / "static" / "panels.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
BOOT_JS = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
|
||||
STYLE_CSS = (REPO / "static" / "style.css").read_text(encoding="utf-8")
|
||||
I18N_JS = (REPO / "static" / "i18n.js").read_text(encoding="utf-8")
|
||||
@@ -224,8 +225,9 @@ def test_stream_fade_uses_incremental_renderer_without_changing_default_path():
|
||||
)
|
||||
assert_contains_all(
|
||||
cleanup_block,
|
||||
["animationend", "span.replaceWith(document.createTextNode"],
|
||||
["animationend", "span.classList.remove('is-new')"],
|
||||
)
|
||||
assert "span.replaceWith" not in cleanup_block
|
||||
assert "_wrapStreamingFadeWords" not in MESSAGES_JS
|
||||
assert "animationDelay" not in renderer_block
|
||||
assert "_STREAM_FADE_STAGGER_MS" not in MESSAGES_JS
|
||||
@@ -350,6 +352,103 @@ if(!_shouldUseLiveProseFade()) throw new Error('regular fade preference should w
|
||||
run_node(script)
|
||||
|
||||
|
||||
def test_fade_cleanup_preserves_the_animated_node_as_the_scroll_anchor():
|
||||
"""Animation cleanup must not replace inline nodes during a live stream.
|
||||
|
||||
Replacing every word node on animationend makes the browser choose a new
|
||||
native scroll anchor while the transcript is still growing. The opacity
|
||||
animation can finish by removing its transient class; final settlement will
|
||||
rebuild the persisted answer as plain text.
|
||||
"""
|
||||
cleanup = function_block(MESSAGES_JS, "_streamFadeBindCleanup")
|
||||
script = r"""
|
||||
class FakeClassList {
|
||||
constructor(names){ this.names=new Set(names); }
|
||||
contains(name){ return this.names.has(name); }
|
||||
remove(name){ this.names.delete(name); }
|
||||
}
|
||||
const listeners={};
|
||||
const document={createTextNode(text){ return {textContent:String(text)}; }};
|
||||
const body={
|
||||
_streamFadeCleanupBound:false,
|
||||
addEventListener(name,fn){ listeners[name]=fn; },
|
||||
};
|
||||
""" + cleanup + r"""
|
||||
_streamFadeBindCleanup(body);
|
||||
const span={
|
||||
textContent:'stable anchor',
|
||||
classList:new FakeClassList(['stream-fade-word','is-new']),
|
||||
style:{
|
||||
removed:[],
|
||||
removeProperty(name){ this.removed.push(name); },
|
||||
},
|
||||
replacements:0,
|
||||
replaceWith(){ this.replacements += 1; },
|
||||
};
|
||||
listeners.animationend({target:span});
|
||||
if(span.replacements!==0) throw new Error('fade cleanup replaced the active scroll-anchor node');
|
||||
if(span.classList.contains('is-new')) throw new Error('fade animation class was not cleared');
|
||||
if(!span.classList.contains('stream-fade-word')) throw new Error('stable fade node lost its identity');
|
||||
if(span.style.removed.join('|')!=='--stream-fade-ms') throw new Error('fade timing override was not cleared');
|
||||
"""
|
||||
run_node(script)
|
||||
|
||||
|
||||
def test_transparent_fade_cleanup_preserves_the_animated_node_as_the_scroll_anchor():
|
||||
"""The transparent-activity fade path must share the messages.js cleanup.
|
||||
|
||||
`_bindTransparentFadeCleanup()` in ui.js is the visible transparent_stream
|
||||
sibling of `_streamFadeBindCleanup()` in messages.js. It used to replace
|
||||
every animated word span with a fresh text node on animationend, which made
|
||||
native scroll anchoring pick a new anchor while the transparent prose row
|
||||
was still growing. Both live prose paths must keep the node stable: only
|
||||
the transient `is-new` class and the `--stream-fade-ms` timing override are
|
||||
removed; final settlement rebuilds the persisted DOM as plain text.
|
||||
"""
|
||||
cleanup = function_block(UI_JS, "_bindTransparentFadeCleanup")
|
||||
assert_contains_all(
|
||||
cleanup,
|
||||
[
|
||||
"animationend",
|
||||
"span.classList.remove('is-new')",
|
||||
"span.style.removeProperty('--stream-fade-ms')",
|
||||
],
|
||||
)
|
||||
assert "span.replaceWith" not in cleanup
|
||||
script = r"""
|
||||
class FakeClassList {
|
||||
constructor(names){ this.names=new Set(names); }
|
||||
contains(name){ return this.names.has(name); }
|
||||
remove(name){ this.names.delete(name); }
|
||||
}
|
||||
const listeners={};
|
||||
const document={createTextNode(text){ return {textContent:String(text)}; }};
|
||||
const body={
|
||||
_transparentFadeCleanupBound:false,
|
||||
addEventListener(name,fn){ listeners[name]=fn; },
|
||||
};
|
||||
""" + cleanup + r"""
|
||||
_bindTransparentFadeCleanup(body);
|
||||
if(body._transparentFadeCleanupBound!==true) throw new Error('transparent fade cleanup was not bound');
|
||||
const span={
|
||||
textContent:'stable anchor',
|
||||
classList:new FakeClassList(['stream-fade-word','is-new']),
|
||||
style:{
|
||||
removed:[],
|
||||
removeProperty(name){ this.removed.push(name); },
|
||||
},
|
||||
replacements:0,
|
||||
replaceWith(){ this.replacements += 1; },
|
||||
};
|
||||
listeners.animationend({target:span});
|
||||
if(span.replacements!==0) throw new Error('transparent fade cleanup replaced the active scroll-anchor node');
|
||||
if(span.classList.contains('is-new')) throw new Error('transparent fade animation class was not cleared');
|
||||
if(!span.classList.contains('stream-fade-word')) throw new Error('stable transparent fade node lost its identity');
|
||||
if(span.style.removed.join('|')!=='--stream-fade-ms') throw new Error('transparent fade timing override was not cleared');
|
||||
"""
|
||||
run_node(script)
|
||||
|
||||
|
||||
def test_transparent_stream_hidden_body_appends_plain_text_only():
|
||||
script = (
|
||||
function_block(MESSAGES_JS, "_renderStreamingFadeMarkdown")
|
||||
|
||||
Reference in new issue
Block a user