fix: address Greptile review — null guards, cancelAnimationFrame, overflow-anchor
- Guard _settleRO against null in ResizeObserver callbacks (P1 crash) - _cancelBottomSettle: also cancelAnimationFrame(_settleRAF) (P2 stale timers) - Remove overflow-anchor:none from .messages — let Firefox use native scroll anchoring to handle DOM-growth scroll adjustments Addresses review comments from @greptile-apps on #3920.
This commit is contained in:
Ming
parent
55e713b945
commit
5ded20c443
2 changed files with 33 additions and 41 deletions
|
|
@ -992,7 +992,7 @@
|
|||
.sidebar-section{padding:14px 14px 8px;}
|
||||
.new-chat-btn{width:100%;padding:9px 12px;border-radius:9px;background:var(--accent-bg);border:1px solid var(--accent-bg-strong);color:var(--accent-text);font-size:13px;cursor:pointer;display:flex;align-items:center;gap:8px;transition:all .15s;margin-bottom:8px;font-weight:500;}
|
||||
.new-chat-btn:hover{background:var(--accent-bg-strong);border-color:var(--accent);}
|
||||
.session-list{flex:1;overflow-y:auto;padding:0 8px 8px;min-height:0;overscroll-behavior-y:contain;touch-action:pan-y;overflow-anchor:none;}
|
||||
.session-list{flex:1;overflow-y:auto;padding:0 8px 8px;min-height:0;overscroll-behavior-y:contain;touch-action:pan-y;}
|
||||
.sidebar-search{position:relative;padding:8px 12px;flex-shrink:0;}
|
||||
.session-search-field{position:relative;display:flex;align-items:center;width:100%;}
|
||||
.sidebar-search input{width:100%;background:var(--bg);border:1px solid var(--border);border-radius:8px;color:var(--text);padding:7px 10px 7px 32px;font-size:13px;outline:none;transition:border-color .15s,box-shadow .15s,background .15s;box-sizing:border-box;}
|
||||
|
|
@ -1588,7 +1588,7 @@
|
|||
.workspace-toggle-btn:disabled{opacity:.38;cursor:not-allowed;}
|
||||
.chip.model{color:var(--accent-text);border-color:var(--accent-bg-strong);background:var(--accent-bg);}
|
||||
.messages-shell{flex:1;min-height:0;position:relative;display:flex;flex-direction:column;}
|
||||
.messages{flex:1;overflow-y:auto;display:flex;flex-direction:column;min-height:0;position:relative;z-index:0;-webkit-overflow-scrolling:touch;touch-action:pan-y;overscroll-behavior-y:contain;overflow-anchor:none;}
|
||||
.messages{flex:1;overflow-y:auto;display:flex;flex-direction:column;min-height:0;position:relative;z-index:0;-webkit-overflow-scrolling:touch;touch-action:pan-y;overscroll-behavior-y:contain;}
|
||||
/* Overlay scroll controls so they do not affect the transcript's native scroll geometry. */
|
||||
.scroll-to-bottom-btn{position:absolute;right:20px;bottom:16px;width:32px;height:32px;border-radius:50%;border:1px solid var(--border2);background:var(--code-bg);color:var(--muted);font-size:16px;cursor:pointer;display:flex;align-items:center;justify-content:center;box-shadow:0 2px 8px rgba(0,0,0,.25);z-index:10;transition:color .12s,border-color .12s,background .12s,transform .12s;}
|
||||
.scroll-to-bottom-btn:hover{color:var(--text);border-color:var(--border);background:var(--hover-bg);}
|
||||
|
|
|
|||
70
static/ui.js
70
static/ui.js
|
|
@ -2358,7 +2358,7 @@ let _settleTimer=0;
|
|||
const NON_MESSAGE_SCROLL_INTENT_SUPPRESS_MS=350;
|
||||
let _touchStartY=null;
|
||||
let _newMessageCueVisible=false;
|
||||
function _cancelBottomSettle(){ _bottomSettleToken++; if(_settleRO){ _settleRO.disconnect(); _settleRO=null; } clearTimeout(_settleTimer); }
|
||||
function _cancelBottomSettle(){ _bottomSettleToken++; if(_settleRO){ _settleRO.disconnect(); _settleRO=null; } clearTimeout(_settleTimer); cancelAnimationFrame(_settleRAF); }
|
||||
function _recordNonMessageScrollIntent(e){
|
||||
const el=document.getElementById('messages');
|
||||
const target=e&&e.target;
|
||||
|
|
@ -3038,23 +3038,20 @@ function _settleMessageScrollToBottom(force){
|
|||
// position when content settles so late layout does not leave the viewport
|
||||
// above the real end. User scroll increments _bottomSettleToken and cancels.
|
||||
//
|
||||
// Firefox settles layout more slowly than Safari. The old rAF-polling
|
||||
// approach read scrollHeight across frames and wrote scrollTop once stable,
|
||||
// but Firefox still paints each rAF check as a visible reflow jitter.
|
||||
// Firefox paints each scrollTop write as a visible reflow step. The old
|
||||
// rAF-polling approach read scrollHeight across frames — the read itself
|
||||
// forced a reflow in Firefox, causing visible jitter.
|
||||
//
|
||||
// ResizeObserver approach: write scrollTop ONCE now, then observe the
|
||||
// messages container for size changes. When no resize fires for 50ms,
|
||||
// do a single final scrollTop write. No intermediate scrollTop writes,
|
||||
// no rAF polling — Firefox never paints intermediate positions.
|
||||
// ResizeObserver approach: the browser notifies us when the container
|
||||
// resizes (no scrollHeight polling needed). On each notification we write
|
||||
// scrollTop once via rAF (batches multiple resize callbacks per frame into
|
||||
// a single write). After 300ms of no resize events, the observer disconnects.
|
||||
const token=++_bottomSettleToken;
|
||||
cancelAnimationFrame(_settleRAF);
|
||||
if(_settleRO){ _settleRO.disconnect(); _settleRO=null; }
|
||||
clearTimeout(_settleTimer);
|
||||
|
||||
// Write the first bottom position synchronously. A final renderMessages()
|
||||
// rebuild can queue a native scroll event from the temporary scrollTop=0
|
||||
// layout state; writing early prevents that event from cancelling the
|
||||
// settle (#3319).
|
||||
// Sync write anchors the viewport immediately.
|
||||
_setMessageScrollToBottom();
|
||||
|
||||
if(force) return;
|
||||
|
|
@ -3062,35 +3059,29 @@ function _settleMessageScrollToBottom(force){
|
|||
const el=document.getElementById('messages');
|
||||
if(!el) return;
|
||||
|
||||
// Observe the messages container for layout changes (KaTeX, Mermaid, images,
|
||||
// Prism — anything that grows the DOM after initial render).
|
||||
let settleTimer;
|
||||
_settleRO=new ResizeObserver(()=>{
|
||||
if(token!==_bottomSettleToken){ _settleRO.disconnect(); _settleRO=null; return; }
|
||||
// Guard: user scrolled away during settling
|
||||
if(token!==_bottomSettleToken){ if(_settleRO){ _settleRO.disconnect(); _settleRO=null; } return; }
|
||||
if(!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){
|
||||
_settleRO.disconnect(); _settleRO=null;
|
||||
if(_settleRO){ _settleRO.disconnect(); _settleRO=null; }
|
||||
_programmaticScroll=false;
|
||||
return;
|
||||
}
|
||||
clearTimeout(settleTimer);
|
||||
settleTimer=setTimeout(()=>{
|
||||
if(token!==_bottomSettleToken){ _settleRO.disconnect(); _settleRO=null; return; }
|
||||
_settleFinalScroll(token);
|
||||
_settleRO.disconnect();
|
||||
_settleRO=null;
|
||||
},50);
|
||||
// Write scrollTop once per frame — ResizeObserver batches multiple
|
||||
// notifications per frame, so this is at most one write per frame.
|
||||
cancelAnimationFrame(_settleRAF);
|
||||
_settleRAF=requestAnimationFrame(()=>{
|
||||
if(token!==_bottomSettleToken) return;
|
||||
_setMessageScrollToBottom();
|
||||
});
|
||||
// After 300ms of quiet, disconnect — layout is stable.
|
||||
clearTimeout(_settleTimer);
|
||||
_settleTimer=setTimeout(()=>{
|
||||
if(token!==_bottomSettleToken) return;
|
||||
if(_settleRO){ _settleRO.disconnect(); _settleRO=null; }
|
||||
_setMessageScrollToBottom();
|
||||
},300);
|
||||
});
|
||||
_settleRO.observe(el);
|
||||
_settleTimer=settleTimer;
|
||||
|
||||
// Safety cap: if nothing fires for 2s (static content), still do a final
|
||||
// scroll to be safe.
|
||||
_settleRAF=setTimeout(()=>{
|
||||
if(token!==_bottomSettleToken) return;
|
||||
if(_settleRO){ _settleRO.disconnect(); _settleRO=null; }
|
||||
_settleFinalScroll(token);
|
||||
},2000);
|
||||
}
|
||||
|
||||
function _settleFinalScroll(token){
|
||||
|
|
@ -3117,12 +3108,13 @@ function scrollToBottom(){
|
|||
_clearNewMessageScrollCue();
|
||||
_scrollPinned=true;
|
||||
_messageUserUnpinned=false;
|
||||
// Write the first bottom position synchronously. A final renderMessages()
|
||||
// rebuild can queue a native scroll event from the temporary scrollTop=0
|
||||
// layout state; if we only schedule delayed settles, that event can cancel
|
||||
// them before the viewport ever reaches the bottom.
|
||||
// Write scrollTop once synchronously to anchor the viewport, then let
|
||||
// ResizeObserver settle handle any late layout growth (Prism, KaTeX,
|
||||
// Mermaid, images). Using force=false so the observer runs — force=true
|
||||
// was skipping the observer and causing Firefox paint jumps when
|
||||
// renderMessages({preserveScroll:true}) + scrollToBottom() fired back-to-back.
|
||||
_setMessageScrollToBottom();
|
||||
_settleMessageScrollToBottom(true);
|
||||
_settleMessageScrollToBottom(false);
|
||||
_syncScrollToBottomCue(false,{newMessage:false});
|
||||
if(typeof _updateSessionStartJumpButton==='function') _updateSessionStartJumpButton();
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue