-
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 all 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 |
|---|---|---|
|
|
@@ -3,8 +3,6 @@ import type { ToolResultMap } from "./chat-types"; | |
| import { MessageItem } from "./MessageItem"; | ||
| import { EmptyState } from "@/components/ui/EmptyState"; | ||
| import { Skeleton } from "@/components/ui/skeleton"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import { ChevronDown } from "lucide-react"; | ||
| import type { ReactNode, RefObject } from "react"; | ||
|
|
||
| interface ChatProps { | ||
|
|
@@ -13,9 +11,8 @@ interface ChatProps { | |
| sessionStatus: SessionStatus; | ||
| parseContent: (content: string) => ReactNode; | ||
| messagesEndRef: RefObject<HTMLDivElement>; | ||
| lastMessageRef: RefObject<HTMLDivElement>; | ||
| messagesContainerRef: RefObject<HTMLDivElement>; | ||
| showScrollButton?: boolean; | ||
| onScrollToBottom?: () => void; | ||
| toolResultMap: ToolResultMap; | ||
| } | ||
|
|
||
|
|
@@ -25,17 +22,16 @@ export function Chat({ | |
| sessionStatus, | ||
| parseContent, | ||
| messagesEndRef, | ||
| lastMessageRef, | ||
| messagesContainerRef, | ||
| showScrollButton = false, | ||
| onScrollToBottom, | ||
| toolResultMap, | ||
| }: ChatProps) { | ||
| return ( | ||
| <div | ||
| id="chat-messages" | ||
| role="log" | ||
| aria-live="polite" | ||
| className="relative flex-1 overflow-y-auto overflow-x-hidden scroll-smooth min-h-0 px-6 pt-6" | ||
| className="relative flex-1 overflow-y-auto overflow-x-hidden scroll-smooth motion-reduce:scroll-auto min-h-0 px-6 pt-6" | ||
| ref={messagesContainerRef} | ||
| > | ||
| {loading ? ( | ||
|
|
@@ -55,43 +51,32 @@ export function Chat({ | |
| ) : ( | ||
| <> | ||
| <div className="flex flex-col gap-6 pb-8 min-h-0"> | ||
| {messages.map(message => ( | ||
| <MessageItem | ||
| {messages.map((message, index) => ( | ||
| <div | ||
| key={message.id} | ||
| message={message} | ||
| parseContent={parseContent} | ||
| toolResultMap={toolResultMap} | ||
| /> | ||
| ref={index === messages.length - 1 ? lastMessageRef : undefined} | ||
| > | ||
| <MessageItem | ||
| message={message} | ||
| parseContent={parseContent} | ||
| toolResultMap={toolResultMap} | ||
| /> | ||
| </div> | ||
| ))} | ||
|
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' && ( | ||
| <div | ||
| role="status" | ||
| aria-live="polite" | ||
| className="flex items-center gap-2 p-2.5 px-3.5 mt-2 mr-auto max-w-[85%] bg-success/10 backdrop-blur-sm border border-success/30 rounded-xl text-success font-medium text-[0.85rem] shadow-sm animate-[pulse_0.6s_ease-in-out_infinite]" | ||
| className="flex items-center gap-2 p-2.5 px-3.5 mt-2 mr-auto max-w-[85%] bg-success/10 backdrop-blur-sm border border-success/30 rounded-xl text-success font-medium text-[0.85rem] shadow-sm animate-[pulse_0.6s_ease_infinite] motion-reduce:animate-none" | ||
| > | ||
| <div className="w-4 h-4 border-2 border-success/20 border-t-success rounded-full animate-spin flex-shrink-0" aria-hidden="true"></div> | ||
| <div className="w-4 h-4 border-2 border-success/20 border-t-success rounded-full animate-spin motion-reduce:animate-none flex-shrink-0" aria-hidden="true"></div> | ||
| <span>Claude is working...</span> | ||
| </div> | ||
| )} | ||
| <div ref={messagesEndRef} /> | ||
| </> | ||
| )} | ||
| {showScrollButton && ( | ||
| <div className="sticky bottom-6 flex justify-end pointer-events-none pb-6"> | ||
| <Button | ||
| variant="secondary" | ||
| size="icon" | ||
| className="rounded-full shadow-lg pointer-events-auto" | ||
| onClick={() => onScrollToBottom?.()} | ||
| title="Scroll to bottom" | ||
| aria-label="Scroll to bottom" | ||
| aria-controls="chat-messages" | ||
| > | ||
| <ChevronDown className="h-4 w-4" aria-hidden="true" /> | ||
| </Button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,21 +43,28 @@ 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]); | ||
|
|
||
| useEffect(() => { | ||
| resizeTextarea(); | ||
| }, []); | ||
|
|
||
| return ( | ||
| <div className="flex-shrink-0 m-0 px-6 pb-4 z-10 flex flex-col gap-3"> | ||
| <div className="relative flex-shrink-0 m-0 px-6 pb-4 z-10 flex flex-col gap-3"> | ||
| {/* Scroll fade overlay - positioned above the input */} | ||
| <div | ||
| className="absolute bottom-full left-0 right-0 h-32 pointer-events-none" | ||
| style={{ | ||
| background: 'linear-gradient(to bottom, transparent 0%, hsl(var(--background)) 100%)' | ||
| }} | ||
| /> | ||
|
|
||
| {/* Glassmorphic ChatBox */} | ||
| <div | ||
| className={` | ||
|
|
@@ -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.
style: ResizeObserver may fire many times during streaming, potentially causing performance issues. Consider throttling or debouncing the scroll checks.
Prompt To Fix With AI