Skip to content

Commit 2d8ee79

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 e4d8fde commit 2d8ee79

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
@@ -2,15 +2,14 @@ import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from "
22
import { useSize } from "react-use"
33
import { useTranslation, Trans } from "react-i18next"
44
import deepEqual from "fast-deep-equal"
5-
import { VSCodeBadge, VSCodeButton } from "@vscode/webview-ui-toolkit/react"
5+
import { VSCodeBadge } from "@vscode/webview-ui-toolkit/react"
66

77
import type { ClineMessage, FollowUpData, SuggestionItem } from "@roo-code/types"
88

99
import { ClineApiReqInfo, ClineAskUseMcpServer, ClineSayTool } from "@roo/ExtensionMessage"
1010
import { COMMAND_OUTPUT_STRING } from "@roo/combineCommandSequences"
1111
import { safeJsonParse } from "@roo/safeJsonParse"
1212

13-
import { useCopyToClipboard } from "@src/utils/clipboard"
1413
import { useExtensionState } from "@src/context/ExtensionStateContext"
1514
import { findMatchingResourceOrTemplate } from "@src/utils/mcp"
1615
import { vscode } from "@src/utils/vscode"
@@ -21,7 +20,6 @@ import { Button } from "@src/components/ui"
2120
import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock"
2221
import UpdateTodoListToolBlock from "./UpdateTodoListToolBlock"
2322
import CodeAccordian from "../common/CodeAccordian"
24-
import CodeBlock from "../common/CodeBlock"
2523
import MarkdownBlock from "../common/MarkdownBlock"
2624
import { ReasoningBlock } from "./ReasoningBlock"
2725
import Thumbnails from "../common/Thumbnails"
@@ -42,6 +40,7 @@ import { AutoApprovedRequestLimitWarning } from "./AutoApprovedRequestLimitWarni
4240
import { CondenseContextErrorRow, CondensingContextRow, ContextCondenseRow } from "./ContextCondenseRow"
4341
import CodebaseSearchResultsDisplay from "./CodebaseSearchResultsDisplay"
4442
import { McpExecution } from "./McpExecution"
43+
import { CollapsibleErrorSection } from "./CollapsibleErrorSection"
4544

4645
interface ChatRowProps {
4746
message: ClineMessage
@@ -115,10 +114,7 @@ export const ChatRowContent = ({
115114

116115
const [reasoningCollapsed, setReasoningCollapsed] = useState(true)
117116
const [isDiffErrorExpanded, setIsDiffErrorExpanded] = useState(false)
118-
const [showCopySuccess, setShowCopySuccess] = useState(false)
119117
const [isErrorExpanded, setIsErrorExpanded] = useState(false) // Default collapsed like diff_error
120-
const [showErrorCopySuccess, setShowErrorCopySuccess] = useState(false)
121-
const { copyWithFeedback } = useCopyToClipboard()
122118

123119
// Memoized callback to prevent re-renders caused by inline arrow functions.
124120
const handleToggleExpand = useCallback(() => {
@@ -904,63 +900,13 @@ export const ChatRowContent = ({
904900
switch (message.say) {
905901
case "diff_error":
906902
return (
907-
<div>
908-
<div className="mt-0 overflow-hidden mb-2">
909-
<div
910-
className={`${
911-
isDiffErrorExpanded ? "border-b border-vscode-editorGroup-border" : ""
912-
} font-normal text-base text-vscode-editor-foreground flex items-center justify-between cursor-pointer focus:outline focus:outline-2 focus:outline-vscode-focusBorder`}
913-
role="button"
914-
tabIndex={0}
915-
aria-expanded={isDiffErrorExpanded}
916-
aria-label={t("chat:diffError.title")}
917-
onClick={() => setIsDiffErrorExpanded(!isDiffErrorExpanded)}
918-
onKeyDown={(e) => {
919-
if (e.key === "Enter" || e.key === " ") {
920-
e.preventDefault()
921-
setIsDiffErrorExpanded(!isDiffErrorExpanded)
922-
}
923-
}}>
924-
<div className="flex items-center gap-2.5 flex-grow">
925-
<span
926-
className="codicon codicon-warning text-vscode-editorWarning-foreground opacity-80"
927-
style={{ fontSize: 16, marginBottom: "-1.5px" }}></span>
928-
<span className="font-bold">{t("chat:diffError.title")}</span>
929-
</div>
930-
<div className="flex items-center">
931-
<VSCodeButton
932-
appearance="icon"
933-
className="p-[3px] h-6 mr-1 text-vscode-editor-foreground flex items-center justify-center bg-transparent"
934-
onClick={(e) => {
935-
e.stopPropagation()
936-
937-
// Call copyWithFeedback and handle the Promise
938-
copyWithFeedback(message.text || "").then((success) => {
939-
if (success) {
940-
// Show checkmark
941-
setShowCopySuccess(true)
942-
943-
// Reset after a brief delay
944-
setTimeout(() => {
945-
setShowCopySuccess(false)
946-
}, 1000)
947-
}
948-
})
949-
}}>
950-
<span
951-
className={`codicon codicon-${showCopySuccess ? "check" : "copy"}`}></span>
952-
</VSCodeButton>
953-
<span
954-
className={`codicon codicon-chevron-${isDiffErrorExpanded ? "up" : "down"}`}></span>
955-
</div>
956-
</div>
957-
{isDiffErrorExpanded && (
958-
<div className="p-2 bg-vscode-editor-background">
959-
<CodeBlock source={message.text || ""} language="xml" />
960-
</div>
961-
)}
962-
</div>
963-
</div>
903+
<CollapsibleErrorSection
904+
title={t("chat:diffError.title")}
905+
content={message.text}
906+
language="xml"
907+
isExpanded={isDiffErrorExpanded}
908+
onToggleExpand={() => setIsDiffErrorExpanded(!isDiffErrorExpanded)}
909+
/>
964910
)
965911
case "subtask_result":
966912
return (
@@ -1127,59 +1073,16 @@ export const ChatRowContent = ({
11271073
</div>
11281074
)
11291075
case "error":
1076+
// Detect language based on content - check if it contains XML-like error tags
1077+
const errorLanguage = message.text?.includes("<error>") ? "xml" : "text"
11301078
return (
1131-
<div>
1132-
<div className="mt-0 overflow-hidden mb-2">
1133-
<div
1134-
className={`${
1135-
isErrorExpanded ? "border-b border-vscode-editorGroup-border" : ""
1136-
} font-normal text-base text-vscode-editor-foreground flex items-center justify-between cursor-pointer focus:outline focus:outline-2 focus:outline-vscode-focusBorder`}
1137-
role="button"
1138-
tabIndex={0}
1139-
aria-expanded={isErrorExpanded}
1140-
aria-label={message.title || t("chat:error")}
1141-
onClick={() => setIsErrorExpanded(!isErrorExpanded)}
1142-
onKeyDown={(e) => {
1143-
if (e.key === "Enter" || e.key === " ") {
1144-
e.preventDefault()
1145-
setIsErrorExpanded(!isErrorExpanded)
1146-
}
1147-
}}>
1148-
<div className="flex items-center gap-2.5 flex-grow">
1149-
<span
1150-
className="codicon codicon-warning text-vscode-editorWarning-foreground opacity-80"
1151-
style={{ fontSize: 16, marginBottom: "-1.5px" }}></span>
1152-
<span className="font-bold">{message.title || t("chat:error")}</span>
1153-
</div>
1154-
<div className="flex items-center">
1155-
<VSCodeButton
1156-
appearance="icon"
1157-
className="p-[3px] h-6 mr-1 text-vscode-editor-foreground flex items-center justify-center bg-transparent"
1158-
onClick={(e) => {
1159-
e.stopPropagation()
1160-
copyWithFeedback(message.text || "").then((success) => {
1161-
if (success) {
1162-
setShowErrorCopySuccess(true)
1163-
setTimeout(() => {
1164-
setShowErrorCopySuccess(false)
1165-
}, 1000)
1166-
}
1167-
})
1168-
}}>
1169-
<span
1170-
className={`codicon codicon-${showErrorCopySuccess ? "check" : "copy"}`}></span>
1171-
</VSCodeButton>
1172-
<span
1173-
className={`codicon codicon-chevron-${isErrorExpanded ? "up" : "down"}`}></span>
1174-
</div>
1175-
</div>
1176-
{isErrorExpanded && (
1177-
<div className="p-2 bg-vscode-editor-background">
1178-
<CodeBlock source={message.text || ""} language="xml" />
1179-
</div>
1180-
)}
1181-
</div>
1182-
</div>
1079+
<CollapsibleErrorSection
1080+
title={message.title || t("chat:error")}
1081+
content={message.text}
1082+
language={errorLanguage}
1083+
isExpanded={isErrorExpanded}
1084+
onToggleExpand={() => setIsErrorExpanded(!isErrorExpanded)}
1085+
/>
11831086
)
11841087
case "completion_result":
11851088
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)