-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: prevent chat auto-scroll when user is reading previous messages #8614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -189,7 +189,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro | |||||||||||||||||||||||||||||||
| const scrollContainerRef = useRef<HTMLDivElement>(null) | ||||||||||||||||||||||||||||||||
| const disableAutoScrollRef = useRef(false) | ||||||||||||||||||||||||||||||||
| const [showScrollToBottom, setShowScrollToBottom] = useState(false) | ||||||||||||||||||||||||||||||||
| const [isAtBottom, setIsAtBottom] = useState(false) | ||||||||||||||||||||||||||||||||
| const [isAtBottom, setIsAtBottom] = useState(true) // Start at bottom by default | ||||||||||||||||||||||||||||||||
| const wasAtBottomBeforeUpdateRef = useRef(true) // Track if user was at bottom before messages update | ||||||||||||||||||||||||||||||||
| const lastTtsRef = useRef<string>("") | ||||||||||||||||||||||||||||||||
| const [wasStreaming, setWasStreaming] = useState<boolean>(false) | ||||||||||||||||||||||||||||||||
| const [showCheckpointWarning, setShowCheckpointWarning] = useState<boolean>(false) | ||||||||||||||||||||||||||||||||
|
|
@@ -1395,9 +1396,18 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro | |||||||||||||||||||||||||||||||
| [scrollToBottomSmooth, scrollToBottomAuto], | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Track if user was at bottom before messages update | ||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||
| wasAtBottomBeforeUpdateRef.current = isAtBottom | ||||||||||||||||||||||||||||||||
| }, [isAtBottom, groupedMessages.length]) // Capture state before the update | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||
| let timer: ReturnType<typeof setTimeout> | undefined | ||||||||||||||||||||||||||||||||
| if (!disableAutoScrollRef.current) { | ||||||||||||||||||||||||||||||||
| // Only auto-scroll if: | ||||||||||||||||||||||||||||||||
| // 1. Auto-scroll is not disabled by user interaction | ||||||||||||||||||||||||||||||||
| // 2. User was at the bottom before the new messages arrived | ||||||||||||||||||||||||||||||||
| // 3. We're in a streaming state (to handle continuous updates) | ||||||||||||||||||||||||||||||||
| if (!disableAutoScrollRef.current && wasAtBottomBeforeUpdateRef.current) { | ||||||||||||||||||||||||||||||||
| timer = setTimeout(() => scrollToBottomSmooth(), 50) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| return () => { | ||||||||||||||||||||||||||||||||
|
|
@@ -1407,19 +1417,44 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| }, [groupedMessages.length, scrollToBottomSmooth]) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const handleWheel = useCallback((event: Event) => { | ||||||||||||||||||||||||||||||||
| const wheelEvent = event as WheelEvent | ||||||||||||||||||||||||||||||||
| const handleWheel = useCallback( | ||||||||||||||||||||||||||||||||
| (event: Event) => { | ||||||||||||||||||||||||||||||||
| const wheelEvent = event as WheelEvent | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if (wheelEvent.deltaY && wheelEvent.deltaY < 0) { | ||||||||||||||||||||||||||||||||
| // Check if the wheel event is within our scroll container | ||||||||||||||||||||||||||||||||
| if (scrollContainerRef.current?.contains(wheelEvent.target as Node)) { | ||||||||||||||||||||||||||||||||
| // User scrolled up | ||||||||||||||||||||||||||||||||
| disableAutoScrollRef.current = true | ||||||||||||||||||||||||||||||||
| // Disable auto-scroll on any deliberate scroll action (up or down) | ||||||||||||||||||||||||||||||||
| // Only re-enable when user scrolls back to bottom | ||||||||||||||||||||||||||||||||
| if (wheelEvent.deltaY !== 0) { | ||||||||||||||||||||||||||||||||
| // Any scroll movement | ||||||||||||||||||||||||||||||||
| if (!isAtBottom) { | ||||||||||||||||||||||||||||||||
| // User is scrolling while not at bottom - disable auto-scroll | ||||||||||||||||||||||||||||||||
| disableAutoScrollRef.current = true | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| // If user is at bottom and scrolling down, keep auto-scroll enabled | ||||||||||||||||||||||||||||||||
| // If user is at bottom and scrolling up, it will be disabled | ||||||||||||||||||||||||||||||||
| if (wheelEvent.deltaY < 0) { | ||||||||||||||||||||||||||||||||
| // Scrolling up always disables auto-scroll | ||||||||||||||||||||||||||||||||
| disableAutoScrollRef.current = true | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| }, []) | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| [isAtBottom], | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| useEvent("wheel", handleWheel, window, { passive: true }) // passive improves scrolling performance | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Also handle touch scrolling for mobile devices | ||||||||||||||||||||||||||||||||
| const handleTouchMove = useCallback(() => { | ||||||||||||||||||||||||||||||||
| // During touch scrolling, check if we're at bottom | ||||||||||||||||||||||||||||||||
| if (!isAtBottom && scrollContainerRef.current) { | ||||||||||||||||||||||||||||||||
| disableAutoScrollRef.current = true | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| }, [isAtBottom]) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| useEvent("touchmove", handleTouchMove, scrollContainerRef.current, { passive: true }) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Effect to handle showing the checkpoint warning after a delay | ||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||
| // Only show the warning when there's a task but no visible messages yet | ||||||||||||||||||||||||||||||||
|
|
@@ -1878,12 +1913,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro | |||||||||||||||||||||||||||||||
| increaseViewportBy={{ top: 3_000, bottom: 1000 }} | ||||||||||||||||||||||||||||||||
| data={groupedMessages} | ||||||||||||||||||||||||||||||||
| itemContent={itemContent} | ||||||||||||||||||||||||||||||||
| atBottomStateChange={(isAtBottom: boolean) => { | ||||||||||||||||||||||||||||||||
| setIsAtBottom(isAtBottom) | ||||||||||||||||||||||||||||||||
| if (isAtBottom) { | ||||||||||||||||||||||||||||||||
| atBottomStateChange={(atBottom: boolean) => { | ||||||||||||||||||||||||||||||||
| setIsAtBottom(atBottom) | ||||||||||||||||||||||||||||||||
| if (atBottom) { | ||||||||||||||||||||||||||||||||
| // User scrolled back to bottom, re-enable auto-scroll | ||||||||||||||||||||||||||||||||
| disableAutoScrollRef.current = false | ||||||||||||||||||||||||||||||||
|
Comment on lines
+1916
to
1920
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic bug: When
Result: User's scroll intent is ignored if they stay within the 10px threshold. To fix, only re-enable auto-scroll when transitioning from
Suggested change
|
||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| setShowScrollToBottom(disableAutoScrollRef.current && !isAtBottom) | ||||||||||||||||||||||||||||||||
| // Show scroll-to-bottom button when auto-scroll is disabled and not at bottom | ||||||||||||||||||||||||||||||||
| setShowScrollToBottom(disableAutoScrollRef.current && !atBottom) | ||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||
| atBottomThreshold={10} | ||||||||||||||||||||||||||||||||
| initialTopMostItemIndex={groupedMessages.length - 1} | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: This effect runs AFTER
groupedMessages.lengthchanges, but the ref is supposed to capture the state BEFORE the update. When new messages arrive, both this effect and the auto-scroll effect (lines 1404-1418) will run in the same render cycle, makingwasAtBottomBeforeUpdateRef.currentreflect the state AFTER the update, not before.To fix this, remove
groupedMessages.lengthfrom the dependency array and capture the state in auseLayoutEffectbefore the DOM updates, or capture it just before the state change that triggers the message update.