Skip to content

Commit 94b4a5c

Browse files
daniel-lxshannesrudolph
authored andcommitted
fix: prevent dirty state on initial mount in ImageGenerationSettings (#7495)
1 parent 67af3ba commit 94b4a5c

File tree

2 files changed

+54
-32
lines changed

2 files changed

+54
-32
lines changed

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,32 @@ export const ImageGenerationSettings = ({
3636
imageGenerationSettings.selectedModel || IMAGE_GENERATION_MODELS[0].value,
3737
)
3838

39-
// Update parent state when local state changes
39+
// Update local state when apiConfiguration changes (e.g., when switching profiles)
4040
useEffect(() => {
41+
setOpenRouterApiKey(imageGenerationSettings.openRouterApiKey || "")
42+
setSelectedModel(imageGenerationSettings.selectedModel || IMAGE_GENERATION_MODELS[0].value)
43+
}, [imageGenerationSettings.openRouterApiKey, imageGenerationSettings.selectedModel])
44+
45+
// Helper function to update settings
46+
const updateSettings = (newApiKey: string, newModel: string) => {
4147
const newSettings = {
42-
openRouterApiKey,
43-
selectedModel,
48+
openRouterApiKey: newApiKey,
49+
selectedModel: newModel,
4450
}
45-
setApiConfigurationField("openRouterImageGenerationSettings", newSettings)
46-
}, [openRouterApiKey, selectedModel, setApiConfigurationField])
51+
setApiConfigurationField("openRouterImageGenerationSettings", newSettings, true)
52+
}
53+
54+
// Handle API key changes
55+
const handleApiKeyChange = (value: string) => {
56+
setOpenRouterApiKey(value)
57+
updateSettings(value, selectedModel)
58+
}
59+
60+
// Handle model selection changes
61+
const handleModelChange = (value: string) => {
62+
setSelectedModel(value)
63+
updateSettings(openRouterApiKey, value)
64+
}
4765

4866
return (
4967
<div className="space-y-4">
@@ -67,7 +85,7 @@ export const ImageGenerationSettings = ({
6785
</label>
6886
<VSCodeTextField
6987
value={openRouterApiKey}
70-
onInput={(e: any) => setOpenRouterApiKey(e.target.value)}
88+
onInput={(e: any) => handleApiKeyChange(e.target.value)}
7189
placeholder={t("settings:experimental.IMAGE_GENERATION.openRouterApiKeyPlaceholder")}
7290
className="w-full"
7391
type="password"
@@ -91,7 +109,7 @@ export const ImageGenerationSettings = ({
91109
</label>
92110
<VSCodeDropdown
93111
value={selectedModel}
94-
onChange={(e: any) => setSelectedModel(e.target.value)}
112+
onChange={(e: any) => handleModelChange(e.target.value)}
95113
className="w-full">
96114
{IMAGE_GENERATION_MODELS.map((model) => (
97115
<VSCodeOption key={model.value} value={model.value} className="py-2 px-3">

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

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { render, fireEvent } from "@testing-library/react"
2-
2+
import { vi } from "vitest"
33
import { ImageGenerationSettings } from "../ImageGenerationSettings"
4+
import type { ProviderSettings } from "@roo-code/types"
45

56
// Mock the translation context
67
vi.mock("@/i18n/TranslationContext", () => ({
@@ -10,49 +11,45 @@ vi.mock("@/i18n/TranslationContext", () => ({
1011
}))
1112

1213
describe("ImageGenerationSettings", () => {
13-
const mockSetOpenRouterImageApiKey = vi.fn()
14-
const mockSetImageGenerationSelectedModel = vi.fn()
14+
const mockSetApiConfigurationField = vi.fn()
1515
const mockOnChange = vi.fn()
1616

1717
const defaultProps = {
1818
enabled: false,
1919
onChange: mockOnChange,
20-
openRouterImageApiKey: undefined,
21-
openRouterImageGenerationSelectedModel: undefined,
22-
setOpenRouterImageApiKey: mockSetOpenRouterImageApiKey,
23-
setImageGenerationSelectedModel: mockSetImageGenerationSelectedModel,
20+
apiConfiguration: {} as ProviderSettings,
21+
setApiConfigurationField: mockSetApiConfigurationField,
2422
}
2523

2624
beforeEach(() => {
2725
vi.clearAllMocks()
2826
})
2927

3028
describe("Initial Mount Behavior", () => {
31-
it("should not call setter functions on initial mount with empty configuration", () => {
29+
it("should not call setApiConfigurationField on initial mount with empty configuration", () => {
3230
render(<ImageGenerationSettings {...defaultProps} />)
3331

34-
// Should NOT call setter functions on initial mount to prevent dirty state
35-
expect(mockSetOpenRouterImageApiKey).not.toHaveBeenCalled()
36-
expect(mockSetImageGenerationSelectedModel).not.toHaveBeenCalled()
32+
// Should NOT call setApiConfigurationField on initial mount to prevent dirty state
33+
expect(mockSetApiConfigurationField).not.toHaveBeenCalled()
3734
})
3835

39-
it("should not call setter functions on initial mount with existing configuration", () => {
40-
render(
41-
<ImageGenerationSettings
42-
{...defaultProps}
43-
openRouterImageApiKey="existing-key"
44-
openRouterImageGenerationSelectedModel="google/gemini-2.5-flash-image-preview:free"
45-
/>,
46-
)
36+
it("should not call setApiConfigurationField on initial mount with existing configuration", () => {
37+
const apiConfiguration = {
38+
openRouterImageGenerationSettings: {
39+
openRouterApiKey: "existing-key",
40+
selectedModel: "google/gemini-2.5-flash-image-preview:free",
41+
},
42+
} as ProviderSettings
4743

48-
// Should NOT call setter functions on initial mount to prevent dirty state
49-
expect(mockSetOpenRouterImageApiKey).not.toHaveBeenCalled()
50-
expect(mockSetImageGenerationSelectedModel).not.toHaveBeenCalled()
44+
render(<ImageGenerationSettings {...defaultProps} apiConfiguration={apiConfiguration} />)
45+
46+
// Should NOT call setApiConfigurationField on initial mount to prevent dirty state
47+
expect(mockSetApiConfigurationField).not.toHaveBeenCalled()
5148
})
5249
})
5350

5451
describe("User Interaction Behavior", () => {
55-
it("should call setimageGenerationSettings when user changes API key", async () => {
52+
it("should call setApiConfigurationField when user changes API key", async () => {
5653
const { getByPlaceholderText } = render(<ImageGenerationSettings {...defaultProps} enabled={true} />)
5754

5855
const apiKeyInput = getByPlaceholderText(
@@ -62,8 +59,15 @@ describe("ImageGenerationSettings", () => {
6259
// Simulate user typing
6360
fireEvent.input(apiKeyInput, { target: { value: "new-api-key" } })
6461

65-
// Should call setimageGenerationSettings
66-
expect(defaultProps.setOpenRouterImageApiKey).toHaveBeenCalledWith("new-api-key")
62+
// Should call setApiConfigurationField with isUserAction=true
63+
expect(mockSetApiConfigurationField).toHaveBeenCalledWith(
64+
"openRouterImageGenerationSettings",
65+
{
66+
openRouterApiKey: "new-api-key",
67+
selectedModel: "google/gemini-2.5-flash-image-preview",
68+
},
69+
true, // This should be true for user actions
70+
)
6771
})
6872

6973
// Note: Testing VSCode dropdown components is complex due to their custom nature

0 commit comments

Comments
 (0)