fix(chat): Remove isButtonPressed state from ChatPanel #278
Conversation
The "new chat" button was disappearing after interacting with the map. This was caused by the `isButtonPressed` state in the `ChatPanel` component, which was a remnant of a previous implementation and was not being reset correctly. This change removes the `isButtonPressed` state and all associated logic from the `ChatPanel` component. The "new chat" button's visibility is now solely dependent on the number of messages and the device type.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughAdded a server-side Changes
Sequence Diagram(s)sequenceDiagram
participant Mobile as MobileIconsBar / ChatPanel
participant UI as Client UI State (useUIState)
participant Actions as AI Actions (useActions)
participant Server as Server Action (clearChat)
Mobile->>UI: setMessages([]) %%{style: fill:#f3f4f6}%%
Mobile->>Actions: clearChat() %%{style: fill:#e6fffa}%%
Actions->>Server: invoke clearChat()
Server-->>Actions: new chatId, messages=[]
Actions-->>UI: propagate new AI state (chatId/messages)
UI-->>Mobile: UI updates (empty conversation)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
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:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
This commit addresses an issue where the "new chat" button did not correctly start a new chat session. The previous implementation using client-side routing (`router.push('/')`) did not reset the AI state, causing the old chat to persist.
This change introduces a new `clearChat` server action that programmatically resets the AI and UI states. Both the desktop (`ChatPanel`) and mobile (`MobileIconsBar`) "new chat" buttons now use this action to ensure a clean state for new conversations. This approach is more robust and aligns with the application's state management patterns.
This also resolves the initial issue where the button would disappear incorrectly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/chat-panel.tsx (2)
4-6: Fix invalid type-only import for AI and add proper typing for useActions.
useUIState<typeof AI>()requires importing AI as a value, not a type. Current import will fail TypeScript. Also typeuseActionsfor safer calls.-import type { AI, UIState } from '@/app/actions' +import { AI } from '@/app/actions' +import type { UIState } from '@/app/actions' @@ - const [, setMessages] = useUIState<typeof AI>() - const { submit, clearChat } = useActions() + const [, setMessages] = useUIState<typeof AI>() + const { submit, clearChat } = useActions<typeof AI>()Also applies to: 21-23
38-51: Prevent duplicate submissions and whitespace-only sends; clear input on send+ const [isSubmitting, setIsSubmitting] = useState(false) @@ const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => { e.preventDefault() + if (isSubmitting) return + if (input.trim().length === 0) return + setIsSubmitting(true) setMessages(currentMessages => [ ...currentMessages, { id: nanoid(), component: <UserMessage message={input} /> } ]) + setInput('') const formData = new FormData(e.currentTarget) // Removed mcp argument from submit call - const responseMessage = await submit(formData) - setMessages(currentMessages => [...currentMessages, responseMessage as any]) + try { + const responseMessage = await submit(formData) + setMessages(currentMessages => [...currentMessages, responseMessage as any]) + } finally { + setIsSubmitting(false) + } } @@ - <Button + <Button type="submit" size={'icon'} variant={'ghost'} className={cn( 'absolute top-1/2 transform -translate-y-1/2', isMobile ? 'right-1' : 'right-2' )} - disabled={input.length === 0} + disabled={isSubmitting || input.trim().length === 0} >After applying, test by rapidly pressing Enter or clicking the send button to confirm only one request is sent per message and blank inputs are blocked.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
app/actions.tsx(2 hunks)components/chat-panel.tsx(2 hunks)components/mobile-icons-bar.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/mobile-icons-bar.tsx (4)
app/actions.tsx (1)
AI(283-353)components/chat.tsx (3)
aiState(56-61)Chat(20-123)path(50-54)app/page.tsx (1)
Page(9-18)app/search/[id]/page.tsx (1)
SearchPage(27-76)
app/actions.tsx (2)
lib/actions/chat.ts (3)
msg(119-127)clearChats(77-99)updateDrawingContext(165-205)components/chat.tsx (2)
aiState(56-61)Chat(20-123)
components/chat-panel.tsx (2)
components/chat.tsx (2)
Chat(20-123)path(50-54)components/sidebar/chat-history-client.tsx (1)
ChatHistoryClient(28-161)
| async function clearChat() { | ||
| 'use server' | ||
|
|
||
| const aiState = getMutableAIState<typeof AI>() | ||
|
|
||
| aiState.done({ | ||
| chatId: nanoid(), | ||
| messages: [] | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return new chatId and preserve other AI state on clear.
Return the generated chatId so clients can sync the URL, and keep other fields (e.g., isSharePage).
-async function clearChat() {
- 'use server'
-
- const aiState = getMutableAIState<typeof AI>()
-
- aiState.done({
- chatId: nanoid(),
- messages: []
- })
-}
+async function clearChat() {
+ 'use server'
+ const aiState = getMutableAIState<typeof AI>()
+ const prev = aiState.get()
+ const newChatId = nanoid()
+ aiState.done({
+ ...prev,
+ chatId: newChatId,
+ messages: []
+ })
+ return { chatId: newChatId }
+}📝 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.
| async function clearChat() { | |
| 'use server' | |
| const aiState = getMutableAIState<typeof AI>() | |
| aiState.done({ | |
| chatId: nanoid(), | |
| messages: [] | |
| }) | |
| } | |
| async function clearChat() { | |
| 'use server' | |
| const aiState = getMutableAIState<typeof AI>() | |
| const prev = aiState.get() | |
| const newChatId = nanoid() | |
| aiState.done({ | |
| ...prev, | |
| chatId: newChatId, | |
| messages: [] | |
| }) | |
| return { chatId: newChatId } | |
| } |
🤖 Prompt for AI Agents
In app/actions.tsx around lines 251 to 260, the clearChat function currently
replaces the AI state wholesale with a new object containing only chatId and
messages; change it to generate the new chatId, read the current mutable AI
state, and call aiState.done with a merged object that preserves existing fields
(for example isSharePage and any other properties) while setting chatId to the
new id and messages to an empty array, and finally return the generated chatId
so callers can sync the URL.
| clearChat, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Expose typed actions to clients expecting clearChat result.
After returning { chatId } from clearChat, ensure clients await and use it to keep route and state consistent.
🤖 Prompt for AI Agents
In app/actions.tsx around lines 286-287, the actions export needs to expose
clearChat with an explicit return type so callers can await the returned {
chatId } and keep route/state consistent; update the clearChat function
signature to return Promise<{ chatId: string }>, ensure the exported actions
object includes the typed clearChat, and update any call sites to await
clearChat() and use the returned chatId to update route/state.
| const handleClear = async () => { | ||
| setMessages([]) | ||
| await clearChat() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider URL-and-state alignment after “New” on desktop.
handleClear resets state but not the route; saved chats use /search/${chatId}. After clear, the path may still reference the old chat. Once clearChat returns the new chatId, replace the URL similar to MobileIconsBar.
Also applies to: 63-83
🤖 Prompt for AI Agents
In components/chat-panel.tsx around lines 53-56 (and similarly 63-83),
handleClear resets local state but doesn't update the URL so the route can still
reference the old chat; change it to await clearChat(), capture the returned new
chatId, then reset messages/state and call
router.replace(`/search/${newChatId}`) (use replace not push) to align URL and
state like MobileIconsBar; ensure router is available and handle
errors/undefined chatId gracefully.
| const handleNewChat = async () => { | ||
| setMessages([]) | ||
| await clearChat() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Guard against double taps; optionally update route after clear.
Add a pending flag to prevent repeated clears, and (once clearChat returns the new chatId) replace the URL so it matches the fresh chat.
export const MobileIconsBar: React.FC = () => {
- const [, setMessages] = useUIState<typeof AI>()
- const { clearChat } = useActions<typeof AI>()
+ const [, setMessages] = useUIState<typeof AI>()
+ const { clearChat } = useActions<typeof AI>()
+ const [isClearing, setIsClearing] = React.useState(false)
- const handleNewChat = async () => {
- setMessages([])
- await clearChat()
+ const handleNewChat = async () => {
+ if (isClearing) return
+ setIsClearing(true)
+ try {
+ setMessages([])
+ const result = await clearChat() // expect { chatId }
+ if (result?.chatId && window.location.pathname.startsWith('/search/')) {
+ window.history.replaceState({}, '', `/search/${result.chatId}`)
+ }
+ } finally {
+ setIsClearing(false)
+ }
}And disable the button while clearing:
- <Button variant="ghost" size="icon" onClick={handleNewChat}>
+ <Button variant="ghost" size="icon" onClick={handleNewChat} disabled={isClearing}>
<Plus className="h-[1.2rem] w-[1.2rem]" />
</Button>📝 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.
| const handleNewChat = async () => { | |
| setMessages([]) | |
| await clearChat() | |
| } | |
| // components/mobile-icons-bar.tsx | |
| export const MobileIconsBar: React.FC = () => { | |
| const [, setMessages] = useUIState<typeof AI>() | |
| const { clearChat } = useActions<typeof AI>() | |
| const [isClearing, setIsClearing] = React.useState(false) | |
| const handleNewChat = async () => { | |
| if (isClearing) return | |
| setIsClearing(true) | |
| try { | |
| setMessages([]) | |
| const result = await clearChat() // expect { chatId } | |
| if (result?.chatId && window.location.pathname.startsWith('/search/')) { | |
| window.history.replaceState({}, '', `/search/${result.chatId}`) | |
| } | |
| } finally { | |
| setIsClearing(false) | |
| } | |
| } | |
| return ( | |
| <div className="mobile-icons-bar"> | |
| {/* other icons… */} | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={handleNewChat} | |
| disabled={isClearing} | |
| > | |
| <Plus className="h-[1.2rem] w-[1.2rem]" /> | |
| </Button> | |
| {/* other icons… */} | |
| </div> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In components/mobile-icons-bar.tsx around lines 25–28, guard against double taps
by adding a local pending state (e.g., isClearing) and early-return if already
pending; set it true before starting, and false in a finally block. Call
setMessages([]) and then await clearChat(), capture its returned newChatId, and
if present call the router replace (or use history.replaceState) to update the
URL to the fresh chatId. Also bind the clear button's disabled attribute to the
pending flag so the UI is disabled while clearing.
This commit addresses an issue where the "new chat" button did not correctly start a new chat session. The previous implementation using client-side routing (`router.push('/')`) did not reset the AI state, causing the old chat to persist.
This change introduces a new `clearChat` server action that programmatically resets the AI and UI states. Both the desktop (`ChatPanel`) and mobile (`MobileIconsBar`) "new chat" buttons now use this action to ensure a clean state for new conversations. This approach is more robust and aligns with the application's state management patterns.
This also resolves the initial issue where the button would disappear incorrectly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/mobile-icons-bar.tsx (1)
32-34: Disable the button while clearing.Improves UX and prevents re-entry.
- <Button variant="ghost" size="icon" onClick={handleNewChat}> + <Button variant="ghost" size="icon" onClick={handleNewChat} disabled={isClearing} aria-busy={isClearing}> <Plus className="h-[1.2rem] w-[1.2rem]" /> </Button>
♻️ Duplicate comments (2)
components/mobile-icons-bar.tsx (2)
25-28: Guard double taps and update URL with new chatId.Prevents repeated clears and keeps the address bar in sync with the fresh chat.
- const handleNewChat = async () => { - setMessages([]) - await clearChat() - } + const handleNewChat = async () => { + if (isClearing) return + setIsClearing(true) + try { + setMessages([]) + const result = await clearChat() // expect { chatId } + if (result?.chatId && window.location.pathname.startsWith('/search/')) { + window.history.replaceState({}, '', `/search/${result.chatId}`) + } + } finally { + setIsClearing(false) + } + }Verify server-side shape and duplication of clearChat, and ensure the old ChatPanel flag is gone:
#!/bin/bash set -euo pipefail echo "Leftover isButtonPressed usages:" rg -n 'isButtonPressed|setIsButtonPressed' -S echo "clearChat definitions:" rg -nP '^\s*(export\s+)?(async\s+)?function\s+clearChat\b|^\s*const\s+clearChat\s*=' -C2
22-24: Type actions and prep pending state.Use the generic for useActions and add a local pending flag for subsequent diff.
- const [, setMessages] = useUIState<typeof AI>() - const { clearChat } = useActions() + const [, setMessages] = useUIState<typeof AI>() + const { clearChat } = useActions<typeof AI>() + const [isClearing, setIsClearing] = React.useState(false)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (2)
.env(1 hunks)components/mobile-icons-bar.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
.env (1)
lib/db/migrate.ts (1)
runMigrations(8-34)
components/mobile-icons-bar.tsx (4)
app/actions.tsx (4)
AI(283-353)aiState(343-478)state(290-340)aiState(278-289)components/chat.tsx (3)
aiState(56-61)Chat(20-123)path(50-54)app/page.tsx (1)
Page(9-18)components/chat-panel.tsx (1)
router(66-68)
🪛 dotenv-linter (3.3.0)
.env
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🔇 Additional comments (1)
components/mobile-icons-bar.tsx (1)
4-6: Type your AI actions for safety.Since AI is exported from app/actions, prefer typed hooks for autocomplete and compile-time checks.
Apply:
-import { useUIState, useActions } from 'ai/rsc' +import { useUIState, useActions } from 'ai/rsc' +// below, use: useActions<typeof AI>()Likely an incorrect or invalid review comment.
| @@ -0,0 +1 @@ | |||
| DATABASE_URL="postgresql://user:password@host:port/db" | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Do not commit .env; move to .env.example and drop quotes.
Committing environment files is risky. Replace this with an example file and ensure .env is gitignored. Also fix the dotenv-linter warning by removing quotes.
Apply:
-DATABASE_URL="postgresql://user:password@host:port/db"
+DATABASE_URL=postgresql://<user>:<password>@<host>:<port>/<db>Also:
- Add to .gitignore:
+.env
+.env.local- Rename this file to .env.example (I can draft it if you want).
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
In .env around line 1, the committed environment file with quoted sensitive
values must be removed from the repo, replaced by an example file, and quotes
dropped; rename the current .env to .env.example (remove any real secrets and
unquote values), add entries .env and .env.local to .gitignore, and remove the
committed .env from git history/staging (e.g., git rm --cached .env and commit)
so only .env.example remains tracked.
2 participants
User description
The "new chat" button was disappearing after interacting with the map. This was caused by the
isButtonPressedstate in theChatPanelcomponent, which was a remnant of a previous implementation and was not being reset correctly.This change removes the
isButtonPressedstate and all associated logic from theChatPanelcomponent. The "new chat" button's visibility is now solely dependent on the number of messages and the device type.PR Type
Bug fix
Description
Remove
isButtonPressedstate causing new chat button disappearanceSimplify button visibility logic to depend only on messages and device type
Clean up associated useEffect and form submission logic
Diagram Walkthrough
File Walkthrough
chat-panel.tsx
Remove isButtonPressed state and associated logiccomponents/chat-panel.tsx
isButtonPressedstate variable and setterSummary by CodeRabbit