Improved error message feedback in settings panel

System233 · System233 · commit 1a3b8700ba2c · 2025-02-26T06:59:23.000+08:00
diff --git a/webview-ui/src/__mocks__/vscrui.ts b/webview-ui/src/__mocks__/vscrui.ts
@@ -8,6 +8,9 @@ export const Dropdown = ({ children, value, onChange }: any) =>
 
 export const Pane = ({ children }: any) => React.createElement("div", { "data-testid": "mock-pane" }, children)
 
+export const Button = ({ children, ...props }: any) =>
+	React.createElement("div", { "data-testid": "mock-button", ...props }, children)
+
 export type DropdownOption = {
 	label: string
 	value: string
diff --git a/webview-ui/src/components/settings/ApiErrorMessage.tsx b/webview-ui/src/components/settings/ApiErrorMessage.tsx
@@ -0,0 +1,16 @@
+import React from "react"
+
+interface ApiErrorMessageProps {
+	errorMessage: string | undefined
+	children?: React.ReactNode
+}
+const ApiErrorMessage = ({ errorMessage, children }: ApiErrorMessageProps) => {
+	return (
+		<div className="text-vscode-errorForeground text-sm">
+			<span style={{ fontSize: "2em" }} className={`codicon codicon-close align-middle mr-1`} />
+			{errorMessage}
+			{children}
+		</div>
+	)
+}
+export default ApiErrorMessage
diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx
@@ -1,4 +1,4 @@
-import { memo, useCallback, useMemo, useState } from "react"
+import React, { memo, useCallback, useEffect, useMemo, useState } from "react"
 import { useDebounce, useEvent } from "react-use"
 import { Checkbox, Dropdown, Pane, type DropdownOption } from "vscrui"
 import { VSCodeLink, VSCodeRadio, VSCodeRadioGroup, VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
@@ -42,23 +42,25 @@ import { ModelInfoView } from "./ModelInfoView"
 import { DROPDOWN_Z_INDEX } from "./styles"
 import { ModelPicker } from "./ModelPicker"
 import { TemperatureControl } from "./TemperatureControl"
+import { validateApiConfiguration, validateModelId } from "@/utils/validate"
+import ApiErrorMessage from "./ApiErrorMessage"
 
 interface ApiOptionsProps {
 	uriScheme: string | undefined
 	apiConfiguration: ApiConfiguration
 	setApiConfigurationField: <K extends keyof ApiConfiguration>(field: K, value: ApiConfiguration[K]) => void
-	apiErrorMessage?: string
-	modelIdErrorMessage?: string
 	fromWelcomeView?: boolean
+	errorMessage: string | undefined
+	setErrorMessage: React.Dispatch<React.SetStateAction<string | undefined>>
 }
 
 const ApiOptions = ({
 	uriScheme,
 	apiConfiguration,
 	setApiConfigurationField,
-	apiErrorMessage,
-	modelIdErrorMessage,
 	fromWelcomeView,
+	errorMessage,
+	setErrorMessage,
 }: ApiOptionsProps) => {
 	const [ollamaModels, setOllamaModels] = useState<string[]>([])
 	const [lmStudioModels, setLmStudioModels] = useState<string[]>([])
@@ -146,6 +148,13 @@ const ApiOptions = ({
 		],
 	)
 
+	useEffect(() => {
+		const apiValidationResult =
+			validateApiConfiguration(apiConfiguration) ||
+			validateModelId(apiConfiguration, glamaModels, openRouterModels, unboundModels)
+		setErrorMessage(apiValidationResult)
+	}, [apiConfiguration, glamaModels, openRouterModels, setErrorMessage, unboundModels])
+
 	const handleMessage = useCallback((event: MessageEvent) => {
 		const message: ExtensionMessage = event.data
 		switch (message.type) {
@@ -626,6 +635,7 @@ const ApiOptions = ({
 							]}
 						/>
 					</div>
+					{errorMessage && <ApiErrorMessage errorMessage={errorMessage} />}
 					<p
 						style={{
 							fontSize: "12px",
@@ -705,6 +715,7 @@ const ApiOptions = ({
 						models={openAiModels}
 						setApiConfigurationField={setApiConfigurationField}
 						defaultModelInfo={openAiModelInfoSaneDefaults}
+						errorMessage={errorMessage}
 					/>
 					<div style={{ display: "flex", alignItems: "center" }}>
 						<Checkbox
@@ -1068,18 +1079,6 @@ const ApiOptions = ({
 					/>
 
 					{/* end Model Info Configuration */}
-
-					<p
-						style={{
-							fontSize: "12px",
-							marginTop: 3,
-							color: "var(--vscode-descriptionForeground)",
-						}}>
-						<span style={{ color: "var(--vscode-errorForeground)" }}>
-							(<span style={{ fontWeight: 500 }}>Note:</span> Roo Code uses complex prompts and works best
-							with Claude models. Less capable models may not work as expected.)
-						</span>
-					</p>
 				</div>
 			)}
 
@@ -1100,6 +1099,7 @@ const ApiOptions = ({
 						placeholder={"e.g. meta-llama-3.1-8b-instruct"}>
 						<span style={{ fontWeight: 500 }}>Model ID</span>
 					</VSCodeTextField>
+					{errorMessage && <ApiErrorMessage errorMessage={errorMessage} />}
 
 					{lmStudioModels.length > 0 && (
 						<VSCodeRadioGroup
@@ -1245,6 +1245,12 @@ const ApiOptions = ({
 						placeholder={"e.g. llama3.1"}>
 						<span style={{ fontWeight: 500 }}>Model ID</span>
 					</VSCodeTextField>
+					{errorMessage && (
+						<div className="text-vscode-errorForeground text-sm">
+							<span style={{ fontSize: "2em" }} className={`codicon codicon-close align-middle mr-1`} />
+							{errorMessage}
+						</div>
+					)}
 					{ollamaModels.length > 0 && (
 						<VSCodeRadioGroup
 							value={
@@ -1321,22 +1327,11 @@ const ApiOptions = ({
 						serviceUrl="https://api.getunbound.ai/models"
 						recommendedModel={unboundDefaultModelId}
 						setApiConfigurationField={setApiConfigurationField}
+						errorMessage={errorMessage}
 					/>
 				</div>
 			)}
 
-			{apiErrorMessage && (
-				<p
-					style={{
-						margin: "-10px 0 4px 0",
-						fontSize: 12,
-						color: "var(--vscode-errorForeground)",
-					}}>
-					<span style={{ fontSize: "2em" }} className={`codicon codicon-close align-middle mr-1`} />
-					{apiErrorMessage}
-				</p>
-			)}
-
 			{selectedProvider === "glama" && (
 				<ModelPicker
 					apiConfiguration={apiConfiguration ?? {}}
@@ -1349,6 +1344,7 @@ const ApiOptions = ({
 					serviceUrl="https://glama.ai/models"
 					recommendedModel="anthropic/claude-3-7-sonnet"
 					setApiConfigurationField={setApiConfigurationField}
+					errorMessage={errorMessage}
 				/>
 			)}
 
@@ -1364,6 +1360,7 @@ const ApiOptions = ({
 					serviceName="OpenRouter"
 					serviceUrl="https://openrouter.ai/models"
 					recommendedModel="anthropic/claude-3.7-sonnet"
+					errorMessage={errorMessage}
 				/>
 			)}
 			{selectedProvider === "requesty" && (
@@ -1378,6 +1375,7 @@ const ApiOptions = ({
 					serviceName="Requesty"
 					serviceUrl="https://requesty.ai"
 					recommendedModel="anthropic/claude-3-7-sonnet-latest"
+					errorMessage={errorMessage}
 				/>
 			)}
 
@@ -1401,6 +1399,7 @@ const ApiOptions = ({
 							{selectedProvider === "deepseek" && createDropdown(deepSeekModels)}
 							{selectedProvider === "mistral" && createDropdown(mistralModels)}
 						</div>
+						{errorMessage && <ApiErrorMessage errorMessage={errorMessage} />}
 						<ModelInfoView
 							selectedModelId={selectedModelId}
 							modelInfo={selectedModelInfo}
@@ -1448,18 +1447,6 @@ const ApiOptions = ({
 					/>
 				</div>
 			)}
-
-			{modelIdErrorMessage && (
-				<p
-					style={{
-						margin: "-10px 0 4px 0",
-						fontSize: 12,
-						color: "var(--vscode-errorForeground)",
-					}}>
-					<span style={{ fontSize: "2em" }} className={`codicon codicon-close align-middle mr-1`} />
-					{modelIdErrorMessage}
-				</p>
-			)}
 		</div>
 	)
 }
diff --git a/webview-ui/src/components/settings/ModelPicker.tsx b/webview-ui/src/components/settings/ModelPicker.tsx
@@ -5,6 +5,7 @@ import { normalizeApiConfiguration } from "./ApiOptions"
 import { ModelInfoView } from "./ModelInfoView"
 import { ApiConfiguration, ModelInfo } from "../../../../src/shared/api"
 import { Combobox, ComboboxContent, ComboboxEmpty, ComboboxInput, ComboboxItem } from "../ui/combobox"
+import ApiErrorMessage from "./ApiErrorMessage"
 
 type ExtractType<T> = NonNullable<
 	{ [K in keyof ApiConfiguration]: Required<ApiConfiguration>[K] extends T ? K : never }[keyof ApiConfiguration]
@@ -30,6 +31,7 @@ interface ModelPickerProps {
 	apiConfiguration: ApiConfiguration
 	setApiConfigurationField: <K extends keyof ApiConfiguration>(field: K, value: ApiConfiguration[K]) => void
 	defaultModelInfo?: ModelInfo
+	errorMessage?: string
 }
 
 export const ModelPicker = ({
@@ -43,6 +45,7 @@ export const ModelPicker = ({
 	apiConfiguration,
 	setApiConfigurationField,
 	defaultModelInfo,
+	errorMessage,
 }: ModelPickerProps) => {
 	const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false)
 
@@ -69,11 +72,16 @@ export const ModelPicker = ({
 	return (
 		<>
 			<div className="font-semibold">Model</div>
-			<Combobox type="single" inputValue={apiConfiguration[modelIdKey]} onInputValueChange={onSelect}>
+			<Combobox
+				style={errorMessage ? { "--color-vscode-dropdown-border": "var(--color-vscode-errorForeground)" } : {}}
+				type="single"
+				inputValue={apiConfiguration[modelIdKey]}
+				onInputValueChange={onSelect}>
 				<ComboboxInput
 					className="border-vscode-errorForeground tefat"
 					placeholder="Search model..."
 					data-testid="model-input"
+					aria-errormessage={errorMessage}
 				/>
 				<ComboboxContent>
 					<ComboboxEmpty>No model found.</ComboboxEmpty>
@@ -85,13 +93,30 @@ export const ModelPicker = ({
 				</ComboboxContent>
 			</Combobox>
 
-			{selectedModelId && selectedModelInfo && (
-				<ModelInfoView
-					selectedModelId={selectedModelId}
-					modelInfo={selectedModelInfo}
-					isDescriptionExpanded={isDescriptionExpanded}
-					setIsDescriptionExpanded={setIsDescriptionExpanded}
-				/>
+			{errorMessage ? (
+				<ApiErrorMessage errorMessage={errorMessage}>
+					<p
+						style={{
+							fontSize: "12px",
+							marginTop: 3,
+							color: "var(--vscode-descriptionForeground)",
+						}}>
+						<span style={{ color: "var(--vscode-errorForeground)" }}>
+							<span style={{ fontWeight: 500 }}>Note:</span> Roo Code uses complex prompts and works best
+							with Claude models. Less capable models may not work as expected.
+						</span>
+					</p>
+				</ApiErrorMessage>
+			) : (
+				selectedModelId &&
+				selectedModelInfo && (
+					<ModelInfoView
+						selectedModelId={selectedModelId}
+						modelInfo={selectedModelInfo}
+						isDescriptionExpanded={isDescriptionExpanded}
+						setIsDescriptionExpanded={setIsDescriptionExpanded}
+					/>
+				)
 			)}
 			<p>
 				The extension automatically fetches the latest list of models available on{" "}
diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx
@@ -1,6 +1,6 @@
 import { forwardRef, memo, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from "react"
 import { VSCodeButton, VSCodeCheckbox, VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
-import { Dropdown, type DropdownOption } from "vscrui"
+import { Button, Dropdown, type DropdownOption } from "vscrui"
 
 import {
 	AlertDialog,
@@ -14,7 +14,6 @@ import {
 } from "@/components/ui"
 
 import { vscode } from "../../utils/vscode"
-import { validateApiConfiguration, validateModelId } from "../../utils/validate"
 import { ExtensionStateContextType, useExtensionState } from "../../context/ExtensionStateContext"
 import { EXPERIMENT_IDS, experimentConfigsMap, ExperimentId } from "../../../../src/shared/experiments"
 import { ApiConfiguration } from "../../../../src/shared/api"
@@ -33,14 +32,13 @@ export interface SettingsViewRef {
 
 const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone }, ref) => {
 	const extensionState = useExtensionState()
-	const [apiErrorMessage, setApiErrorMessage] = useState<string | undefined>(undefined)
-	const [modelIdErrorMessage, setModelIdErrorMessage] = useState<string | undefined>(undefined)
 	const [commandInput, setCommandInput] = useState("")
 	const [isDiscardDialogShow, setDiscardDialogShow] = useState(false)
 	const [cachedState, setCachedState] = useState(extensionState)
 	const [isChangeDetected, setChangeDetected] = useState(false)
 	const prevApiConfigName = useRef(extensionState.currentApiConfigName)
 	const confirmDialogHandler = useRef<() => void>()
+	const [errorMessage, setErrorMessage] = useState<string | undefined>(undefined)
 
 	// TODO: Reduce WebviewMessage/ExtensionState complexity
 	const { currentApiConfigName } = extensionState
@@ -135,20 +133,9 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone },
 			}
 		})
 	}, [])
-
+	const isSettingValid = !errorMessage
 	const handleSubmit = () => {
-		const apiValidationResult = validateApiConfiguration(apiConfiguration)
-
-		const modelIdValidationResult = validateModelId(
-			apiConfiguration,
-			extensionState.glamaModels,
-			extensionState.openRouterModels,
-		)
-
-		setApiErrorMessage(apiValidationResult)
-		setModelIdErrorMessage(modelIdValidationResult)
-
-		if (!apiValidationResult && !modelIdValidationResult) {
+		if (isSettingValid) {
 			vscode.postMessage({ type: "alwaysAllowReadOnly", bool: alwaysAllowReadOnly })
 			vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite })
 			vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute })
@@ -177,23 +164,6 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone },
 		}
 	}
 
-	useEffect(() => {
-		setApiErrorMessage(undefined)
-		setModelIdErrorMessage(undefined)
-	}, [apiConfiguration])
-
-	// Initial validation on mount
-	useEffect(() => {
-		const apiValidationResult = validateApiConfiguration(apiConfiguration)
-		const modelIdValidationResult = validateModelId(
-			apiConfiguration,
-			extensionState.glamaModels,
-			extensionState.openRouterModels,
-		)
-		setApiErrorMessage(apiValidationResult)
-		setModelIdErrorMessage(modelIdValidationResult)
-	}, [apiConfiguration, extensionState.glamaModels, extensionState.openRouterModels])
-
 	const checkUnsaveChanges = useCallback(
 		(then: () => void) => {
 			if (isChangeDetected) {
@@ -287,13 +257,14 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone },
 						justifyContent: "space-between",
 						gap: "6px",
 					}}>
-					<VSCodeButton
-						appearance="primary"
-						title={isChangeDetected ? "Save changes" : "Nothing changed"}
+					<Button
+						appearance={isSettingValid ? "primary" : "secondary"}
+						className={!isSettingValid ? "!border-vscode-errorForeground" : ""}
+						title={!isSettingValid ? errorMessage : isChangeDetected ? "Save changes" : "Nothing changed"}
 						onClick={handleSubmit}
-						disabled={!isChangeDetected}>
+						disabled={!isChangeDetected || !isSettingValid}>
 						Save
-					</VSCodeButton>
+					</Button>
 					<VSCodeButton
 						appearance="secondary"
 						title="Discard unsaved changes and close settings panel"
@@ -344,8 +315,8 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone },
 							uriScheme={extensionState.uriScheme}
 							apiConfiguration={apiConfiguration}
 							setApiConfigurationField={setApiConfigurationField}
-							apiErrorMessage={apiErrorMessage}
-							modelIdErrorMessage={modelIdErrorMessage}
+							errorMessage={errorMessage}
+							setErrorMessage={setErrorMessage}
 						/>
 					</div>
 				</div>
diff --git a/webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx b/webview-ui/src/components/settings/__tests__/ApiOptions.test.tsx
diff --git a/webview-ui/src/components/welcome/WelcomeView.tsx b/webview-ui/src/components/welcome/WelcomeView.tsx
