Skip to content

Commit bfdd158

Browse files
authored
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.
1 parent 3f4af18 commit bfdd158

File tree

7 files changed

+36
-16
lines changed

7 files changed

+36
-16
lines changed

webview-ui/src/components/settings/ApiOptions.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,11 @@ import { buildDocLink } from "@src/utils/docLinks"
105105
export interface ApiOptionsProps {
106106
uriScheme: string | undefined
107107
apiConfiguration: ProviderSettings
108-
setApiConfigurationField: <K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => void
108+
setApiConfigurationField: <K extends keyof ProviderSettings>(
109+
field: K,
110+
value: ProviderSettings[K],
111+
isUserAction?: boolean,
112+
) => void
109113
fromWelcomeView?: boolean
110114
errorMessage: string | undefined
111115
setErrorMessage: React.Dispatch<React.SetStateAction<string | undefined>>
@@ -280,7 +284,7 @@ const ApiOptions = ({
280284
const shouldSetDefault = !modelId
281285

282286
if (shouldSetDefault) {
283-
setApiConfigurationField(field, defaultValue)
287+
setApiConfigurationField(field, defaultValue, false)
284288
}
285289
}
286290

webview-ui/src/components/settings/ModelPicker.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@ interface ModelPickerProps {
4646
serviceName: string
4747
serviceUrl: string
4848
apiConfiguration: ProviderSettings
49-
setApiConfigurationField: <K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => void
49+
setApiConfigurationField: <K extends keyof ProviderSettings>(
50+
field: K,
51+
value: ProviderSettings[K],
52+
isUserAction?: boolean,
53+
) => void
5054
organizationAllowList: OrganizationAllowList
5155
errorMessage?: string
5256
}
@@ -124,7 +128,7 @@ export const ModelPicker = ({
124128
useEffect(() => {
125129
if (!selectedModelId && !isInitialized.current) {
126130
const initialValue = modelIds.includes(selectedModelId) ? selectedModelId : defaultModelId
127-
setApiConfigurationField(modelIdKey, initialValue)
131+
setApiConfigurationField(modelIdKey, initialValue, false) // false = automatic initialization
128132
}
129133

130134
isInitialized.current = true

webview-ui/src/components/settings/SettingsView.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,17 +219,17 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
219219
}, [])
220220

221221
const setApiConfigurationField = useCallback(
222-
<K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => {
222+
<K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K], isUserAction: boolean = true) => {
223223
setCachedState((prevState) => {
224224
if (prevState.apiConfiguration?.[field] === value) {
225225
return prevState
226226
}
227227

228228
const previousValue = prevState.apiConfiguration?.[field]
229229

230-
// Don't treat initial sync from undefined to a defined value as a user change
231-
// This prevents the dirty state when the component initializes and auto-syncs the model ID
232-
const isInitialSync = previousValue === undefined && value !== undefined
230+
// Only skip change detection for automatic initialization (not user actions)
231+
// This prevents the dirty state when the component initializes and auto-syncs values
232+
const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined
233233

234234
if (!isInitialSync) {
235235
setChangeDetected(true)

webview-ui/src/components/settings/ThinkingBudget.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ import { useSelectedModel } from "@src/components/ui/hooks/useSelectedModel"
2020

2121
interface ThinkingBudgetProps {
2222
apiConfiguration: ProviderSettings
23-
setApiConfigurationField: <K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => void
23+
setApiConfigurationField: <K extends keyof ProviderSettings>(
24+
field: K,
25+
value: ProviderSettings[K],
26+
isUserAction?: boolean,
27+
) => void
2428
modelInfo?: ModelInfo
2529
}
2630

@@ -71,7 +75,7 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
7175
// Set default reasoning effort when model supports it and no value is set
7276
useEffect(() => {
7377
if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort && defaultReasoningEffort) {
74-
setApiConfigurationField("reasoningEffort", defaultReasoningEffort)
78+
setApiConfigurationField("reasoningEffort", defaultReasoningEffort, false)
7579
}
7680
}, [isReasoningEffortSupported, apiConfiguration.reasoningEffort, defaultReasoningEffort, setApiConfigurationField])
7781

@@ -91,7 +95,7 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
9195
// appropriately.
9296
useEffect(() => {
9397
if (isReasoningBudgetSupported && customMaxThinkingTokens > modelMaxThinkingTokens) {
94-
setApiConfigurationField("modelMaxThinkingTokens", modelMaxThinkingTokens)
98+
setApiConfigurationField("modelMaxThinkingTokens", modelMaxThinkingTokens, false)
9599
}
96100
}, [isReasoningBudgetSupported, customMaxThinkingTokens, modelMaxThinkingTokens, setApiConfigurationField])
97101

webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe("ThinkingBudget", () => {
112112
)
113113

114114
// Effect should trigger and cap the value
115-
expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxThinkingTokens", 8000) // 80% of 10000
115+
expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxThinkingTokens", 8000, false) // 80% of 10000
116116
})
117117

118118
it("should use default thinking tokens if not provided", () => {

webview-ui/src/components/settings/providers/HuggingFace.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ type HuggingFaceModel = {
3434

3535
type HuggingFaceProps = {
3636
apiConfiguration: ProviderSettings
37-
setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void
37+
setApiConfigurationField: (
38+
field: keyof ProviderSettings,
39+
value: ProviderSettings[keyof ProviderSettings],
40+
isUserAction?: boolean,
41+
) => void
3842
}
3943

4044
export const HuggingFace = ({ apiConfiguration, setApiConfigurationField }: HuggingFaceProps) => {
@@ -93,7 +97,7 @@ export const HuggingFace = ({ apiConfiguration, setApiConfigurationField }: Hugg
9397
// Set to "auto" as default
9498
const defaultProvider = "auto"
9599
setSelectedProvider(defaultProvider)
96-
setApiConfigurationField("huggingFaceInferenceProvider", defaultProvider)
100+
setApiConfigurationField("huggingFaceInferenceProvider", defaultProvider, false) // false = automatic default
97101
}
98102
}
99103
}

webview-ui/src/components/settings/providers/Unbound.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import { ModelPicker } from "../ModelPicker"
1717

1818
type UnboundProps = {
1919
apiConfiguration: ProviderSettings
20-
setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void
20+
setApiConfigurationField: (
21+
field: keyof ProviderSettings,
22+
value: ProviderSettings[keyof ProviderSettings],
23+
isUserAction?: boolean,
24+
) => void
2125
routerModels?: RouterModels
2226
organizationAllowList: OrganizationAllowList
2327
modelValidationError?: string
@@ -110,7 +114,7 @@ export const Unbound = ({
110114

111115
if (!currentModelId || !modelExists) {
112116
const firstAvailableModelId = Object.keys(updatedModels)[0]
113-
setApiConfigurationField("unboundModelId", firstAvailableModelId)
117+
setApiConfigurationField("unboundModelId", firstAvailableModelId, false) // false = automatic model selection
114118
}
115119
}
116120

0 commit comments

Comments
 (0)