From 53304df70d4d2c4a8af042fa2ff8c28df72e97ce Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 11 Mar 2026 17:08:52 -0800 Subject: [PATCH] fix: disallow deselecting dynamic dropdown value (#22931) This was accomplished by switching from the comboxbox (which has deselection logic) to a select (which does not). As a side effect, the dropdowns are wider now, which seems to match better with other inputs anyway. And, it seems it uses the `combobox` role instead of `button` which also seems to make more sense. Lastly, they lose some bolding. --- .../DynamicParameter.jest.tsx | 6 +- .../DynamicParameter/DynamicParameter.tsx | 57 +++++++------------ .../CreateWorkspacePage.jest.tsx | 3 +- 3 files changed, 25 insertions(+), 41 deletions(-) diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.jest.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.jest.tsx index 33112d45e2..f5e9d370f3 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.jest.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.jest.tsx @@ -191,7 +191,7 @@ describe("DynamicParameter", () => { />, ); - const select = screen.getByRole("button"); + const select = screen.getByRole("combobox"); await waitFor(async () => { await userEvent.click(select); }); @@ -211,7 +211,7 @@ describe("DynamicParameter", () => { />, ); - const select = screen.getByRole("button"); + const select = screen.getByRole("combobox"); await waitFor(async () => { await userEvent.click(select); }); @@ -703,7 +703,7 @@ describe("DynamicParameter", () => { />, ); - expect(screen.getByRole("button")).toBeInTheDocument(); + expect(screen.getByRole("combobox")).toBeInTheDocument(); }); it("handles null/undefined values", () => { diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx index 50bc040f39..0609c25382 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx @@ -7,14 +7,6 @@ import type { import { Badge } from "components/Badge/Badge"; import { Button } from "components/Button/Button"; import { Checkbox } from "components/Checkbox/Checkbox"; -import { - Combobox, - ComboboxButton, - ComboboxContent, - ComboboxItem, - ComboboxList, - ComboboxTrigger, -} from "components/Combobox/Combobox"; import { ExternalImage } from "components/ExternalImage/ExternalImage"; import { Input } from "components/Input/Input"; import { Label } from "components/Label/Label"; @@ -24,6 +16,13 @@ import { type Option, } from "components/MultiSelectCombobox/MultiSelectCombobox"; import { RadioGroup, RadioGroupItem } from "components/RadioGroup/RadioGroup"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "components/Select/Select"; import { Slider } from "components/Slider/Slider"; import { Stack } from "components/Stack/Stack"; import { Switch } from "components/Switch/Switch"; @@ -335,41 +334,25 @@ const ParameterField: FC = ({ } case "dropdown": { - const selectedOption = parameter.options.find( - (opt) => opt.value.value === value, - ); return ( - onChange(newValue ?? "")} + disabled={disabled} > - - + - - - - {parameter.options.map((option) => ( - - {option.name} - - ))} - - - + + + {parameter.options.map((option) => ( + + {option.name} + + ))} + + ); } diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx index 7bcf37df5e..b603836574 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx @@ -140,7 +140,8 @@ describe("CreateWorkspacePage", () => { const instanceTypeField = screen.getByTestId( "parameter-field-instance_type", ); - const instanceTypeSelect = within(instanceTypeField).getByRole("button"); + const instanceTypeSelect = + within(instanceTypeField).getByRole("combobox"); expect(instanceTypeSelect).toBeInTheDocument(); jest.useFakeTimers();