fix(chat): Prevent image loads and content reflows from accidentally disabling autoscroll#2966
fix(chat): Prevent image loads and content reflows from accidentally disabling autoscroll#2966PEyacher wants to merge 1 commit into
Conversation
…disabling chat autoscroll. Detect intentional upward scrolling via wheel and touch events
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/chat/ScrollContainer.tsx">
<violation number="1" location="frontend/src/components/chat/ScrollContainer.tsx:49">
P2: Using `flushSync` in `syncScrollButtonVisibility` forces unnecessary synchronous React commits from high-frequency callbacks (`ResizeObserver`, wheel, touch handlers). The state only toggles a CSS class on a single element, and other call sites in this same file already update it directly without `flushSync`.</violation>
<violation number="2" location="frontend/src/components/chat/ScrollContainer.tsx:242">
P1: `handleScroll` no longer disables autoscroll when the user scrolls away from the bottom, relying solely on `wheel` and `touchmove` to detect upward intent. This misses scrollbar drag and keyboard scrolling (PageUp, Home, Space, arrow keys), which do not emit `wheel` events. When a user scrolls up via those methods, `autoScrollRef.current` stays `true`, and subsequent message arrivals or content resizes in `updateSpacerHeight` force-scroll back to the bottom, overriding the user's intent to read earlier messages.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| checkScrollEnd(); | ||
| }; | ||
|
|
||
| // onScroll event handler - ONLY re-enables autoscroll when at bottom |
There was a problem hiding this comment.
P1: handleScroll no longer disables autoscroll when the user scrolls away from the bottom, relying solely on wheel and touchmove to detect upward intent. This misses scrollbar drag and keyboard scrolling (PageUp, Home, Space, arrow keys), which do not emit wheel events. When a user scrolls up via those methods, autoScrollRef.current stays true, and subsequent message arrivals or content resizes in updateSpacerHeight force-scroll back to the bottom, overriding the user's intent to read earlier messages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/chat/ScrollContainer.tsx, line 242:
<comment>`handleScroll` no longer disables autoscroll when the user scrolls away from the bottom, relying solely on `wheel` and `touchmove` to detect upward intent. This misses scrollbar drag and keyboard scrolling (PageUp, Home, Space, arrow keys), which do not emit `wheel` events. When a user scrolls up via those methods, `autoScrollRef.current` stays `true`, and subsequent message arrivals or content resizes in `updateSpacerHeight` force-scroll back to the bottom, overriding the user's intent to read earlier messages.</comment>
<file context>
@@ -182,42 +239,105 @@ export default function ScrollContainer({
checkScrollEnd();
};
+ // onScroll event handler - ONLY re-enables autoscroll when at bottom
+ // Disabling autoscroll is handled by wheel/touch events to avoid false triggers from content reflow
const handleScroll = () => {
</file context>
|
|
||
| const syncScrollButtonVisibility = useCallback(() => { | ||
| const autoScrollDisabled = autoScrollRef ? !autoScrollRef.current : false; | ||
| flushSync(() => { |
There was a problem hiding this comment.
P2: Using flushSync in syncScrollButtonVisibility forces unnecessary synchronous React commits from high-frequency callbacks (ResizeObserver, wheel, touch handlers). The state only toggles a CSS class on a single element, and other call sites in this same file already update it directly without flushSync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/chat/ScrollContainer.tsx, line 49:
<comment>Using `flushSync` in `syncScrollButtonVisibility` forces unnecessary synchronous React commits from high-frequency callbacks (`ResizeObserver`, wheel, touch handlers). The state only toggles a CSS class on a single element, and other call sites in this same file already update it directly without `flushSync`.</comment>
<file context>
@@ -28,12 +29,28 @@ export default function ScrollContainer({
+
+ const syncScrollButtonVisibility = useCallback(() => {
+ const autoScrollDisabled = autoScrollRef ? !autoScrollRef.current : false;
+ flushSync(() => {
+ setShowScrollButton(autoScrollDisabled);
+ });
</file context>
Fix for #2965.
Summary:
Summary by cubic
Prevents chat autoscroll from being turned off by image loads or layout shifts. Autoscroll now only disables when the user intentionally scrolls up.
ResizeObserverto keep spacer and button visibility in sync.Written for commit b5341ef. Summary will update on new commits.