Skip to content

Commit 3cf1116

Browse files
committed
fix: address PR review feedback for message queueing feature
- Restore original ChatView tests from main branch - Fix broken test by updating ChatTextArea mock - Add comprehensive tests for message queueing (simplified due to mocking constraints) - Fix race condition using useRef and setTimeout in queue processing - Extract QueuedMessage interface to shared types.ts file - Replace inline styles with Tailwind classes in QueuedMessages - Add i18n support for 'Queued Messages:' text - Add keyboard navigation for removing queued messages - Add JSDoc for fromQueue parameter in handleSendMessage
1 parent c02a115 commit 3cf1116

File tree

4 files changed

+1484
-96
lines changed

4 files changed

+1484
-96
lines changed

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,7 @@ import ProfileViolationWarning from "./ProfileViolationWarning"
5656
import { CheckpointWarning } from "./CheckpointWarning"
5757
import QueuedMessages from "./QueuedMessages"
5858
import { getLatestTodo } from "@roo/todo"
59-
60-
interface QueuedMessage {
61-
text: string
62-
images: string[]
63-
}
59+
import { QueuedMessage } from "./types"
6460

6561
export interface ChatViewProps {
6662
isHidden: boolean
@@ -161,6 +157,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
161157
const [sendingDisabled, setSendingDisabled] = useState(false)
162158
const [selectedImages, setSelectedImages] = useState<string[]>([])
163159
const [messageQueue, setMessageQueue] = useState<QueuedMessage[]>([])
160+
const isProcessingQueueRef = useRef(false)
164161

165162
// we need to hold on to the ask because useEffect > lastMessage will always let us know when an ask comes in and handle it, but by the time handleMessage is called, the last message might not be the ask anymore (it could be a say that followed)
166163
const [clineAsk, setClineAsk] = useState<ClineAsk | undefined>(undefined)
@@ -545,6 +542,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
545542
disableAutoScrollRef.current = false
546543
}, [])
547544

545+
/**
546+
* Handles sending messages to the extension
547+
* @param text - The message text to send
548+
* @param images - Array of image data URLs to send with the message
549+
* @param fromQueue - Internal flag indicating if this message is being sent from the queue (prevents re-queueing)
550+
*/
548551
const handleSendMessage = useCallback(
549552
(text: string, images: string[], fromQueue = false) => {
550553
text = text.trim()
@@ -596,10 +599,16 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
596599
)
597600

598601
useEffect(() => {
599-
if (!sendingDisabled && messageQueue.length > 0) {
602+
if (!sendingDisabled && messageQueue.length > 0 && !isProcessingQueueRef.current) {
603+
isProcessingQueueRef.current = true
600604
const nextMessage = messageQueue[0]
601-
handleSendMessage(nextMessage.text, nextMessage.images, true)
602-
setMessageQueue((prev) => prev.slice(1))
605+
606+
// Use setTimeout to ensure state updates are processed
607+
setTimeout(() => {
608+
handleSendMessage(nextMessage.text, nextMessage.images, true)
609+
setMessageQueue((prev) => prev.slice(1))
610+
isProcessingQueueRef.current = false
611+
}, 0)
603612
}
604613
}, [sendingDisabled, messageQueue, handleSendMessage])
605614

webview-ui/src/components/chat/QueuedMessages.tsx

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,46 @@
11
import React from "react"
22
import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"
3+
import { useTranslation } from "react-i18next"
34
import Thumbnails from "../common/Thumbnails"
4-
5-
interface QueuedMessage {
6-
text: string
7-
images: string[]
8-
}
5+
import { QueuedMessage } from "./types"
96

107
interface QueuedMessagesProps {
118
queue: QueuedMessage[]
129
onRemove: (index: number) => void
1310
}
1411

1512
const QueuedMessages: React.FC<QueuedMessagesProps> = ({ queue, onRemove }) => {
13+
const { t } = useTranslation("chat")
14+
1615
if (queue.length === 0) {
1716
return null
1817
}
1918

2019
return (
21-
<div className="p-2 border-t border-vscode-panel-border">
22-
<div className="text-vscode-descriptionForeground mb-2">Queued Messages:</div>
20+
<div className="p-2 border-t border-vscode-panel-border" data-testid="queued-messages">
21+
<div className="text-vscode-descriptionForeground mb-2">{t("queuedMessages.title")}</div>
2322
<div className="flex flex-col gap-2">
2423
{queue.map((message, index) => (
2524
<div key={index} className="flex items-center gap-2 p-2 rounded-md bg-vscode-input-background">
2625
<div className="flex-grow">
2726
<p className="text-vscode-input-foreground">{message.text}</p>
2827
{message.images.length > 0 && (
29-
<Thumbnails images={message.images} style={{ marginTop: "8px" }} />
28+
<div className="mt-2">
29+
<Thumbnails images={message.images} />
30+
</div>
3031
)}
3132
</div>
32-
<VSCodeButton appearance="icon" aria-label="Remove message" onClick={() => onRemove(index)}>
33+
<VSCodeButton
34+
appearance="icon"
35+
aria-label={t("queuedMessages.removeMessage")}
36+
onClick={() => onRemove(index)}
37+
onKeyDown={(e: React.KeyboardEvent) => {
38+
if (e.key === "Enter" || e.key === " ") {
39+
e.preventDefault()
40+
onRemove(index)
41+
}
42+
}}
43+
tabIndex={0}>
3344
<span className="codicon codicon-close"></span>
3445
</VSCodeButton>
3546
</div>

0 commit comments

Comments
 (0)