Skip to content

Commit 5362597

Browse files
committed
refactor: reduce code duplication and add comprehensive test coverage
- Extract checkApiRequestInProgress helper to reduce duplication - Add isCancelPending state to prevent rapid cancel clicks - Add comprehensive test suite for cancel button behavior - Test coverage for API request state tracking - Test proper button state during tool approval dialogs - Fix React Hook dependency warning
1 parent 28940f6 commit 5362597

File tree

2 files changed

+551
-54
lines changed

2 files changed

+551
-54
lines changed

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

Lines changed: 37 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
176176
const textAreaRef = useRef<HTMLTextAreaElement>(null)
177177
const [sendingDisabled, setSendingDisabled] = useState(false)
178178
const [selectedImages, setSelectedImages] = useState<string[]>([])
179+
const [isCancelPending, setIsCancelPending] = useState(false)
179180

180181
// 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)
181182
const [clineAsk, setClineAsk] = useState<ClineAsk | undefined>(undefined)
@@ -514,6 +515,32 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
514515
prevExpandedRowsRef.current = expandedRows // Store current state for next comparison
515516
}, [expandedRows])
516517

518+
// Helper function to check if an API request is actively in progress
519+
const checkApiRequestInProgress = useCallback((messages: ClineMessage[]) => {
520+
const isLastMessagePartial = messages.at(-1)?.partial === true
521+
522+
if (isLastMessagePartial) {
523+
return true
524+
}
525+
526+
const lastApiReqStarted = findLast(messages, (message: ClineMessage) => message.say === "api_req_started")
527+
528+
if (
529+
lastApiReqStarted &&
530+
lastApiReqStarted.text !== null &&
531+
lastApiReqStarted.text !== undefined &&
532+
lastApiReqStarted.say === "api_req_started"
533+
) {
534+
const cost = JSON.parse(lastApiReqStarted.text).cost
535+
536+
if (cost === undefined) {
537+
return true // API request has not finished yet.
538+
}
539+
}
540+
541+
return false
542+
}, [])
543+
517544
const isStreaming = useMemo(() => {
518545
// Checking clineAsk isn't enough since messages effect may be called
519546
// again for a tool for example, set clineAsk to its value, and if the
@@ -531,63 +558,15 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
531558
return false
532559
}
533560

534-
const isLastMessagePartial = modifiedMessages.at(-1)?.partial === true
535-
536-
if (isLastMessagePartial) {
537-
return true
538-
} else {
539-
const lastApiReqStarted = findLast(
540-
modifiedMessages,
541-
(message: ClineMessage) => message.say === "api_req_started",
542-
)
543-
544-
if (
545-
lastApiReqStarted &&
546-
lastApiReqStarted.text !== null &&
547-
lastApiReqStarted.text !== undefined &&
548-
lastApiReqStarted.say === "api_req_started"
549-
) {
550-
const cost = JSON.parse(lastApiReqStarted.text).cost
551-
552-
if (cost === undefined) {
553-
return true // API request has not finished yet.
554-
}
555-
}
556-
}
557-
558-
return false
559-
}, [modifiedMessages, clineAsk, enableButtons, primaryButtonText])
561+
return checkApiRequestInProgress(modifiedMessages)
562+
}, [modifiedMessages, clineAsk, enableButtons, primaryButtonText, checkApiRequestInProgress])
560563

561564
// Track the actual streaming state for the cancel button separately.
562565
// This ensures the cancel button remains enabled during API requests,
563566
// even when there's a tool approval dialog.
564567
const isApiRequestInProgress = useMemo(() => {
565-
const isLastMessagePartial = modifiedMessages.at(-1)?.partial === true
566-
567-
if (isLastMessagePartial) {
568-
return true
569-
}
570-
571-
const lastApiReqStarted = findLast(
572-
modifiedMessages,
573-
(message: ClineMessage) => message.say === "api_req_started",
574-
)
575-
576-
if (
577-
lastApiReqStarted &&
578-
lastApiReqStarted.text !== null &&
579-
lastApiReqStarted.text !== undefined &&
580-
lastApiReqStarted.say === "api_req_started"
581-
) {
582-
const cost = JSON.parse(lastApiReqStarted.text).cost
583-
584-
if (cost === undefined) {
585-
return true // API request has not finished yet.
586-
}
587-
}
588-
589-
return false
590-
}, [modifiedMessages])
568+
return checkApiRequestInProgress(modifiedMessages)
569+
}, [modifiedMessages, checkApiRequestInProgress])
591570

592571
const markFollowUpAsAnswered = useCallback(() => {
593572
const lastFollowUpMessage = messagesRef.current.findLast((msg: ClineMessage) => msg.ask === "followup")
@@ -764,9 +743,13 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
764743

765744
const trimmedInput = text?.trim()
766745

767-
if (isStreaming || isApiRequestInProgress) {
746+
if ((isStreaming || isApiRequestInProgress) && !isCancelPending) {
747+
// Prevent rapid clicks by setting pending state
748+
setIsCancelPending(true)
768749
vscode.postMessage({ type: "cancelTask" })
769750
setDidClickCancel(true)
751+
// Reset pending state after a short delay
752+
setTimeout(() => setIsCancelPending(false), 1000)
770753
return
771754
}
772755

@@ -804,7 +787,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
804787
setClineAsk(undefined)
805788
setEnableButtons(false)
806789
},
807-
[clineAsk, startNewTask, isStreaming, isApiRequestInProgress],
790+
[clineAsk, startNewTask, isStreaming, isApiRequestInProgress, isCancelPending],
808791
)
809792

810793
const { info: model } = useSelectedModel(apiConfiguration)

0 commit comments

Comments
 (0)