Skip to content

Commit 4d22dac

Browse files
committed
🐞 fix(settings): Fix Wrong model after login (#213)
Previously we were spreading the entire `prevCachedState.apiConfiguration` object when the keys changed, which would wipe out unsaved changes the user might have made to things like `prevCachedState.apiConfiguration.kiloModel`. Now we only set the keys that might be set via callback and don't trigger a save explicitly. The user will need to click 'save' in order to exit the SettingView like normal (same as changing any other setting). - update state with specific tokens/keys to avoid overwriting unsaved changes - simplify useEffect dependencies by using direct variables **Dev thoughts** This does make me think.. should "login" keys / tokens be a "setting"? seems like it should be in the Settings view UI, but possibly separate from the other settings state of the api. I feel like it's not intuitive to have to click a "save" button once you login to something for it to remain saved. **Test plan**: Verified that logging in / out no longer overwrites other non-saved changes in the Settings. Now users will have to click save for any changes they make, including login state. (This should be ok since we have that confirmation dialog if they don't.) **Closes** #123
1 parent acb96eb commit 4d22dac

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

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

+14-8
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,22 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, o
172172
// kilocode_change start
173173
// Temporary way of making sure that the Settings view updates its local state properly when receiving
174174
// api keys from providers that support url callbacks. This whole Settings View needs proper with this local state thing later
175+
const { kilocodeToken, openRouterApiKey, glamaApiKey, requestyApiKey } = extensionState.apiConfiguration ?? {}
175176
useEffect(() => {
176-
setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState }))
177-
setChangeDetected(false)
177+
setCachedState((prevCachedState) => ({
178+
...prevCachedState,
179+
apiConfiguration: {
180+
...prevCachedState.apiConfiguration,
181+
// Only set specific tokens/keys instead of spreading the entire
182+
// `prevCachedState.apiConfiguration` since it may contain unsaved changes
183+
kilocodeToken,
184+
openRouterApiKey,
185+
glamaApiKey,
186+
requestyApiKey,
187+
},
188+
}))
178189
// eslint-disable-next-line react-hooks/exhaustive-deps
179-
}, [
180-
extensionState.apiConfiguration?.kilocodeToken,
181-
extensionState.apiConfiguration?.openRouterApiKey,
182-
extensionState.apiConfiguration?.glamaApiKey,
183-
extensionState.apiConfiguration?.requestyApiKey,
184-
])
190+
}, [kilocodeToken, openRouterApiKey, glamaApiKey, requestyApiKey])
185191

186192
useEffect(() => {
187193
// Only update if we're not already detecting changes

0 commit comments

Comments
 (0)