From bfdd1581292bba434e65f23b8d9b203ea25aa051 Mon Sep 17 00:00:00 2001 From: Daniel <57051444+daniel-lxs@users.noreply.github.com> Date: Thu, 14 Aug 2025 18:48:25 -0500 Subject: [PATCH] Fix: Enable save button for provider dropdown and checkbox changes (#7113) * fix: enable save button for provider dropdown and checkbox changes The save button wasn't enabling when users changed provider settings like dropdowns and checkboxes because setApiConfigurationField was treating all changes from undefined to a defined value as 'initial sync' and not marking the form as dirty. Added an optional isUserAction parameter (defaults to true) to distinguish: - User actions (should enable save button) - the default - Automatic initialization (shouldn't enable save button) - pass false This fixes the issue where changing provider dropdowns, checkboxes with default values, and other settings wouldn't enable the save button. * fix: remove incorrect isUserAction=false from apiModelId sync The useEffect that syncs apiModelId with selectedModelId was incorrectly passing isUserAction=false, which prevented the save button from enabling when users selected a different model. Since this effect responds to all selectedModelId changes (including user selections), it should use the default isUserAction=true behavior. * test: fix ThinkingBudget test to expect isUserAction parameter The test now correctly expects setApiConfigurationField to be called with three arguments including the isUserAction=false parameter for automatic thinking token adjustments. --- webview-ui/src/components/settings/ApiOptions.tsx | 8 ++++++-- webview-ui/src/components/settings/ModelPicker.tsx | 8 ++++++-- webview-ui/src/components/settings/SettingsView.tsx | 8 ++++---- webview-ui/src/components/settings/ThinkingBudget.tsx | 10 +++++++--- .../settings/__tests__/ThinkingBudget.spec.tsx | 2 +- .../src/components/settings/providers/HuggingFace.tsx | 8 ++++++-- .../src/components/settings/providers/Unbound.tsx | 8 ++++++-- 7 files changed, 36 insertions(+), 16 deletions(-) diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index dcdf072a11..b51b171354 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -105,7 +105,11 @@ import { buildDocLink } from "@src/utils/docLinks" export interface ApiOptionsProps { uriScheme: string | undefined apiConfiguration: ProviderSettings - setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void + setApiConfigurationField: ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, + ) => void fromWelcomeView?: boolean errorMessage: string | undefined setErrorMessage: React.Dispatch> @@ -280,7 +284,7 @@ const ApiOptions = ({ const shouldSetDefault = !modelId if (shouldSetDefault) { - setApiConfigurationField(field, defaultValue) + setApiConfigurationField(field, defaultValue, false) } } diff --git a/webview-ui/src/components/settings/ModelPicker.tsx b/webview-ui/src/components/settings/ModelPicker.tsx index 2160d4b3c5..0753f9fc2b 100644 --- a/webview-ui/src/components/settings/ModelPicker.tsx +++ b/webview-ui/src/components/settings/ModelPicker.tsx @@ -46,7 +46,11 @@ interface ModelPickerProps { serviceName: string serviceUrl: string apiConfiguration: ProviderSettings - setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void + setApiConfigurationField: ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, + ) => void organizationAllowList: OrganizationAllowList errorMessage?: string } @@ -124,7 +128,7 @@ export const ModelPicker = ({ useEffect(() => { if (!selectedModelId && !isInitialized.current) { const initialValue = modelIds.includes(selectedModelId) ? selectedModelId : defaultModelId - setApiConfigurationField(modelIdKey, initialValue) + setApiConfigurationField(modelIdKey, initialValue, false) // false = automatic initialization } isInitialized.current = true diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 987d245105..9866c1e235 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -219,7 +219,7 @@ const SettingsView = forwardRef(({ onDone, t }, []) const setApiConfigurationField = useCallback( - (field: K, value: ProviderSettings[K]) => { + (field: K, value: ProviderSettings[K], isUserAction: boolean = true) => { setCachedState((prevState) => { if (prevState.apiConfiguration?.[field] === value) { return prevState @@ -227,9 +227,9 @@ const SettingsView = forwardRef(({ onDone, t const previousValue = prevState.apiConfiguration?.[field] - // Don't treat initial sync from undefined to a defined value as a user change - // This prevents the dirty state when the component initializes and auto-syncs the model ID - const isInitialSync = previousValue === undefined && value !== undefined + // Only skip change detection for automatic initialization (not user actions) + // This prevents the dirty state when the component initializes and auto-syncs values + const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined if (!isInitialSync) { setChangeDetected(true) diff --git a/webview-ui/src/components/settings/ThinkingBudget.tsx b/webview-ui/src/components/settings/ThinkingBudget.tsx index c2aa234983..7a85e61e7a 100644 --- a/webview-ui/src/components/settings/ThinkingBudget.tsx +++ b/webview-ui/src/components/settings/ThinkingBudget.tsx @@ -20,7 +20,11 @@ import { useSelectedModel } from "@src/components/ui/hooks/useSelectedModel" interface ThinkingBudgetProps { apiConfiguration: ProviderSettings - setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void + setApiConfigurationField: ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, + ) => void modelInfo?: ModelInfo } @@ -71,7 +75,7 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod // Set default reasoning effort when model supports it and no value is set useEffect(() => { if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort && defaultReasoningEffort) { - setApiConfigurationField("reasoningEffort", defaultReasoningEffort) + setApiConfigurationField("reasoningEffort", defaultReasoningEffort, false) } }, [isReasoningEffortSupported, apiConfiguration.reasoningEffort, defaultReasoningEffort, setApiConfigurationField]) @@ -91,7 +95,7 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod // appropriately. useEffect(() => { if (isReasoningBudgetSupported && customMaxThinkingTokens > modelMaxThinkingTokens) { - setApiConfigurationField("modelMaxThinkingTokens", modelMaxThinkingTokens) + setApiConfigurationField("modelMaxThinkingTokens", modelMaxThinkingTokens, false) } }, [isReasoningBudgetSupported, customMaxThinkingTokens, modelMaxThinkingTokens, setApiConfigurationField]) diff --git a/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx b/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx index fa7493edc6..9bc235e5f0 100644 --- a/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx @@ -112,7 +112,7 @@ describe("ThinkingBudget", () => { ) // Effect should trigger and cap the value - expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxThinkingTokens", 8000) // 80% of 10000 + expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxThinkingTokens", 8000, false) // 80% of 10000 }) it("should use default thinking tokens if not provided", () => { diff --git a/webview-ui/src/components/settings/providers/HuggingFace.tsx b/webview-ui/src/components/settings/providers/HuggingFace.tsx index 8716739d80..94f60cc4c8 100644 --- a/webview-ui/src/components/settings/providers/HuggingFace.tsx +++ b/webview-ui/src/components/settings/providers/HuggingFace.tsx @@ -34,7 +34,11 @@ type HuggingFaceModel = { type HuggingFaceProps = { apiConfiguration: ProviderSettings - setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void + setApiConfigurationField: ( + field: keyof ProviderSettings, + value: ProviderSettings[keyof ProviderSettings], + isUserAction?: boolean, + ) => void } export const HuggingFace = ({ apiConfiguration, setApiConfigurationField }: HuggingFaceProps) => { @@ -93,7 +97,7 @@ export const HuggingFace = ({ apiConfiguration, setApiConfigurationField }: Hugg // Set to "auto" as default const defaultProvider = "auto" setSelectedProvider(defaultProvider) - setApiConfigurationField("huggingFaceInferenceProvider", defaultProvider) + setApiConfigurationField("huggingFaceInferenceProvider", defaultProvider, false) // false = automatic default } } } diff --git a/webview-ui/src/components/settings/providers/Unbound.tsx b/webview-ui/src/components/settings/providers/Unbound.tsx index a242ee4b3f..2dd105a663 100644 --- a/webview-ui/src/components/settings/providers/Unbound.tsx +++ b/webview-ui/src/components/settings/providers/Unbound.tsx @@ -17,7 +17,11 @@ import { ModelPicker } from "../ModelPicker" type UnboundProps = { apiConfiguration: ProviderSettings - setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void + setApiConfigurationField: ( + field: keyof ProviderSettings, + value: ProviderSettings[keyof ProviderSettings], + isUserAction?: boolean, + ) => void routerModels?: RouterModels organizationAllowList: OrganizationAllowList modelValidationError?: string @@ -110,7 +114,7 @@ export const Unbound = ({ if (!currentModelId || !modelExists) { const firstAvailableModelId = Object.keys(updatedModels)[0] - setApiConfigurationField("unboundModelId", firstAvailableModelId) + setApiConfigurationField("unboundModelId", firstAvailableModelId, false) // false = automatic model selection } }