Skip to content

Commit dfeb752

Browse files
committed
fix: implement scalable solution for settings change detection
- Replace hardcoded exception for openRouterSpecificProvider with a scalable approach - Track which fields have been explicitly modified by user interaction - Fields are only considered 'changed' if they've been modified by the user, not during initial loading - Reset tracking when settings are saved, discarded, or reloaded - This ensures the Save button only enables for actual user changes, not initial data loading
1 parent 7e4a87b commit dfeb752

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,17 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
197197
setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState }))
198198
prevApiConfigName.current = currentApiConfigName
199199
setChangeDetected(false)
200+
// Reset user modified fields when loading new configuration
201+
setUserModifiedFields(new Set())
200202
}, [currentApiConfigName, extensionState, isChangeDetected])
201203

202204
// Bust the cache when settings are imported.
203205
useEffect(() => {
204206
if (settingsImportedAt) {
205207
setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState }))
206208
setChangeDetected(false)
209+
// Reset user modified fields when importing settings
210+
setUserModifiedFields(new Set())
207211
}
208212
}, [settingsImportedAt, extensionState])
209213

@@ -218,28 +222,35 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
218222
})
219223
}, [])
220224

225+
// Track which fields have been explicitly set by user interaction
226+
const [userModifiedFields, setUserModifiedFields] = useState<Set<keyof ProviderSettings>>(new Set())
227+
221228
const setApiConfigurationField = useCallback(
222-
<K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => {
229+
<K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K], isUserAction: boolean = true) => {
223230
setCachedState((prevState) => {
224231
if (prevState.apiConfiguration?.[field] === value) {
225232
return prevState
226233
}
227234

228235
const previousValue = prevState.apiConfiguration?.[field]
229236

237+
// Track if this field has been modified by the user
238+
if (isUserAction) {
239+
setUserModifiedFields((prev) => new Set(prev).add(field))
240+
}
241+
230242
// 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-
// Exception: openRouterSpecificProvider should always trigger change detection
243+
// unless this field has been explicitly modified by the user before
233244
const isInitialSync =
234-
previousValue === undefined && value !== undefined && field !== "openRouterSpecificProvider"
245+
previousValue === undefined && value !== undefined && !userModifiedFields.has(field)
235246

236247
if (!isInitialSync) {
237248
setChangeDetected(true)
238249
}
239250
return { ...prevState, apiConfiguration: { ...prevState.apiConfiguration, [field]: value } }
240251
})
241252
},
242-
[],
253+
[userModifiedFields],
243254
)
244255

245256
const setExperimentEnabled: SetExperimentEnabled = useCallback((id: ExperimentId, enabled: boolean) => {
@@ -348,6 +359,8 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
348359
vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting })
349360
vscode.postMessage({ type: "profileThresholds", values: profileThresholds })
350361
setChangeDetected(false)
362+
// Reset user modified fields after saving
363+
setUserModifiedFields(new Set())
351364
}
352365
}
353366

@@ -371,6 +384,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
371384
// Discard changes: Reset state and flag
372385
setCachedState(extensionState) // Revert to original state
373386
setChangeDetected(false) // Reset change flag
387+
setUserModifiedFields(new Set()) // Reset user modified fields
374388
confirmDialogHandler.current?.() // Execute the pending action (e.g., tab switch)
375389
}
376390
// If confirm is false (Cancel), do nothing, dialog closes automatically

0 commit comments

Comments
 (0)