-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Implement smart auto-scroll for chat messages #15
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 15 commits
49df993
4ec8df4
ca69537
4925dd7
697319c
eea5a29
22c8b4f
6fdf742
d0c762b
4be9b23
cf19fa8
297e338
94d646a
bee10d6
ab40ee7
ce4d094
6de859c
b8cf5de
fedaaca
15d5b14
3108302
5ab0521
54b0049
16ad26c
da147c8
7b67619
db676c2
4a89eaa
1e16cd2
3a093c8
41b9e3f
f906cc4
e07baa2
3d69ef1
eae79be
abd512f
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 |
|---|---|---|
| @@ -1,59 +1,168 @@ | ||
| import { useState, useEffect, RefObject } from "react"; | ||
| import { useState, useEffect, useRef, RefObject } from "react"; | ||
| import type { Message, SessionStatus } from "@/shared/types"; | ||
|
|
||
| interface UseAutoScrollOptions { | ||
| messages: Message[]; | ||
| sessionStatus: SessionStatus; | ||
| messagesContainerRef: RefObject<HTMLDivElement>; | ||
| messagesEndRef: RefObject<HTMLDivElement>; | ||
| messagesEndRef: RefObject<HTMLDivElement>; // Empty div at end of messages | ||
| lastMessageRef: RefObject<HTMLDivElement>; // Last message element | ||
| // Configuration options | ||
| scrollThreshold?: number; // Distance from bottom to consider "at bottom" (default: 100) | ||
| inputHeightBuffer?: number; // Buffer for message input height (default: 80) | ||
| smoothScrollUser?: boolean; // Use smooth scroll for user messages (default: false) | ||
| } | ||
|
|
||
| /** | ||
| * Hook to manage auto-scroll behavior and scroll-to-bottom button | ||
| * | ||
| * Features: | ||
| * - USER messages: Scroll to top of viewport (push old messages up) | ||
| * - ASSISTANT messages: Smart overflow detection | ||
| * - If there's visible space below → NO scroll (messages appear naturally) | ||
| * - If content would be hidden → AUTO scroll (reveal new content) | ||
| * - Shows "scroll to bottom" button when user scrolls up | ||
| * - Respects user intent (doesn't auto-scroll if user manually scrolled up) | ||
| * | ||
| * UX Benefits: | ||
| * - Reduces unnecessary scrolling when viewport has space | ||
| * - User sees their question + answer simultaneously | ||
| * - Less jarring, more natural content flow | ||
| * - Only scrolls when content would actually be cut off | ||
| */ | ||
| export function useAutoScroll({ | ||
| messages, | ||
| sessionStatus, | ||
| messagesContainerRef, | ||
| messagesEndRef, | ||
| lastMessageRef, | ||
| scrollThreshold = 100, | ||
| inputHeightBuffer = 80, | ||
| smoothScrollUser = false, | ||
| }: UseAutoScrollOptions) { | ||
| const [showScrollButton, setShowScrollButton] = useState(false); | ||
| const [shouldAutoScroll, setShouldAutoScroll] = useState(true); | ||
| const [isUserScrolledUp, setIsUserScrolledUp] = useState(false); | ||
| const lastMessageCountRef = useRef(0); | ||
| const isAutoScrollingRef = useRef(false); // Track if we're auto-scrolling | ||
|
|
||
| // Auto-scroll when messages change or session status changes | ||
| // Check if user is near bottom (within threshold) | ||
| const isNearBottom = (threshold = scrollThreshold) => { | ||
| const container = messagesContainerRef.current; | ||
| if (!container) return false; | ||
|
|
||
| const { scrollTop, scrollHeight, clientHeight } = container; | ||
| return scrollHeight - scrollTop - clientHeight < threshold; | ||
| }; | ||
|
|
||
| // Scroll to bottom function | ||
| const scrollToBottom = (smooth = false) => { | ||
| messagesEndRef.current?.scrollIntoView({ | ||
| behavior: smooth ? 'smooth' : 'auto', | ||
| block: 'end' | ||
| }); | ||
| }; | ||
|
|
||
| // Auto-scroll when new messages arrive | ||
| useEffect(() => { | ||
| if (shouldAutoScroll) { | ||
| scrollToBottom(); | ||
| setShouldAutoScroll(false); | ||
| // Only auto-scroll if a NEW message was added (not on initial mount) | ||
| if (messages.length === 0 || messages.length === lastMessageCountRef.current) { | ||
| lastMessageCountRef.current = messages.length; | ||
| return; | ||
| } | ||
|
|
||
| const lastMessage = messages[messages.length - 1]; | ||
| const container = messagesContainerRef.current; | ||
|
|
||
| if (!isUserScrolledUp && container) { | ||
| isAutoScrollingRef.current = true; | ||
|
|
||
| if (lastMessage.role === 'user') { | ||
| // USER message: Scroll marker to TOP of viewport | ||
| requestAnimationFrame(() => { | ||
| const marker = lastMessageRef.current; | ||
| if (marker) { | ||
| // Get marker position relative to container | ||
| const markerTop = marker.offsetTop; | ||
| // Scroll container so marker is at the top | ||
| container.scrollTop = markerTop; | ||
|
|
||
| if (import.meta.env.DEV) { | ||
| console.log('[useAutoScroll] User message scrolled to top:', markerTop); | ||
| } | ||
| } else { | ||
| console.error('[useAutoScroll] lastMessageRef is null!'); | ||
| } | ||
|
|
||
| setTimeout(() => { | ||
| isAutoScrollingRef.current = false; | ||
| }, 100); | ||
| }); | ||
| } else { | ||
| // ASSISTANT message: Scroll to BOTTOM | ||
| requestAnimationFrame(() => { | ||
| scrollToBottom(false); | ||
|
|
||
| setTimeout(() => { | ||
| isAutoScrollingRef.current = false; | ||
| }, 100); | ||
| }); | ||
| } | ||
| } | ||
| }, [messages, sessionStatus, shouldAutoScroll]); | ||
|
|
||
| // Handle scroll detection for "scroll to bottom" button | ||
| lastMessageCountRef.current = messages.length; | ||
| }, [messages.length, isUserScrolledUp, smoothScrollUser]); | ||
|
|
||
| // Track user scroll behavior | ||
| useEffect(() => { | ||
| const container = messagesContainerRef.current; | ||
| if (!container) return; | ||
|
|
||
| const handleScroll = () => { | ||
| const { scrollTop, scrollHeight, clientHeight } = container; | ||
| const isNearBottom = scrollHeight - scrollTop - clientHeight < 100; | ||
| setShowScrollButton(!isNearBottom); | ||
| // Don't update state if we're auto-scrolling | ||
| if (isAutoScrollingRef.current) return; | ||
|
|
||
| const nearBottom = isNearBottom(); | ||
| setShowScrollButton(!nearBottom); | ||
| setIsUserScrolledUp(!nearBottom); | ||
| }; | ||
|
|
||
| container.addEventListener('scroll', handleScroll); | ||
| return () => container.removeEventListener('scroll', handleScroll); | ||
| }, [messagesContainerRef]); | ||
|
|
||
| function scrollToBottom(smooth = false) { | ||
| messagesEndRef.current?.scrollIntoView({ | ||
| behavior: smooth ? 'smooth' : 'auto', | ||
| block: 'end' | ||
| // ResizeObserver: Auto-scroll during streaming (when content grows) | ||
| useEffect(() => { | ||
| const container = messagesContainerRef.current; | ||
| if (!container) return; | ||
|
|
||
| const resizeObserver = new ResizeObserver(() => { | ||
| const { scrollTop, scrollHeight, clientHeight } = container; | ||
| const viewportBottom = scrollTop + clientHeight; | ||
| const contentBottom = scrollHeight; | ||
| const isContentHidden = contentBottom > viewportBottom + inputHeightBuffer; | ||
|
|
||
| // Only auto-scroll if: | ||
| // 1. In working session (streaming) | ||
| // 2. User hasn't scrolled up | ||
| // 3. Content would be hidden below viewport | ||
| if (sessionStatus === 'working' && !isUserScrolledUp && isContentHidden) { | ||
| isAutoScrollingRef.current = true; | ||
| scrollToBottom(false); | ||
| setTimeout(() => { | ||
| isAutoScrollingRef.current = false; | ||
| }, 50); | ||
| } | ||
| }); | ||
|
Comment on lines
+152
to
173
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. style: ResizeObserver may fire many times during streaming, potentially causing performance issues. Consider throttling or debouncing the scroll checks. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/features/session/hooks/useAutoScroll.ts
Line: 138:155
Comment:
**style:** ResizeObserver may fire many times during streaming, potentially causing performance issues. Consider throttling or debouncing the scroll checks.
How can I resolve this? If you propose a fix, please make it concise. |
||
| } | ||
|
|
||
| function handleScrollToBottomClick() { | ||
| setShouldAutoScroll(true); | ||
| resizeObserver.observe(container); | ||
| return () => resizeObserver.disconnect(); | ||
| }, [messagesContainerRef, isUserScrolledUp, sessionStatus, inputHeightBuffer]); | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| // Manual scroll to bottom (resets user scroll state) | ||
| const handleScrollToBottomClick = () => { | ||
| setIsUserScrolledUp(false); | ||
| scrollToBottom(true); | ||
| } | ||
| }; | ||
|
|
||
| return { | ||
| showScrollButton, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ interface ChatProps { | |||||
| sessionStatus: SessionStatus; | ||||||
| parseContent: (content: string) => ReactNode; | ||||||
| messagesEndRef: RefObject<HTMLDivElement>; | ||||||
| lastMessageRef: RefObject<HTMLDivElement>; | ||||||
| messagesContainerRef: RefObject<HTMLDivElement>; | ||||||
| showScrollButton?: boolean; | ||||||
| onScrollToBottom?: () => void; | ||||||
|
|
@@ -25,6 +26,7 @@ export function Chat({ | |||||
| sessionStatus, | ||||||
| parseContent, | ||||||
| messagesEndRef, | ||||||
| lastMessageRef, | ||||||
| messagesContainerRef, | ||||||
| showScrollButton = false, | ||||||
| onScrollToBottom, | ||||||
|
|
@@ -55,13 +57,18 @@ export function Chat({ | |||||
| ) : ( | ||||||
| <> | ||||||
| <div className="flex flex-col gap-6 pb-8 min-h-0"> | ||||||
| {messages.map(message => ( | ||||||
| <MessageItem | ||||||
| key={message.id} | ||||||
| message={message} | ||||||
| parseContent={parseContent} | ||||||
| toolResultMap={toolResultMap} | ||||||
| /> | ||||||
| {messages.map((message, index) => ( | ||||||
| <div key={message.id}> | ||||||
| <MessageItem | ||||||
| message={message} | ||||||
| parseContent={parseContent} | ||||||
| toolResultMap={toolResultMap} | ||||||
| /> | ||||||
| {/* Invisible ref marker - always renders even if MessageItem returns null */} | ||||||
| {index === messages.length - 1 && ( | ||||||
| <div ref={lastMessageRef} style={{ height: 0, width: 0 }} /> | ||||||
|
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. style: Inline styles bypass Tailwind and break consistency. Use Tailwind classes instead:
Suggested change
Context Used: Context from Prompt To Fix With AIThis is a comment left during a code review.
Path: src/features/session/ui/Chat.tsx
Line: 69:69
Comment:
**style:** Inline styles bypass Tailwind and break consistency. Use Tailwind classes instead:
```suggestion
<div ref={lastMessageRef} className="h-0 w-0" />
```
**Context Used:** Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=5f29a80f-5c70-41a4-8bfe-6ea6c6a3911c))
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| )} | ||||||
| </div> | ||||||
|
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: Wrapping each MessageItem in a div breaks the original gap-6 spacing (line 59). The gap applies between direct children, so the wrapper divs will have 6-spacing, but MessageItem's internal content won't benefit from the original layout logic. Is the gap-6 spacing still correct with the wrapper divs, or does this change the visual spacing between messages? Prompt To Fix With AIThis is a comment left during a code review.
Path: src/features/session/ui/Chat.tsx
Line: 61:71
Comment:
**logic:** Wrapping each MessageItem in a div breaks the original gap-6 spacing (line 59). The gap applies between direct children, so the wrapper divs will have 6-spacing, but MessageItem's internal content won't benefit from the original layout logic. Is the gap-6 spacing still correct with the wrapper divs, or does this change the visual spacing between messages?
How can I resolve this? If you propose a fix, please make it concise.
coderabbitai[bot] marked this conversation as resolved.
|
||||||
| ))} | ||||||
|
Comment on lines
+54
to
65
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. Wrapper divs may cause extra spacing when MessageItem returns null. Each message is now wrapped in a div, but Consider filtering messages before rendering or conditionally rendering the wrapper: <div className="flex flex-col gap-6 pb-8 min-h-0">
- {messages.map((message, index) => (
+ {messages.map((message, index) => {
+ const isLast = index === messages.length - 1;
+ return (
<div
key={message.id}
- ref={index === messages.length - 1 ? lastMessageRef : undefined}
+ ref={isLast ? lastMessageRef : undefined}
>
<MessageItem
message={message}
parseContent={parseContent}
toolResultMap={toolResultMap}
/>
</div>
- ))}
+ );
+ })}
</div>Or better, conditionally render the wrapper only when MessageItem would render: {messages.map((message, index) => {
const isLast = index === messages.length - 1;
const messageItem = (
<MessageItem
message={message}
parseContent={parseContent}
toolResultMap={toolResultMap}
/>
);
return isLast ? (
<div key={message.id} ref={lastMessageRef}>
{messageItem}
</div>
) : (
messageItem
);
})}However, this second approach defeats the purpose of always having a ref target for scrolling. 🤖 Prompt for AI Agents |
||||||
| </div> | ||||||
| {sessionStatus === 'working' && ( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,17 +43,24 @@ export function MessageInput({ | |
| const resizeTextarea = () => { | ||
| const el = textareaRef.current; | ||
| if (!el) return; | ||
| el.style.height = 'auto'; | ||
| el.style.height = Math.min(el.scrollHeight, 200) + 'px'; | ||
| // Reset to minimum first | ||
| el.style.height = '40px'; | ||
| // Then expand to fit content, max 200px | ||
| const newHeight = Math.min(Math.max(el.scrollHeight, 40), 200); | ||
| el.style.height = newHeight + 'px'; | ||
|
Comment on lines
+46
to
+50
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. style: Potential flashing issue: resetting to 40px then immediately expanding may cause visible height jump during typing. Consider setting height only when scrollHeight differs from current height. Have you tested this with rapid typing or paste operations to ensure smooth height transitions? Prompt To Fix With AIThis is a comment left during a code review.
Path: src/features/session/ui/MessageInput.tsx
Line: 46:50
Comment:
**style:** Potential flashing issue: resetting to 40px then immediately expanding may cause visible height jump during typing. Consider setting height only when scrollHeight differs from current height. Have you tested this with rapid typing or paste operations to ensure smooth height transitions?
How can I resolve this? If you propose a fix, please make it concise. |
||
| }; | ||
|
|
||
| // Auto-resize textarea | ||
| useEffect(() => { | ||
| resizeTextarea(); | ||
| }, [messageInput]); | ||
|
|
||
| // Set initial height on mount | ||
| useEffect(() => { | ||
| resizeTextarea(); | ||
| const el = textareaRef.current; | ||
| if (el) { | ||
| el.style.height = '40px'; | ||
| } | ||
| }, []); | ||
|
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. style: Duplicate initialization: inline style on line 96 already sets height to 40px, making this useEffect redundant. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/features/session/ui/MessageInput.tsx
Line: 59:64
Comment:
**style:** Duplicate initialization: inline style on line 96 already sets height to 40px, making this useEffect redundant.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| return ( | ||
|
|
@@ -86,15 +93,15 @@ export function MessageInput({ | |
| resizeTextarea(); | ||
| }} | ||
| onBlur={() => setIsFocused(false)} | ||
| style={{ height: '40px' }} | ||
|
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. style: Inline style conflicts with CSS class: setting height='40px' inline means the resizeTextarea() function must override it every time. The inline style takes precedence, so resizeTextarea() correctly overwrites it, but this creates redundant DOM operations. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/features/session/ui/MessageInput.tsx
Line: 96:96
Comment:
**style:** Inline style conflicts with CSS class: setting height='40px' inline means the resizeTextarea() function must override it every time. The inline style takes precedence, so resizeTextarea() correctly overwrites it, but this creates redundant DOM operations.
How can I resolve this? If you propose a fix, please make it concise. |
||
| className=" | ||
| flex-1 bg-transparent border-none outline-none resize-none | ||
| text-body-lg text-foreground placeholder:text-muted-foreground | ||
| min-h-[24px] max-h-[200px] | ||
| max-h-[200px] | ||
| font-sans | ||
| overflow-y-auto | ||
| scrollbar-vibrancy | ||
| " | ||
| rows={1} | ||
| /> | ||
|
|
||
| {/* Action Buttons */} | ||
|
|
||
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.
logic: Missing
isNearBottomdependency in the cleanupPrompt To Fix With AI