Skip to content

Commit 74d52aa

Browse files
committed
Fix issues with select dropdown
1 parent 541b54e commit 74d52aa

File tree

2 files changed

+36
-16
lines changed

2 files changed

+36
-16
lines changed

webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,21 @@ describe("SelectDropdown", () => {
115115
expect(trigger.classList.toString()).toContain("custom-trigger-class")
116116
})
117117

118+
it("ensures open state is controlled via props", () => {
119+
// Test that the component accepts and uses the open state controlled prop
120+
render(<SelectDropdown value="option1" options={options} onChange={onChangeMock} />)
121+
122+
// The component should render the dropdown root with correct props
123+
const dropdown = screen.getByTestId("dropdown-root")
124+
expect(dropdown).toBeInTheDocument()
125+
126+
// Verify trigger and content are rendered
127+
const trigger = screen.getByTestId("dropdown-trigger")
128+
const content = screen.getByTestId("dropdown-content")
129+
expect(trigger).toBeInTheDocument()
130+
expect(content).toBeInTheDocument()
131+
})
132+
118133
// Tests for the new functionality
119134
describe("Option types", () => {
120135
it("renders separator options correctly", () => {
@@ -131,20 +146,6 @@ describe("SelectDropdown", () => {
131146
expect(separators.length).toBe(1)
132147
})
133148

134-
it("renders string separator (backward compatibility) correctly", () => {
135-
const optionsWithStringSeparator = [
136-
{ value: "option1", label: "Option 1" },
137-
{ value: "sep-1", label: "────", disabled: true },
138-
{ value: "option2", label: "Option 2" },
139-
]
140-
141-
render(<SelectDropdown value="option1" options={optionsWithStringSeparator} onChange={onChangeMock} />)
142-
143-
// Check for separator
144-
const separators = screen.getAllByTestId("dropdown-separator")
145-
expect(separators.length).toBe(1)
146-
})
147-
148149
it("renders shortcut options correctly", () => {
149150
const shortcutText = "Ctrl+K"
150151
const optionsWithShortcut = [

webview-ui/src/components/ui/select-dropdown.tsx

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
DropdownMenuSeparator,
88
} from "./dropdown-menu"
99
import { cn } from "@/lib/utils"
10+
import { useEffect, useState } from "react"
1011

1112
// Constants for option types
1213
export enum DropdownOptionType {
@@ -57,6 +58,19 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
5758
},
5859
ref,
5960
) => {
61+
// Track open state
62+
const [open, setOpen] = React.useState(false)
63+
const [portalContainer, setPortalContainer] = useState<HTMLElement>()
64+
65+
useEffect(() => {
66+
// The dropdown menu uses a portal from @shadcn/ui which by default renders
67+
// at the document root. This causes the menu to remain visible even when
68+
// the parent ChatView component is hidden (during settings/history view).
69+
// By moving the portal inside ChatView, the menu will properly hide when
70+
// its parent is hidden.
71+
setPortalContainer(document.getElementById("chat-view-portal") || undefined)
72+
}, [])
73+
6074
// Find the selected option label
6175
const selectedOption = options.find((option) => option.value === value)
6276
const displayText = selectedOption?.label || placeholder || ""
@@ -69,13 +83,15 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
6983
type: "action",
7084
action: option.value,
7185
})
86+
setOpen(false)
7287
return
7388
}
7489
onChange(option.value)
90+
setOpen(false)
7591
}
7692

7793
return (
78-
<DropdownMenu>
94+
<DropdownMenu open={open} onOpenChange={setOpen}>
7995
<DropdownMenuTrigger
8096
ref={ref}
8197
disabled={disabled}
@@ -112,13 +128,16 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
112128
<DropdownMenuContent
113129
align={align}
114130
sideOffset={sideOffset}
131+
onEscapeKeyDown={() => setOpen(false)}
132+
onInteractOutside={() => setOpen(false)}
133+
container={portalContainer}
115134
className={cn(
116135
"bg-vscode-dropdown-background text-vscode-dropdown-foreground border border-vscode-dropdown-border z-50",
117136
contentClassName,
118137
)}>
119138
{options.map((option, index) => {
120139
// Handle separator type
121-
if (option.type === DropdownOptionType.SEPARATOR || option.label.includes("────")) {
140+
if (option.type === DropdownOptionType.SEPARATOR) {
122141
return <DropdownMenuSeparator key={`sep-${index}`} />
123142
}
124143

0 commit comments

Comments
 (0)