Skip to content

Commit e9f6590

Browse files
daniel-lxsdaniel-lxs
committed
fix: address review feedback - hide grounding features in welcome view and only show for Gemini models
1 parent 5220b36 commit e9f6590

File tree

3 files changed

+89
-27
lines changed

3 files changed

+89
-27
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,11 @@ const ApiOptions = ({
466466
)}
467467

468468
{selectedProvider === "vertex" && (
469-
<Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={setApiConfigurationField} />
469+
<Vertex
470+
apiConfiguration={apiConfiguration}
471+
setApiConfigurationField={setApiConfigurationField}
472+
fromWelcomeView={fromWelcomeView}
473+
/>
470474
)}
471475

472476
{selectedProvider === "gemini" && (

‎webview-ui/src/components/settings/providers/Vertex.tsx‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import { inputEventTransform } from "../transforms"
1212
type VertexProps = {
1313
apiConfiguration: ProviderSettings
1414
setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void
15+
fromWelcomeView?: boolean
1516
}
1617

17-
export const Vertex = ({ apiConfiguration, setApiConfigurationField }: VertexProps) => {
18+
export const Vertex = ({ apiConfiguration, setApiConfigurationField, fromWelcomeView }: VertexProps) => {
1819
const { t } = useAppTranslation()
1920

2021
const handleInputChange = useCallback(
@@ -93,27 +94,29 @@ export const Vertex = ({ apiConfiguration, setApiConfigurationField }: VertexPro
9394
</Select>
9495
</div>
9596

96-
<div className="mt-6">
97-
<Checkbox
98-
data-testid="checkbox-url-context"
99-
checked={!!apiConfiguration.enableUrlContext}
100-
onChange={(checked: boolean) => setApiConfigurationField("enableUrlContext", checked)}>
101-
{t("settings:providers.geminiParameters.urlContext.title")}
102-
</Checkbox>
103-
<div className="text-sm text-vscode-descriptionForeground mb-3 mt-1.5">
104-
{t("settings:providers.geminiParameters.urlContext.description")}
105-
</div>
97+
{!fromWelcomeView && apiConfiguration.apiModelId?.startsWith("gemini") && (
98+
<div className="mt-6">
99+
<Checkbox
100+
data-testid="checkbox-url-context"
101+
checked={!!apiConfiguration.enableUrlContext}
102+
onChange={(checked: boolean) => setApiConfigurationField("enableUrlContext", checked)}>
103+
{t("settings:providers.geminiParameters.urlContext.title")}
104+
</Checkbox>
105+
<div className="text-sm text-vscode-descriptionForeground mb-3 mt-1.5">
106+
{t("settings:providers.geminiParameters.urlContext.description")}
107+
</div>
106108

107-
<Checkbox
108-
data-testid="checkbox-grounding-search"
109-
checked={!!apiConfiguration.enableGrounding}
110-
onChange={(checked: boolean) => setApiConfigurationField("enableGrounding", checked)}>
111-
{t("settings:providers.geminiParameters.groundingSearch.title")}
112-
</Checkbox>
113-
<div className="text-sm text-vscode-descriptionForeground mb-3 mt-1.5">
114-
{t("settings:providers.geminiParameters.groundingSearch.description")}
109+
<Checkbox
110+
data-testid="checkbox-grounding-search"
111+
checked={!!apiConfiguration.enableGrounding}
112+
onChange={(checked: boolean) => setApiConfigurationField("enableGrounding", checked)}>
113+
{t("settings:providers.geminiParameters.groundingSearch.title")}
114+
</Checkbox>
115+
<div className="text-sm text-vscode-descriptionForeground mb-3 mt-1.5">
116+
{t("settings:providers.geminiParameters.groundingSearch.description")}
117+
</div>
115118
</div>
116-
</div>
119+
)}
117120
</>
118121
)
119122
}

‎webview-ui/src/components/settings/providers/__tests__/Vertex.spec.tsx‎

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe("Vertex", () => {
4747
vertexRegion: "",
4848
enableUrlContext: false,
4949
enableGrounding: false,
50+
apiModelId: "gemini-2.0-flash-001",
5051
}
5152

5253
const mockSetApiConfigurationField = vi.fn()
@@ -113,7 +114,7 @@ describe("Vertex", () => {
113114
})
114115

115116
describe("URL Context Checkbox", () => {
116-
it("should render URL context checkbox unchecked by default", () => {
117+
it("should render URL context checkbox unchecked by default for Gemini models", () => {
117118
render(
118119
<Vertex
119120
apiConfiguration={defaultApiConfiguration}
@@ -126,8 +127,35 @@ describe("Vertex", () => {
126127
expect(checkbox.checked).toBe(false)
127128
})
128129

129-
it("should render URL context checkbox checked when enableUrlContext is true", () => {
130-
const apiConfiguration = { ...defaultApiConfiguration, enableUrlContext: true }
130+
it("should NOT render URL context checkbox for non-Gemini models", () => {
131+
const apiConfiguration = { ...defaultApiConfiguration, apiModelId: "claude-3-opus@20240229" }
132+
render(
133+
<Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={mockSetApiConfigurationField} />,
134+
)
135+
136+
const urlContextCheckbox = screen.queryByTestId("checkbox-url-context")
137+
expect(urlContextCheckbox).toBeNull()
138+
})
139+
140+
it("should NOT render URL context checkbox when fromWelcomeView is true", () => {
141+
render(
142+
<Vertex
143+
apiConfiguration={defaultApiConfiguration}
144+
setApiConfigurationField={mockSetApiConfigurationField}
145+
fromWelcomeView={true}
146+
/>,
147+
)
148+
149+
const urlContextCheckbox = screen.queryByTestId("checkbox-url-context")
150+
expect(urlContextCheckbox).toBeNull()
151+
})
152+
153+
it("should render URL context checkbox checked when enableUrlContext is true for Gemini models", () => {
154+
const apiConfiguration = {
155+
...defaultApiConfiguration,
156+
enableUrlContext: true,
157+
apiModelId: "gemini-2.0-flash-001",
158+
}
131159
render(
132160
<Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={mockSetApiConfigurationField} />,
133161
)
@@ -156,7 +184,7 @@ describe("Vertex", () => {
156184
})
157185

158186
describe("Grounding with Google Search Checkbox", () => {
159-
it("should render grounding search checkbox unchecked by default", () => {
187+
it("should render grounding search checkbox unchecked by default for Gemini models", () => {
160188
render(
161189
<Vertex
162190
apiConfiguration={defaultApiConfiguration}
@@ -169,8 +197,35 @@ describe("Vertex", () => {
169197
expect(checkbox.checked).toBe(false)
170198
})
171199

172-
it("should render grounding search checkbox checked when enableGrounding is true", () => {
173-
const apiConfiguration = { ...defaultApiConfiguration, enableGrounding: true }
200+
it("should NOT render grounding search checkbox for non-Gemini models", () => {
201+
const apiConfiguration = { ...defaultApiConfiguration, apiModelId: "claude-3-opus@20240229" }
202+
render(
203+
<Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={mockSetApiConfigurationField} />,
204+
)
205+
206+
const groundingCheckbox = screen.queryByTestId("checkbox-grounding-search")
207+
expect(groundingCheckbox).toBeNull()
208+
})
209+
210+
it("should NOT render grounding search checkbox when fromWelcomeView is true", () => {
211+
render(
212+
<Vertex
213+
apiConfiguration={defaultApiConfiguration}
214+
setApiConfigurationField={mockSetApiConfigurationField}
215+
fromWelcomeView={true}
216+
/>,
217+
)
218+
219+
const groundingCheckbox = screen.queryByTestId("checkbox-grounding-search")
220+
expect(groundingCheckbox).toBeNull()
221+
})
222+
223+
it("should render grounding search checkbox checked when enableGrounding is true for Gemini models", () => {
224+
const apiConfiguration = {
225+
...defaultApiConfiguration,
226+
enableGrounding: true,
227+
apiModelId: "gemini-2.0-flash-001",
228+
}
174229
render(
175230
<Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={mockSetApiConfigurationField} />,
176231
)

0 commit comments

Comments
 (0)