Skip to content

Commit 91c53b9

Browse files
hannesrudolphhannesrudolph
committed
refactor: address code review feedback for error display
- Extract shared CollapsibleErrorSection component to eliminate code duplication - Use Tailwind CSS classes instead of inline styles for consistency - Add proper accessibility attributes (role, tabIndex, aria-expanded, keyboard handlers) - Auto-detect language for error content (xml for error tags, text otherwise) - Maintain feature parity between error and diff_error displays
1 parent 37a65c4 commit 91c53b9

File tree

2 files changed

+91
-115
lines changed

2 files changed

+91
-115
lines changed

‎webview-ui/src/components/chat/ChatRow.tsx‎

Lines changed: 18 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { McpExecution } from "./McpExecution"
44
import { useSize } from "react-use"
55
import { useTranslation, Trans } from "react-i18next"
66
import deepEqual from "fast-deep-equal"
7-
import { VSCodeBadge, VSCodeButton } from "@vscode/webview-ui-toolkit/react"
7+
import { VSCodeBadge } from "@vscode/webview-ui-toolkit/react"
88

99
import type { ClineMessage } from "@roo-code/types"
1010
import { Mode } from "@roo/modes"
@@ -14,7 +14,6 @@ import { COMMAND_OUTPUT_STRING } from "@roo/combineCommandSequences"
1414
import { safeJsonParse } from "@roo/safeJsonParse"
1515
import { FollowUpData, SuggestionItem } from "@roo-code/types"
1616

17-
import { useCopyToClipboard } from "@src/utils/clipboard"
1817
import { useExtensionState } from "@src/context/ExtensionStateContext"
1918
import { findMatchingResourceOrTemplate } from "@src/utils/mcp"
2019
import { vscode } from "@src/utils/vscode"
@@ -28,7 +27,6 @@ import { MAX_IMAGES_PER_MESSAGE } from "./ChatView"
2827
import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock"
2928
import UpdateTodoListToolBlock from "./UpdateTodoListToolBlock"
3029
import CodeAccordian from "../common/CodeAccordian"
31-
import CodeBlock from "../common/CodeBlock"
3230
import MarkdownBlock from "../common/MarkdownBlock"
3331
import { ReasoningBlock } from "./ReasoningBlock"
3432
import Thumbnails from "../common/Thumbnails"
@@ -46,6 +44,7 @@ import { CommandExecutionError } from "./CommandExecutionError"
4644
import { AutoApprovedRequestLimitWarning } from "./AutoApprovedRequestLimitWarning"
4745
import { CondenseContextErrorRow, CondensingContextRow, ContextCondenseRow } from "./ContextCondenseRow"
4846
import CodebaseSearchResultsDisplay from "./CodebaseSearchResultsDisplay"
47+
import { CollapsibleErrorSection } from "./CollapsibleErrorSection"
4948

5049
interface ChatRowProps {
5150
message: ClineMessage
@@ -117,14 +116,11 @@ export const ChatRowContent = ({
117116
const { mcpServers, alwaysAllowMcp, currentCheckpoint, mode } = useExtensionState()
118117
const [reasoningCollapsed, setReasoningCollapsed] = useState(true)
119118
const [isDiffErrorExpanded, setIsDiffErrorExpanded] = useState(false)
120-
const [showCopySuccess, setShowCopySuccess] = useState(false)
121119
const [isErrorExpanded, setIsErrorExpanded] = useState(false) // Default collapsed like diff_error
122-
const [showErrorCopySuccess, setShowErrorCopySuccess] = useState(false)
123120
const [isEditing, setIsEditing] = useState(false)
124121
const [editedContent, setEditedContent] = useState("")
125122
const [editMode, setEditMode] = useState<Mode>(mode || "code")
126123
const [editImages, setEditImages] = useState<string[]>([])
127-
const { copyWithFeedback } = useCopyToClipboard()
128124

129125
// Handle message events for image selection during edit mode
130126
useEffect(() => {
@@ -856,63 +852,13 @@ export const ChatRowContent = ({
856852
switch (message.say) {
857853
case "diff_error":
858854
return (
859-
<div>
860-
<div className="mt-0 overflow-hidden mb-2">
861-
<div
862-
className={`${
863-
isDiffErrorExpanded ? "border-b border-vscode-editorGroup-border" : ""
864-
} font-normal text-base text-vscode-editor-foreground flex items-center justify-between cursor-pointer focus:outline focus:outline-2 focus:outline-vscode-focusBorder`}
865-
role="button"
866-
tabIndex={0}
867-
aria-expanded={isDiffErrorExpanded}
868-
aria-label={t("chat:diffError.title")}
869-
onClick={() => setIsDiffErrorExpanded(!isDiffErrorExpanded)}
870-
onKeyDown={(e) => {
871-
if (e.key === "Enter" || e.key === " ") {
872-
e.preventDefault()
873-
setIsDiffErrorExpanded(!isDiffErrorExpanded)
874-
}
875-
}}>
876-
<div className="flex items-center gap-2.5 flex-grow">
877-
<span
878-
className="codicon codicon-warning text-vscode-editorWarning-foreground opacity-80"
879-
style={{ fontSize: 16, marginBottom: "-1.5px" }}></span>
880-
<span className="font-bold">{t("chat:diffError.title")}</span>
881-
</div>
882-
<div className="flex items-center">
883-
<VSCodeButton
884-
appearance="icon"
885-
className="p-[3px] h-6 mr-1 text-vscode-editor-foreground flex items-center justify-center bg-transparent"
886-
onClick={(e) => {
887-
e.stopPropagation()
888-
889-
// Call copyWithFeedback and handle the Promise
890-
copyWithFeedback(message.text || "").then((success) => {
891-
if (success) {
892-
// Show checkmark
893-
setShowCopySuccess(true)
894-
895-
// Reset after a brief delay
896-
setTimeout(() => {
897-
setShowCopySuccess(false)
898-
}, 1000)
899-
}
900-
})
901-
}}>
902-
<span
903-
className={`codicon codicon-${showCopySuccess ? "check" : "copy"}`}></span>
904-
</VSCodeButton>
905-
<span
906-
className={`codicon codicon-chevron-${isDiffErrorExpanded ? "up" : "down"}`}></span>
907-
</div>
908-
</div>
909-
{isDiffErrorExpanded && (
910-
<div className="p-2 bg-vscode-editor-background">
911-
<CodeBlock source={message.text || ""} language="xml" />
912-
</div>
913-
)}
914-
</div>
915-
</div>
855+
<CollapsibleErrorSection
856+
title={t("chat:diffError.title")}
857+
content={message.text}
858+
language="xml"
859+
isExpanded={isDiffErrorExpanded}
860+
onToggleExpand={() => setIsDiffErrorExpanded(!isDiffErrorExpanded)}
861+
/>
916862
)
917863
case "subtask_result":
918864
return (
@@ -1104,59 +1050,16 @@ export const ChatRowContent = ({
11041050
</div>
11051051
)
11061052
case "error":
1053+
// Detect language based on content - check if it contains XML-like error tags
1054+
const errorLanguage = message.text?.includes("<error>") ? "xml" : "text"
11071055
return (
1108-
<div>
1109-
<div className="mt-0 overflow-hidden mb-2">
1110-
<div
1111-
className={`${
1112-
isErrorExpanded ? "border-b border-vscode-editorGroup-border" : ""
1113-
} font-normal text-base text-vscode-editor-foreground flex items-center justify-between cursor-pointer focus:outline focus:outline-2 focus:outline-vscode-focusBorder`}
1114-
role="button"
1115-
tabIndex={0}
1116-
aria-expanded={isErrorExpanded}
1117-
aria-label={message.title || t("chat:error")}
1118-
onClick={() => setIsErrorExpanded(!isErrorExpanded)}
1119-
onKeyDown={(e) => {
1120-
if (e.key === "Enter" || e.key === " ") {
1121-
e.preventDefault()
1122-
setIsErrorExpanded(!isErrorExpanded)
1123-
}
1124-
}}>
1125-
<div className="flex items-center gap-2.5 flex-grow">
1126-
<span
1127-
className="codicon codicon-warning text-vscode-editorWarning-foreground opacity-80"
1128-
style={{ fontSize: 16, marginBottom: "-1.5px" }}></span>
1129-
<span className="font-bold">{message.title || t("chat:error")}</span>
1130-
</div>
1131-
<div className="flex items-center">
1132-
<VSCodeButton
1133-
appearance="icon"
1134-
className="p-[3px] h-6 mr-1 text-vscode-editor-foreground flex items-center justify-center bg-transparent"
1135-
onClick={(e) => {
1136-
e.stopPropagation()
1137-
copyWithFeedback(message.text || "").then((success) => {
1138-
if (success) {
1139-
setShowErrorCopySuccess(true)
1140-
setTimeout(() => {
1141-
setShowErrorCopySuccess(false)
1142-
}, 1000)
1143-
}
1144-
})
1145-
}}>
1146-
<span
1147-
className={`codicon codicon-${showErrorCopySuccess ? "check" : "copy"}`}></span>
1148-
</VSCodeButton>
1149-
<span
1150-
className={`codicon codicon-chevron-${isErrorExpanded ? "up" : "down"}`}></span>
1151-
</div>
1152-
</div>
1153-
{isErrorExpanded && (
1154-
<div className="p-2 bg-vscode-editor-background">
1155-
<CodeBlock source={message.text || ""} language="xml" />
1156-
</div>
1157-
)}
1158-
</div>
1159-
</div>
1056+
<CollapsibleErrorSection
1057+
title={message.title || t("chat:error")}
1058+
content={message.text}
1059+
language={errorLanguage}
1060+
isExpanded={isErrorExpanded}
1061+
onToggleExpand={() => setIsErrorExpanded(!isErrorExpanded)}
1062+
/>
11601063
)
11611064
case "completion_result":
11621065
return (

‎webview-ui/src/components/chat/CollapsibleErrorSection.tsx‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import React from "react"
2+
import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"
3+
import CodeBlock from "../common/CodeBlock"
4+
import { useCopyToClipboard } from "@src/utils/clipboard"
5+
6+
interface CollapsibleErrorSectionProps {
7+
title: string
8+
content: string | null | undefined
9+
language?: string
10+
isExpanded: boolean
11+
onToggleExpand: () => void
12+
}
13+
14+
export const CollapsibleErrorSection: React.FC<CollapsibleErrorSectionProps> = ({
15+
title,
16+
content,
17+
language = "xml",
18+
isExpanded,
19+
onToggleExpand,
20+
}) => {
21+
const [showCopySuccess, setShowCopySuccess] = React.useState(false)
22+
const { copyWithFeedback } = useCopyToClipboard()
23+
24+
const handleCopy = async (e: React.MouseEvent) => {
25+
e.stopPropagation()
26+
const success = await copyWithFeedback(content || "")
27+
if (success) {
28+
setShowCopySuccess(true)
29+
setTimeout(() => setShowCopySuccess(false), 1000)
30+
}
31+
}
32+
33+
const handleKeyDown = (e: React.KeyboardEvent) => {
34+
if (e.key === "Enter" || e.key === " ") {
35+
e.preventDefault()
36+
onToggleExpand()
37+
}
38+
}
39+
40+
return (
41+
<div className="mt-0 overflow-hidden mb-2">
42+
<div
43+
className={`${
44+
isExpanded ? "border-b border-vscode-editorGroup-border" : ""
45+
} font-normal text-base text-vscode-editor-foreground flex items-center justify-between cursor-pointer focus:outline focus:outline-2 focus:outline-vscode-focusBorder`}
46+
role="button"
47+
tabIndex={0}
48+
aria-expanded={isExpanded}
49+
aria-label={title}
50+
onClick={onToggleExpand}
51+
onKeyDown={handleKeyDown}>
52+
<div className="flex items-center gap-2.5 flex-grow">
53+
<span className="codicon codicon-warning text-vscode-editorWarning-foreground opacity-80 text-base -mb-0.5"></span>
54+
<span className="font-bold">{title}</span>
55+
</div>
56+
<div className="flex items-center">
57+
<VSCodeButton
58+
appearance="icon"
59+
className="p-[3px] h-6 mr-1 text-vscode-editor-foreground flex items-center justify-center bg-transparent"
60+
onClick={handleCopy}>
61+
<span className={`codicon codicon-${showCopySuccess ? "check" : "copy"}`}></span>
62+
</VSCodeButton>
63+
<span className={`codicon codicon-chevron-${isExpanded ? "up" : "down"}`}></span>
64+
</div>
65+
</div>
66+
{isExpanded && (
67+
<div className="p-2 bg-vscode-editor-background">
68+
<CodeBlock source={content || ""} language={language} />
69+
</div>
70+
)}
71+
</div>
72+
)
73+
}

0 commit comments

Comments
 (0)