-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Display example prompts on desktop #299
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
Conversation
Refactored the chat component to ensure that example prompts are displayed on desktop, tablet, and mobile devices. - Created a new `ConditionalChatMessages` component to handle the logic of displaying either the `EmptyScreen` or the `ChatMessages` based on whether there are messages. - Updated the `Chat` component to use the new `ConditionalChatMessages` component in both mobile and desktop layouts, removing duplicated code and improving maintainability.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@ngoiyaeric has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughCentralizes chat message rendering by introducing ConditionalChatMessages. chat.tsx now delegates between EmptyScreen and ChatMessages through this new component for both mobile and desktop layouts, passing messages, showEmptyScreen, and setInput. ChatPanel usage persists. MapDataProvider remains wrapping both branches. No exported/public signatures in existing files changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Chat as Chat (chat.tsx)
participant CCM as ConditionalChatMessages
participant ES as EmptyScreen
participant CM as ChatMessages
User->>Chat: Open chat / interact
Chat->>CCM: { messages, showEmptyScreen, setInput }
alt showEmptyScreen = true
CCM->>ES: Render EmptyScreen
ES-->>User: Prompt for first message
User->>ES: Submit message text
ES-->>CCM: submitMessage(text)
CCM-->>Chat: setInput(text)
else showEmptyScreen = false
CCM->>CM: Render ChatMessages(messages)
CM-->>User: Display messages
end
Note over Chat: ChatPanel remains rendered separately
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
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.
Main issue: the new component uses any[] for messages, which reduces type safety and maintainability. Prefer deriving prop types from ChatMessages and EmptyScreen using React.ComponentProps to keep them in sync. Minor readability improvement: pass setInput directly to EmptyScreen rather than wrapping it in an arrow function. Overall, the refactor improves reusability and removes duplication.
Additional notes (1)
- Readability |
components/conditional-chat-messages.tsx:16-18
You can simplify the inline function by passingsetInputdirectly. This reduces noise and avoids creating a new function each render.
Summary of changes
- Replaced inline conditional rendering of
EmptyScreen/ChatMessagesincomponents/chat.tsxwith a new reusableConditionalChatMessagescomponent for both mobile and desktop layouts. - Added
components/conditional-chat-messages.tsxto encapsulate the logic of showing example prompts when there are no messages. - Removed duplicated display logic in
Chatand cleaned up the previous TODO for desktop empty state by using the new component.
| showEmptyScreen, | ||
| setInput, | ||
| }: { | ||
| messages: any[]; |
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.
Avoid using any[] for messages. This weakens type safety and can hide interface mismatches. Since ConditionalChatMessages forwards messages directly to ChatMessages, you can derive the type from ChatMessages's props instead of using any. Also, type the setInput callback based on EmptyScreen's submitMessage prop to prevent future drift between components.
Suggested pattern: derive prop types via React.ComponentProps and Pick so they always stay in sync with the child components.
Suggestion
You can tighten the props without changing runtime behavior:
import { ChatMessages } from './chat-messages'
import { EmptyScreen } from './empty-screen'
type ConditionalChatMessagesProps = {
showEmptyScreen: boolean
setInput: React.ComponentProps<typeof EmptyScreen>['submitMessage']
} & Pick<React.ComponentProps<typeof ChatMessages>, 'messages'>
export function ConditionalChatMessages({
messages,
showEmptyScreen,
setInput,
}: ConditionalChatMessagesProps) {
if (showEmptyScreen) {
return <EmptyScreen submitMessage={setInput} />
}
return <ChatMessages messages={messages} />
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
This commit updates the chat component to display example prompts on desktop, tablet, and mobile devices, and positions them below the chat input area. - The `Chat` component is modified to render the `ChatPanel` (input) before the `EmptyScreen` (prompts) or `ChatMessages` (history). - This change ensures a consistent layout across all screen sizes, with the input field always appearing above the content area. - The unnecessary `conditional-chat-messages.tsx` component has been removed, and its logic is now handled directly within `chat.tsx`.
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat.tsx (1)
100-106: Remove nested MapDataProvider in components/chat.tsx
Chat is already wrapped by MapDataProvider at the page level in app/page.tsx and app/search/[id]/page.tsx, so the inner providers in both mobile and desktop branches reinitialize the context and reset state. Remove the wrappers:- return ( - <MapDataProvider> - <div className="flex justify-start items-start"> + return ( + <div className="flex justify-start items-start"> … - </MapDataProvider> )Apply the same removal around the mobile branch’s
<MapDataProvider>.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/chat.tsx(3 hunks)components/conditional-chat-messages.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/conditional-chat-messages.tsx (3)
components/empty-screen.tsx (2)
EmptyScreen(27-59)submitMessage(46-48)components/chat-messages.tsx (2)
ChatMessages(11-70)ChatMessagesProps(7-9)components/chat-panel.tsx (2)
ChatPanel(20-177)ChatPanelProps(14-18)
components/chat.tsx (4)
components/conditional-chat-messages.tsx (1)
ConditionalChatMessages(4-24)components/chat-messages.tsx (1)
ChatMessages(11-70)app/page.tsx (1)
Page(9-18)components/empty-screen.tsx (1)
EmptyScreen(27-59)
🔇 Additional comments (1)
components/conditional-chat-messages.tsx (1)
4-12: Confirm UX: Should clicking an example prompt prefill or auto‑send?Right now, submitMessage only sets the input. If the intended behavior is “tap example → send immediately,” we should instead call the same submit/append handler used by ChatPanel (or accept an optional onSubmitMessage prop here and prefer it over setInput). Do you want me to wire that up?
components/chat.tsx
Outdated
| <ConditionalChatMessages | ||
| messages={messages} | ||
| showEmptyScreen={showEmptyScreen} | ||
| setInput={setInput} | ||
| /> |
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.
🛠️ Refactor suggestion | 🟠 Major
Avoid derived state for showEmptyScreen; compute from messages.
You can drop the state + effect and derive it inline to reduce renders and avoid staleness.
- const [showEmptyScreen, setShowEmptyScreen] = useState(false)
-
- useEffect(() => {
- setShowEmptyScreen(messages.length === 0)
- }, [messages])
+ const showEmptyScreen = messages.length === 0This keeps usage here unchanged:
<ConditionalChatMessages messages={messages} showEmptyScreen={showEmptyScreen} setInput={setInput} />🤖 Prompt for AI Agents
In components/chat.tsx around lines 88 to 92, drop the local showEmptyScreen
state and effect and instead pass a derived boolean computed from messages
directly to ConditionalChatMessages (e.g. replace showEmptyScreen with
messages.length === 0 or a small predicate you prefer). Remove the useState and
useEffect that set showEmptyScreen, and update the JSX to
<ConditionalChatMessages messages={messages} showEmptyScreen={messages.length
=== 0} setInput={setInput} /> (or use your chosen derived expression) so renders
are not driven by stale derived state.
| messages: any[]; | ||
| showEmptyScreen: boolean; | ||
| setInput: (value: string) => void; |
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.
🛠️ Refactor suggestion | 🟠 Major
Strongly type messages; avoid any[].
Use the existing UIState to keep types consistent with ChatPanel/ChatMessages.
+import type { UIState } from 'ai/rsc'
export function ConditionalChatMessages({
messages,
showEmptyScreen,
setInput,
}: {
- messages: any[];
+ messages: UIState;
showEmptyScreen: boolean;
setInput: (value: string) => void;
}) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/conditional-chat-messages.tsx around lines 9 to 11, the messages
prop is typed as any[]; replace it with the strongly-typed messages from the
existing UIState used by ChatPanel/ChatMessages (e.g. UIState['messages']).
Import the UIState type from the same module where ChatPanel/ChatMessages get
it, update the prop signature to use that type instead of any[], and adjust any
local usages to match the UIState message shape.
| <EmptyScreen | ||
| submitMessage={(message) => { | ||
| setInput(message); | ||
| }} | ||
| /> |
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.
🧹 Nitpick | 🔵 Trivial
Pass setInput directly to avoid per‑render function allocation.
- <EmptyScreen
- submitMessage={(message) => {
- setInput(message);
- }}
- />
+ <EmptyScreen submitMessage={setInput} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <EmptyScreen | |
| submitMessage={(message) => { | |
| setInput(message); | |
| }} | |
| /> | |
| <EmptyScreen submitMessage={setInput} /> |
🤖 Prompt for AI Agents
In components/conditional-chat-messages.tsx around lines 15 to 19, you're
creating a new arrow function on each render for the EmptyScreen submitMessage
prop which causes per-render allocation; instead pass the existing setInput
function directly (submitMessage={setInput}). Ensure setInput's signature
matches the EmptyScreen prop (e.g., (message: string) => void) or adapt types so
it accepts the same parameter, then remove the inline arrow wrapper.
User description
Refactored the chat component to ensure that example prompts are displayed on desktop, tablet, and mobile devices.
ConditionalChatMessagescomponent to handle the logic of displaying either theEmptyScreenor theChatMessagesbased on whether there are messages.Chatcomponent to use the newConditionalChatMessagescomponent in both mobile and desktop layouts, removing duplicated code and improving maintainability.PR Type
Enhancement
Description
Refactored chat component to display example prompts on all devices
Created reusable
ConditionalChatMessagescomponent for message display logicUnified mobile and desktop layouts by removing code duplication
Added empty screen support for desktop layout
Diagram Walkthrough
File Walkthrough
chat.tsx
Unified chat layouts using conditional componentcomponents/chat.tsx
ChatMessagesandEmptyScreenusage withConditionalChatMessagesconditional-chat-messages.tsx
Added conditional chat messages componentcomponents/conditional-chat-messages.tsx
EmptyScreenwhen no messages existChatMessageswhen messages are presentSummary by CodeRabbit
New Features
Refactor
Chores