Fix: ensure that chat scrolls to bottom properly (even when resizing etc.) #3024
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughThe chat tab components are refactored to eliminate imperative scroll-to-bottom behavior via refs and callbacks. ChatMessages transitions from a forwardRef component with a ChatMessagesHandle to a plain observer component using new Conversation rendering primitives. ChatInput and ChatTabContent APIs are updated to remove scroll-related props and ref handling. Changes
Sequence DiagramsequenceDiagram
autonumber
participant User
participant ChatInput as ChatInput<br/>(Updated)
participant ChatTabContent as ChatTabContent<br/>(Updated)
participant ChatMessages as ChatMessages<br/>(Updated)
participant Conversation as Conversation<br/>Components
rect rgb(220, 240, 255)
Note over ChatInput,Conversation: Before: Imperative Scroll Control
User->>ChatInput: Type & send message
ChatInput->>ChatInput: onScrollToBottom()
ChatTabContent->>ChatMessages: handleScrollToBottom via ref
ChatMessages->>ChatMessages: scrollToBottom()
end
rect rgb(240, 220, 255)
Note over ChatInput,Conversation: After: Declarative Scroll Control
User->>ChatInput: Type & send message
ChatInput->>ChatTabContent: Update messages prop
ChatTabContent->>ChatMessages: Pass messages & props
ChatMessages->>Conversation: Render with<br/>ConversationScrollButton
Conversation->>Conversation: Auto-scroll handled<br/>declaratively
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span three related files with cohesive refactoring patterns (removal of imperative scroll mechanisms), but involve multiple API signature changes, interface deletions, and structural component changes that require verification of consistent propagation across the component tree. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsx (1)
1-23: Add client boundary: this component uses hooks (useChat).Without 'use client', this will break at runtime in Next.js app dir.
As per coding guidelines
+ 'use client'; + import { type ChatMessage } from '@onlook/models'; import { useChat } from '../../../../_hooks/use-chat';apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
93-108: Enter handling callshandleEnterSelection()twice — can double-apply actions.Call it once and branch on the result.
- useEffect(() => { - const handleGlobalKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Enter' && suggestionRef.current?.handleEnterSelection()) { - e.preventDefault(); - e.stopPropagation(); - // Stop the event from bubbling to the canvas - e.stopImmediatePropagation(); - // Handle the suggestion selection - suggestionRef.current.handleEnterSelection(); - } - }; + useEffect(() => { + const handleGlobalKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Enter') { + const handled = suggestionRef.current?.handleEnterSelection?.(); + if (handled) { + e.preventDefault(); + e.stopPropagation(); + e.stopImmediatePropagation(); + } + } + };
🧹 Nitpick comments (7)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (3)
78-89: Great move to wrap with Conversation; localize “Thinking …”.Replace hardcoded text per i18n guideline.
As per coding guidelines
- {isStreaming && <div className="flex w-full h-full flex-row items-center gap-2 px-4 my-2 text-small content-start text-foreground-secondary"> - <Icons.LoadingSpinner className="animate-spin" /> - <p>Thinking ...</p> - </div>} + {isStreaming && ( + <div className="flex w-full h-full flex-row items-center gap-2 px-4 my-2 text-small content-start text-foreground-secondary"> + <Icons.LoadingSpinner className="animate-spin" /> + <p>{t(transKeys.editor.panels.edit.tabs.chat.thinking)}</p> + </div> + )}
37-62: Avoid redundant keys on children.Inner components already wrapped; remove keys on AssistantMessage/UserMessage.
- messageNode = <AssistantMessage key={message.id} message={message} isStreaming={isStreaming} />; + messageNode = <AssistantMessage message={message} isStreaming={isStreaming} />; ... - <UserMessage - key={message.id} + <UserMessage onEditMessage={onEditMessage} message={message} />
79-83: Skip “system” messages to prevent empty rows.Current renderMessage returns an empty wrapper for system; filter them out.
- {messages.map((message) => renderMessage(message))} + {messages + .filter((m) => m.role !== 'system') + .map((message) => renderMessage(message))}apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsx (1)
2-2: Prefer path alias instead of deep relative import.Use @/ for src mapping to improve maintainability.
As per coding guidelines
-import { useChat } from '../../../../_hooks/use-chat'; +import { useChat } from '@/app/project/[id]/_hooks/use-chat';apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (3)
163-169: Localize placeholder for ASK mode.Avoid hardcoded user text; use next‑intl key.
As per coding guidelines
- if (chatMode === ChatType.ASK) { - return 'Ask a question about your project...'; - } + if (chatMode === ChatType.ASK) { + return t(transKeys.editor.panels.edit.tabs.chat.input.askPlaceholder); + }
462-465: Localize tooltip “Stop response”.Replace with translation key.
As per coding guidelines
- <TooltipContent side="top" sideOffset={6} hideArrow> - {'Stop response'} - </TooltipContent> + <TooltipContent side="top" sideOffset={6} hideArrow> + {t(transKeys.editor.panels.edit.tabs.chat.actions.stopResponse)} + </TooltipContent>
181-183: Audit remaining hardcoded user-visible strings.These toasts/messages should use next‑intl.
As per coding guidelines
Examples to replace with t(transKeys...):
- Line 181: "Pasted image"
- Lines 213, 219: "Failed to get branch data", "Failed to load image file"
- Lines 236-237: "Image added to chat"
- Lines 299-300: "Added ${n} images to chat"
- Lines 360-361: "Failed to capture screenshot. Error: ..."
Also applies to: 213-221, 236-237, 299-300, 360-361
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-tab-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
packages/ui/src/components/ai-elements/conversation.tsx (1)
Conversation(12-20)
🔇 Additional comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
28-33: No remaining ChatMessages scroll API references
Confirmed no occurrences ofChatMessagesHandle,onScrollToBottom, orScrollControllerin the codebase.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Improves chat scrolling by removing manual scroll handling and relying on UI components for automatic scrolling.
onScrollToBottomprop and its usage inChatInput.scrollToBottommethod andScrollControllerinChatMessages.ConversationScrollButtoninChatMessagesfor automatic scrolling.ChatMessagesby removingforwardRefanduseImperativeHandle.ChatTabContentto removechatMessagesRefand related logic.ChatInputfor button styling.This description was created by
for f1d8881. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit