-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: improve error display with custom titles and expandable UI #7794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
47c21fb
eef6c03
7e65f6f
b34dc7c
daaf458
45a877d
b5d5268
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,113 @@ | ||||||
| # Implementation Plan: Improve Error Display in Webview | ||||||
|
|
||||||
| ## Overview | ||||||
|
|
||||||
| Currently, `cline.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with: | ||||||
|
||||||
| Currently, `cline.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with: | |
| Currently, `this.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with: |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1169,10 +1169,33 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |
| `Roo tried to use ${toolName}${ | ||
| relPath ? ` for '${relPath.toPosix()}'` : "" | ||
| } without value for required parameter '${paramName}'. Retrying...`, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| { | ||
| metadata: { | ||
| title: "Missing Parameter Error", | ||
| }, | ||
| }, | ||
| ) | ||
| return formatResponse.toolError(formatResponse.missingToolParameterError(paramName)) | ||
| } | ||
|
|
||
| /** | ||
| * Helper method to say an error with a custom title | ||
| * @param title - The title to display for the error | ||
| * @param text - The error message text | ||
| * @param images - Optional images to include | ||
| */ | ||
| async sayError(title: string, text: string, images?: string[]) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The helper method is a nice addition! Consider adding a JSDoc example to make it more discoverable: |
||
| await this.say("error", text, images, undefined, undefined, undefined, { | ||
| metadata: { | ||
| title, | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| // Lifecycle | ||
| // Start / Resume / Abort / Dispose | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -120,7 +120,9 @@ export const ChatRowContent = ({ | |||||
| const { info: model } = useSelectedModel(apiConfiguration) | ||||||
| const [reasoningCollapsed, setReasoningCollapsed] = useState(true) | ||||||
| const [isDiffErrorExpanded, setIsDiffErrorExpanded] = useState(false) | ||||||
| const [isErrorExpanded, setIsErrorExpanded] = useState(false) | ||||||
| const [showCopySuccess, setShowCopySuccess] = useState(false) | ||||||
| const [showErrorCopySuccess, setShowErrorCopySuccess] = useState(false) | ||||||
| const [isEditing, setIsEditing] = useState(false) | ||||||
| const [editedContent, setEditedContent] = useState("") | ||||||
| const [editMode, setEditMode] = useState<Mode>(mode || "code") | ||||||
|
|
@@ -1243,16 +1245,99 @@ export const ChatRowContent = ({ | |||||
| </div> | ||||||
| ) | ||||||
| case "error": | ||||||
| // Extract custom title from metadata if available | ||||||
| const errorTitle = (message as any).metadata?.title || t("chat:error") | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The expandable error UI implementation is effective—extracting a custom error title from message metadata and showing error details upon expansion improves user experience. Consider refactoring repeated inline styles into reusable CSS classes or style objects, and replacing the use of 'any' with proper TypeScript types for message metadata.
Suggested change
|
||||||
|
|
||||||
| return ( | ||||||
| <> | ||||||
| {title && ( | ||||||
| <div style={headerStyle}> | ||||||
| {icon} | ||||||
| {title} | ||||||
| <div> | ||||||
| <div | ||||||
| style={{ | ||||||
| marginTop: "0px", | ||||||
| overflow: "hidden", | ||||||
| marginBottom: "8px", | ||||||
| }}> | ||||||
| <div | ||||||
| style={{ | ||||||
| borderBottom: isErrorExpanded | ||||||
| ? "1px solid var(--vscode-editorGroup-border)" | ||||||
| : "none", | ||||||
| fontWeight: "normal", | ||||||
| fontSize: "var(--vscode-font-size)", | ||||||
| color: "var(--vscode-editor-foreground)", | ||||||
| display: "flex", | ||||||
| alignItems: "center", | ||||||
| justifyContent: "space-between", | ||||||
| cursor: "pointer", | ||||||
| }} | ||||||
| onClick={() => setIsErrorExpanded(!isErrorExpanded)}> | ||||||
| <div | ||||||
| style={{ | ||||||
| display: "flex", | ||||||
| alignItems: "center", | ||||||
| gap: "10px", | ||||||
| flexGrow: 1, | ||||||
| }}> | ||||||
| <span | ||||||
| className="codicon codicon-error" | ||||||
| style={{ | ||||||
| color: "var(--vscode-errorForeground)", | ||||||
| fontSize: 16, | ||||||
| marginBottom: "-1.5px", | ||||||
| }}></span> | ||||||
| <span style={{ fontWeight: "bold", color: "var(--vscode-errorForeground)" }}> | ||||||
| {errorTitle} | ||||||
| </span> | ||||||
| </div> | ||||||
| <div style={{ display: "flex", alignItems: "center" }}> | ||||||
| <VSCodeButton | ||||||
| appearance="icon" | ||||||
| style={{ | ||||||
| padding: "3px", | ||||||
| height: "24px", | ||||||
| marginRight: "4px", | ||||||
| color: "var(--vscode-editor-foreground)", | ||||||
| display: "flex", | ||||||
| alignItems: "center", | ||||||
| justifyContent: "center", | ||||||
| background: "transparent", | ||||||
| }} | ||||||
| onClick={(e) => { | ||||||
| e.stopPropagation() | ||||||
|
|
||||||
| // Call copyWithFeedback and handle the Promise | ||||||
| copyWithFeedback(message.text || "").then((success) => { | ||||||
| if (success) { | ||||||
| // Show checkmark | ||||||
| setShowErrorCopySuccess(true) | ||||||
|
|
||||||
| // Reset after a brief delay | ||||||
| setTimeout(() => { | ||||||
| setShowErrorCopySuccess(false) | ||||||
| }, 1000) | ||||||
| } | ||||||
| }) | ||||||
| }}> | ||||||
| <span | ||||||
| className={`codicon codicon-${showErrorCopySuccess ? "check" : "copy"}`}></span> | ||||||
| </VSCodeButton> | ||||||
| <span | ||||||
| className={`codicon codicon-chevron-${isErrorExpanded ? "up" : "down"}`}></span> | ||||||
| </div> | ||||||
| </div> | ||||||
| )} | ||||||
| <p style={{ ...pStyle, color: "var(--vscode-errorForeground)" }}>{message.text}</p> | ||||||
| </> | ||||||
| {isErrorExpanded && ( | ||||||
| <div | ||||||
| style={{ | ||||||
| padding: "8px", | ||||||
| backgroundColor: "var(--vscode-editor-background)", | ||||||
| borderTop: "none", | ||||||
| }}> | ||||||
| <p style={{ ...pStyle, color: "var(--vscode-errorForeground)", margin: 0 }}> | ||||||
| {message.text} | ||||||
| </p> | ||||||
| </div> | ||||||
| )} | ||||||
| </div> | ||||||
| </div> | ||||||
| ) | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider extracting this expandable error UI into a reusable component. The error display logic here (lines 1252-1341) is very similar to the diff_error display above (lines 964-1050). Creating a shared component could reduce code duplication and make the UI more maintainable. |
||||||
| case "completion_result": | ||||||
| return ( | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This planning document was helpful during development but could be removed from the final PR to keep the repository clean. Consider moving it to a GitHub issue or discussion if you want to preserve the implementation details.