Skip to content

Commit 622da63

Browse files
authored
fix: prevent dirty state on initial mount in ImageGenerationSettings (#7495)
1 parent 6ef9dbd commit 622da63

File tree

2 files changed

+119
-7
lines changed

2 files changed

+119
-7
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">
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { render, fireEvent } from "@testing-library/react"
2+
import { vi } from "vitest"
3+
import { ImageGenerationSettings } from "../ImageGenerationSettings"
4+
import type { ProviderSettings } from "@roo-code/types"
5+
6+
// Mock the translation context
7+
vi.mock("@/i18n/TranslationContext", () => ({
8+
useAppTranslation: () => ({
9+
t: (key: string) => key,
10+
}),
11+
}))
12+
13+
describe("ImageGenerationSettings", () => {
14+
const mockSetApiConfigurationField = vi.fn()
15+
const mockOnChange = vi.fn()
16+
17+
const defaultProps = {
18+
enabled: false,
19+
onChange: mockOnChange,
20+
apiConfiguration: {} as ProviderSettings,
21+
setApiConfigurationField: mockSetApiConfigurationField,
22+
}
23+
24+
beforeEach(() => {
25+
vi.clearAllMocks()
26+
})
27+
28+
describe("Initial Mount Behavior", () => {
29+
it("should not call setApiConfigurationField on initial mount with empty configuration", () => {
30+
render(<ImageGenerationSettings {...defaultProps} />)
31+
32+
// Should NOT call setApiConfigurationField on initial mount to prevent dirty state
33+
expect(mockSetApiConfigurationField).not.toHaveBeenCalled()
34+
})
35+
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
43+
44+
render(<ImageGenerationSettings {...defaultProps} apiConfiguration={apiConfiguration} />)
45+
46+
// Should NOT call setApiConfigurationField on initial mount to prevent dirty state
47+
expect(mockSetApiConfigurationField).not.toHaveBeenCalled()
48+
})
49+
})
50+
51+
describe("User Interaction Behavior", () => {
52+
it("should call setApiConfigurationField when user changes API key", async () => {
53+
const { getByPlaceholderText } = render(<ImageGenerationSettings {...defaultProps} enabled={true} />)
54+
55+
const apiKeyInput = getByPlaceholderText(
56+
"settings:experimental.IMAGE_GENERATION.openRouterApiKeyPlaceholder",
57+
)
58+
59+
// Simulate user typing
60+
fireEvent.input(apiKeyInput, { target: { value: "new-api-key" } })
61+
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+
)
71+
})
72+
73+
// Note: Testing VSCode dropdown components is complex due to their custom nature
74+
// The key functionality (not marking as dirty on initial mount) is already tested above
75+
})
76+
77+
describe("Conditional Rendering", () => {
78+
it("should render input fields when enabled is true", () => {
79+
const { getByPlaceholderText } = render(<ImageGenerationSettings {...defaultProps} enabled={true} />)
80+
81+
expect(
82+
getByPlaceholderText("settings:experimental.IMAGE_GENERATION.openRouterApiKeyPlaceholder"),
83+
).toBeInTheDocument()
84+
})
85+
86+
it("should not render input fields when enabled is false", () => {
87+
const { queryByPlaceholderText } = render(<ImageGenerationSettings {...defaultProps} enabled={false} />)
88+
89+
expect(
90+
queryByPlaceholderText("settings:experimental.IMAGE_GENERATION.openRouterApiKeyPlaceholder"),
91+
).not.toBeInTheDocument()
92+
})
93+
})
94+
})

0 commit comments

Comments
 (0)