From 44e4feecb8a924a45cc39228cb95f868331a5497 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 25 Aug 2025 17:20:03 -0600 Subject: [PATCH] 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 --- webview-ui/src/components/chat/ChatRow.tsx | 133 +++--------------- .../chat/CollapsibleErrorSection.tsx | 73 ++++++++++ 2 files changed, 91 insertions(+), 115 deletions(-) create mode 100644 webview-ui/src/components/chat/CollapsibleErrorSection.tsx diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 30533fc9d6..c666257705 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -2,7 +2,7 @@ import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from " import { useSize } from "react-use" import { useTranslation, Trans } from "react-i18next" import deepEqual from "fast-deep-equal" -import { VSCodeBadge, VSCodeButton } from "@vscode/webview-ui-toolkit/react" +import { VSCodeBadge } from "@vscode/webview-ui-toolkit/react" import type { ClineMessage, FollowUpData, SuggestionItem } from "@roo-code/types" import { Mode } from "@roo/modes" @@ -11,7 +11,6 @@ import { ClineApiReqInfo, ClineAskUseMcpServer, ClineSayTool } from "@roo/Extens import { COMMAND_OUTPUT_STRING } from "@roo/combineCommandSequences" import { safeJsonParse } from "@roo/safeJsonParse" -import { useCopyToClipboard } from "@src/utils/clipboard" import { useExtensionState } from "@src/context/ExtensionStateContext" import { findMatchingResourceOrTemplate } from "@src/utils/mcp" import { vscode } from "@src/utils/vscode" @@ -22,7 +21,6 @@ import { Button } from "@src/components/ui" import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock" import UpdateTodoListToolBlock from "./UpdateTodoListToolBlock" import CodeAccordian from "../common/CodeAccordian" -import CodeBlock from "../common/CodeBlock" import MarkdownBlock from "../common/MarkdownBlock" import { ReasoningBlock } from "./ReasoningBlock" import Thumbnails from "../common/Thumbnails" @@ -47,6 +45,7 @@ import { McpExecution } from "./McpExecution" import { ChatTextArea } from "./ChatTextArea" import { MAX_IMAGES_PER_MESSAGE } from "./ChatView" import { useSelectedModel } from "../ui/hooks/useSelectedModel" +import { CollapsibleErrorSection } from "./CollapsibleErrorSection" interface ChatRowProps { message: ClineMessage @@ -120,14 +119,11 @@ export const ChatRowContent = ({ const { info: model } = useSelectedModel(apiConfiguration) const [reasoningCollapsed, setReasoningCollapsed] = useState(true) const [isDiffErrorExpanded, setIsDiffErrorExpanded] = useState(false) - const [showCopySuccess, setShowCopySuccess] = useState(false) const [isEditing, setIsEditing] = useState(false) const [editedContent, setEditedContent] = useState("") const [editMode, setEditMode] = useState(mode || "code") const [editImages, setEditImages] = useState([]) const [isErrorExpanded, setIsErrorExpanded] = useState(false) // Default collapsed like diff_error - const [showErrorCopySuccess, setShowErrorCopySuccess] = useState(false) - const { copyWithFeedback } = useCopyToClipboard() // Handle message events for image selection during edit mode useEffect(() => { @@ -961,63 +957,13 @@ export const ChatRowContent = ({ switch (message.say) { case "diff_error": return ( -
-
-
setIsDiffErrorExpanded(!isDiffErrorExpanded)} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault() - setIsDiffErrorExpanded(!isDiffErrorExpanded) - } - }}> -
- - {t("chat:diffError.title")} -
-
- { - e.stopPropagation() - - // Call copyWithFeedback and handle the Promise - copyWithFeedback(message.text || "").then((success) => { - if (success) { - // Show checkmark - setShowCopySuccess(true) - - // Reset after a brief delay - setTimeout(() => { - setShowCopySuccess(false) - }, 1000) - } - }) - }}> - - - -
-
- {isDiffErrorExpanded && ( -
- -
- )} -
-
+ setIsDiffErrorExpanded(!isDiffErrorExpanded)} + /> ) case "subtask_result": return ( @@ -1216,59 +1162,16 @@ export const ChatRowContent = ({ ) case "error": + // Detect language based on content - check if it contains XML-like error tags + const errorLanguage = message.text?.includes("") ? "xml" : "text" return ( -
-
-
setIsErrorExpanded(!isErrorExpanded)} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault() - setIsErrorExpanded(!isErrorExpanded) - } - }}> -
- - {message.title || t("chat:error")} -
-
- { - e.stopPropagation() - copyWithFeedback(message.text || "").then((success) => { - if (success) { - setShowErrorCopySuccess(true) - setTimeout(() => { - setShowErrorCopySuccess(false) - }, 1000) - } - }) - }}> - - - -
-
- {isErrorExpanded && ( -
- -
- )} -
-
+ setIsErrorExpanded(!isErrorExpanded)} + /> ) case "completion_result": return ( diff --git a/webview-ui/src/components/chat/CollapsibleErrorSection.tsx b/webview-ui/src/components/chat/CollapsibleErrorSection.tsx new file mode 100644 index 0000000000..3ff5af524c --- /dev/null +++ b/webview-ui/src/components/chat/CollapsibleErrorSection.tsx @@ -0,0 +1,73 @@ +import React from "react" +import { VSCodeButton } from "@vscode/webview-ui-toolkit/react" +import CodeBlock from "../common/CodeBlock" +import { useCopyToClipboard } from "@src/utils/clipboard" + +interface CollapsibleErrorSectionProps { + title: string + content: string | null | undefined + language?: string + isExpanded: boolean + onToggleExpand: () => void +} + +export const CollapsibleErrorSection: React.FC = ({ + title, + content, + language = "xml", + isExpanded, + onToggleExpand, +}) => { + const [showCopySuccess, setShowCopySuccess] = React.useState(false) + const { copyWithFeedback } = useCopyToClipboard() + + const handleCopy = async (e: React.MouseEvent) => { + e.stopPropagation() + const success = await copyWithFeedback(content || "") + if (success) { + setShowCopySuccess(true) + setTimeout(() => setShowCopySuccess(false), 1000) + } + } + + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault() + onToggleExpand() + } + } + + return ( +
+
+
+ + {title} +
+
+ + + + +
+
+ {isExpanded && ( +
+ +
+ )} +
+ ) +}