-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(chat): Prevent input clearing when clicking chat buttons #1521
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
🦋 Changeset detectedLatest commit: b9e7ecf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The chat box is no longer cleared when clicking buttons Previously, we had a bug where if either of the buttons in the agent chat was clicked, the ChatTextArea would get cleared. Now, the text box will only get cleared if the message is sent as part of the response. Moved input clearing logic to only execute when a message is successfully sent, preventing the input field from clearing prematurely when clicking "yes" or "no" buttons in the chat interface. This improves user experience by retaining the input content if an action fails or requires further interaction Fixes #1520
WalkthroughThe patch updates the agent chat interface so that the chat input area is only cleared when a message is actually sent, rather than on any button click. The logic for clearing the input and selected images is now directly tied to successful message dispatch, preserving user input during retries or other button interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent UI
participant User as User
participant ChatTextArea as Chat Input
participant Backend as Message Handler
User->>ChatTextArea: Type message
User->>Agent: Clicks button (e.g., Retry or Send)
alt Message is sent (text or images exist)
Agent->>Backend: Send message
Backend-->>Agent: Response
Agent->>ChatTextArea: Clear input and images
else Button click without sending message
Agent-->>ChatTextArea: Preserve input and images
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
webview-ui/src/components/chat/ChatView.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
a755932 to
b9e7ecf
Compare
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.
Pull Request Overview
This PR fixes a user experience issue where the chat input field was being cleared prematurely when clicking "yes" or "no" buttons in the agent chat interface. The fix ensures that input clearing only occurs when a message is actually sent, preserving user input if an action fails or requires further interaction.
Key changes:
- Moved input clearing logic inside the conditional blocks that handle successful message sending
- Removed parameters from button click handlers as they no longer need to pass input values directly
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| webview-ui/src/components/chat/ChatView.tsx | Relocated input clearing logic to execute only after successful message sending, and simplified button click handler signatures |
| .changeset/calm-pears-fry.md | Added changeset documentation describing the bug fix |
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: 0
♻️ Duplicate comments (1)
webview-ui/src/components/chat/ChatView.tsx (1)
1912-1912: Correctly addresses past review feedback with improved design.The previous review comment suggested passing
inputValueandselectedImagesas parameters, but the current implementation uses a better approach by accessing these values directly from the closure. This decouples the button clicks from parameter passing and ensures the handlers always work with current state values, which aligns perfectly with the new input clearing logic.
🧹 Nitpick comments (1)
webview-ui/src/components/chat/ChatView.tsx (1)
661-663: Add required kilocode_change comments per coding guidelines.The changed lines need kilocode_change comments as specified in the coding guidelines:
Lines 661-663 and 724-726 (multi-line changes):
+ // kilocode_change start // Clear input state after sending setInputValue("") setSelectedImages([]) + // kilocode_change endLines 1912 and 1934 (single line changes):
- onClick={() => handlePrimaryButtonClick()}> + onClick={() => handlePrimaryButtonClick()}> // kilocode_changeAlso applies to: 724-726, 1912-1912, 1934-1934
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/calm-pears-fry.md(1 hunks)webview-ui/src/components/chat/ChatView.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/!(*kilocode*)/**/*.{ts,tsx,js,jsx,html,css,scss}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/!(*kilocode*)/**/*.{ts,tsx,js,jsx,html,css,scss}: For single line changes, add the comment '// kilocode_change' at the end of the line.
For multiple consecutive lines changed, wrap them with '// kilocode_change start' and '// kilocode_change end' comments.
Files:
webview-ui/src/components/chat/ChatView.tsx
**/!(*kilocode*)/**/*.{html,jsx,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
In HTML/JSX/TSX files, use '{/* kilocode_change start /}' and '{/ kilocode_change end */}' to wrap multi-line changes.
Files:
webview-ui/src/components/chat/ChatView.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: storybook-playwright-snapshot
- GitHub Check: test-extension (windows-latest)
- GitHub Check: test-webview (windows-latest)
🔇 Additional comments (3)
webview-ui/src/components/chat/ChatView.tsx (2)
661-663: LGTM! Input clearing logic properly relocated.The input clearing now occurs only after successfully sending a message, which prevents premature clearing when buttons are clicked but ensures the input is cleared when a message is actually dispatched. This correctly implements the fix described in the PR objectives.
724-726: LGTM! Consistent implementation for secondary button.The secondary button handler now follows the same pattern as the primary button, clearing the input state only after successfully sending a message. This ensures consistent behavior across both button types.
.changeset/calm-pears-fry.md (1)
1-8: LGTM! Changeset accurately documents the fix.The changeset clearly describes the behavior change: preventing premature input clearing on button clicks while ensuring clearing occurs only when messages are actually sent. This documentation aligns perfectly with the implemented changes and PR objectives.
Previously, we had a bug where if either of the buttons in the agent chat was clicked, the ChatTextArea would get cleared. Now, the text box will only get cleared if the message is sent as part of the response.
Moved input clearing logic to only execute when a message is successfully sent, preventing the input field from clearing prematurely when clicking "yes" or "no" buttons in the chat interface. This improves user experience by retaining the input content if an action fails or requires further interaction
Fixes #1520
Opened the same PR on the Roo side: RooCodeInc/Roo-Code#6222
Summary by CodeRabbit