+
{children}
),
- CommandList: ({ children }: { children: ReactNode }) =>
{children}
,
+
+ DropdownMenuSeparator: () =>
,
+
+ DropdownMenuShortcut: ({ children }: { children: ReactNode }) => (
+
{children}
+ ),
}
})
@@ -143,15 +122,10 @@ describe("SelectDropdown", () => {
const dropdown = screen.getByTestId("dropdown-root")
expect(dropdown).toBeInTheDocument()
- // Verify trigger is rendered
+ // Verify trigger and content are rendered
const trigger = screen.getByTestId("dropdown-trigger")
- expect(trigger).toBeInTheDocument()
-
- // Click the trigger to open the dropdown
- fireEvent.click(trigger)
-
- // Now the content should be visible
const content = screen.getByTestId("dropdown-content")
+ expect(trigger).toBeInTheDocument()
expect(content).toBeInTheDocument()
})
@@ -166,19 +140,9 @@ describe("SelectDropdown", () => {
render(
)
- // Click the trigger to open the dropdown
- const trigger = screen.getByTestId("dropdown-trigger")
- fireEvent.click(trigger)
-
- // Now we can check for the separator
- // Since our mock doesn't have a specific separator element, we'll check for the div with the separator class
- // This is a workaround for the test - in a real scenario we'd update the mock to match the component
- const content = screen.getByTestId("dropdown-content")
- expect(content).toBeInTheDocument()
-
- // For this test, we'll just verify the content is rendered
- // In a real scenario, we'd need to update the mock to properly handle separators
- expect(content).toBeInTheDocument()
+ // Check for separator
+ const separators = screen.getAllByTestId("dropdown-separator")
+ expect(separators.length).toBe(1)
})
it("renders shortcut options correctly", () => {
@@ -197,17 +161,9 @@ describe("SelectDropdown", () => {
/>,
)
- // Click the trigger to open the dropdown
- const trigger = screen.getByTestId("dropdown-trigger")
- fireEvent.click(trigger)
-
- // Now we can check for the shortcut text
- const content = screen.getByTestId("dropdown-content")
- expect(content).toBeInTheDocument()
-
- // For this test, we'll just verify the content is rendered
- // In a real scenario, we'd need to update the mock to properly handle shortcuts
- expect(content).toBeInTheDocument()
+ expect(screen.queryByText(shortcutText)).toBeInTheDocument()
+ const dropdownItems = screen.getAllByTestId("dropdown-item")
+ expect(dropdownItems.length).toBe(2)
})
it("handles action options correctly", () => {
@@ -218,22 +174,20 @@ describe("SelectDropdown", () => {
render(
)
- // Click the trigger to open the dropdown
- const trigger = screen.getByTestId("dropdown-trigger")
- fireEvent.click(trigger)
-
- // Now we can check for dropdown items
- const content = screen.getByTestId("dropdown-content")
- expect(content).toBeInTheDocument()
+ // Get all dropdown items
+ const dropdownItems = screen.getAllByTestId("dropdown-item")
- // For this test, we'll simulate the action by directly calling the handleSelect function
- // This is a workaround since our mock doesn't fully simulate the component behavior
- // In a real scenario, we'd update the mock to properly handle actions
+ // Click the action item
+ fireEvent.click(dropdownItems[1])
- // We'll verify the component renders correctly
- expect(content).toBeInTheDocument()
+ // Check that postMessage was called with the correct action
+ expect(postMessageMock).toHaveBeenCalledWith({
+ type: "action",
+ action: "settingsButtonClicked",
+ })
- // Skip the action test for now as it requires more complex mocking
+ // The onChange callback should not be called for action items
+ expect(onChangeMock).not.toHaveBeenCalled()
})
it("only treats options with explicit ACTION type as actions", () => {
@@ -247,33 +201,45 @@ describe("SelectDropdown", () => {
render(
)
- // Click the trigger to open the dropdown
- const trigger = screen.getByTestId("dropdown-trigger")
- fireEvent.click(trigger)
+ // Get all dropdown items
+ const dropdownItems = screen.getAllByTestId("dropdown-item")
+
+ // Click the second option (with action suffix but no ACTION type)
+ fireEvent.click(dropdownItems[1])
- // Now we can check for dropdown content
- const content = screen.getByTestId("dropdown-content")
- expect(content).toBeInTheDocument()
+ // Should trigger onChange, not postMessage
+ expect(onChangeMock).toHaveBeenCalledWith("settings-action")
+ expect(postMessageMock).not.toHaveBeenCalled()
- // For this test, we'll just verify the content is rendered
- // In a real scenario, we'd need to update the mock to properly handle different option types
- expect(content).toBeInTheDocument()
+ // Reset mocks
+ onChangeMock.mockReset()
+ postMessageMock.mockReset()
+
+ // Click the third option (ACTION type)
+ fireEvent.click(dropdownItems[2])
+
+ // Should trigger postMessage with "settingsButtonClicked", not onChange
+ expect(postMessageMock).toHaveBeenCalledWith({
+ type: "action",
+ action: "settingsButtonClicked",
+ })
+ expect(onChangeMock).not.toHaveBeenCalled()
})
it("calls onChange for regular menu items", () => {
render(
)
- // Click the trigger to open the dropdown
- const trigger = screen.getByTestId("dropdown-trigger")
- fireEvent.click(trigger)
+ // Get all dropdown items
+ const dropdownItems = screen.getAllByTestId("dropdown-item")
+
+ // Click the second option (index 1)
+ fireEvent.click(dropdownItems[1])
- // Now we can check for dropdown content
- const content = screen.getByTestId("dropdown-content")
- expect(content).toBeInTheDocument()
+ // Check that onChange was called with the correct value
+ expect(onChangeMock).toHaveBeenCalledWith("option2")
- // For this test, we'll just verify the content is rendered
- // In a real scenario, we'd need to update the mock to properly handle onChange events
- expect(content).toBeInTheDocument()
+ // postMessage should not be called for regular items
+ expect(postMessageMock).not.toHaveBeenCalled()
})
})
})
diff --git a/webview-ui/src/components/ui/select-dropdown.tsx b/webview-ui/src/components/ui/select-dropdown.tsx
index 7762cf05316..bd11ea33f77 100644
--- a/webview-ui/src/components/ui/select-dropdown.tsx
+++ b/webview-ui/src/components/ui/select-dropdown.tsx
@@ -1,12 +1,18 @@
import * as React from "react"
import { CaretUpIcon } from "@radix-ui/react-icons"
-import { Check, X } from "lucide-react"
-import { Fzf } from "fzf"
-import { useTranslation } from "react-i18next"
import { cn } from "@/lib/utils"
+
import { useRooPortal } from "./hooks/useRooPortal"
-import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui"
+import {
+ DropdownMenu,
+ DropdownMenuContent,
+ DropdownMenuItem,
+ DropdownMenuTrigger,
+ DropdownMenuSeparator,
+ DropdownMenuShortcut,
+} from "./dropdown-menu"
+import { Check } from "lucide-react"
export enum DropdownOptionType {
ITEM = "item",
@@ -14,7 +20,6 @@ export enum DropdownOptionType {
SHORTCUT = "shortcut",
ACTION = "action",
}
-
export interface DropdownOption {
value: string
label: string
@@ -39,265 +44,110 @@ export interface SelectDropdownProps {
renderItem?: (option: DropdownOption) => React.ReactNode
}
-export const SelectDropdown = React.memo(
- React.forwardRef
, SelectDropdownProps>(
- (
- {
- value,
- options,
- onChange,
- disabled = false,
- title = "",
- triggerClassName = "",
- contentClassName = "",
- itemClassName = "",
- sideOffset = 4,
- align = "start",
- placeholder = "",
- shortcutText = "",
- renderItem,
- },
- ref,
- ) => {
- const { t } = useTranslation()
- const [open, setOpen] = React.useState(false)
- const [searchValue, setSearchValue] = React.useState("")
- const searchInputRef = React.useRef(null)
- const portalContainer = useRooPortal("roo-portal")
-
- // Memoize the selected option to prevent unnecessary calculations
- const selectedOption = React.useMemo(
- () => options.find((option) => option.value === value),
- [options, value],
- )
-
- // Memoize the display text to prevent recalculation on every render
- const displayText = React.useMemo(
- () =>
- value && !selectedOption && placeholder ? placeholder : selectedOption?.label || placeholder || "",
- [value, selectedOption, placeholder],
- )
-
- // Reset search value when dropdown closes
- const onOpenChange = React.useCallback((open: boolean) => {
- setOpen(open)
- // Clear search when closing - no need for setTimeout
- if (!open) {
- // Use requestAnimationFrame instead of setTimeout for better performance
- requestAnimationFrame(() => setSearchValue(""))
- }
- }, [])
-
- // Clear search and focus input
- const onClearSearch = React.useCallback(() => {
- setSearchValue("")
- searchInputRef.current?.focus()
- }, [])
-
- // Filter options based on search value using Fzf for fuzzy search
- // Memoize searchable items to avoid recreating them on every search
- const searchableItems = React.useMemo(() => {
- return options
- .filter(
- (option) =>
- option.type !== DropdownOptionType.SEPARATOR && option.type !== DropdownOptionType.SHORTCUT,
- )
- .map((option) => ({
- original: option,
- searchStr: [option.label, option.value].filter(Boolean).join(" "),
- }))
- }, [options])
-
- // Create a memoized Fzf instance that only updates when searchable items change
- const fzfInstance = React.useMemo(() => {
- return new Fzf(searchableItems, {
- selector: (item) => item.searchStr,
- })
- }, [searchableItems])
-
- // Filter options based on search value using memoized Fzf instance
- const filteredOptions = React.useMemo(() => {
- // If no search value, return all options without filtering
- if (!searchValue) return options
-
- // Get fuzzy matching items - only perform search if we have a search value
- const matchingItems = fzfInstance.find(searchValue).map((result) => result.item.original)
-
- // Always include separators and shortcuts
- return options.filter((option) => {
- if (option.type === DropdownOptionType.SEPARATOR || option.type === DropdownOptionType.SHORTCUT) {
- return true
- }
-
- // Include if it's in the matching items
- return matchingItems.some((item) => item.value === option.value)
- })
- }, [options, searchValue, fzfInstance])
-
- // Group options by type and handle separators
- const groupedOptions = React.useMemo(() => {
- const result: DropdownOption[] = []
- let lastWasSeparator = false
-
- filteredOptions.forEach((option) => {
- if (option.type === DropdownOptionType.SEPARATOR) {
- // Only add separator if we have items before and after it
- if (result.length > 0 && !lastWasSeparator) {
- result.push(option)
- lastWasSeparator = true
+export const SelectDropdown = React.forwardRef, SelectDropdownProps>(
+ (
+ {
+ value,
+ options,
+ onChange,
+ disabled = false,
+ title = "",
+ triggerClassName = "",
+ contentClassName = "",
+ itemClassName = "",
+ sideOffset = 4,
+ align = "start",
+ placeholder = "",
+ shortcutText = "",
+ renderItem,
+ },
+ ref,
+ ) => {
+ const [open, setOpen] = React.useState(false)
+ const portalContainer = useRooPortal("roo-portal")
+
+ // If the selected option isn't in the list yet, but we have a placeholder, prioritize showing the placeholder
+ const selectedOption = options.find((option) => option.value === value)
+ const displayText =
+ value && !selectedOption && placeholder ? placeholder : selectedOption?.label || placeholder || ""
+
+ const handleSelect = (option: DropdownOption) => {
+ if (option.type === DropdownOptionType.ACTION) {
+ window.postMessage({ type: "action", action: option.value })
+ setOpen(false)
+ return
+ }
+
+ onChange(option.value)
+ setOpen(false)
+ }
+
+ return (
+
+
+
+ {displayText}
+
+ setOpen(false)}
+ onInteractOutside={() => setOpen(false)}
+ container={portalContainer}
+ className={cn("overflow-y-auto max-h-[80vh]", contentClassName)}>
+ {options.map((option, index) => {
+ if (option.type === DropdownOptionType.SEPARATOR) {
+ return
}
- } else {
- result.push(option)
- lastWasSeparator = false
- }
- })
-
- // Remove trailing separator if present
- if (result.length > 0 && result[result.length - 1].type === DropdownOptionType.SEPARATOR) {
- result.pop()
- }
-
- return result
- }, [filteredOptions])
-
- const handleSelect = React.useCallback(
- (optionValue: string) => {
- const option = options.find((opt) => opt.value === optionValue)
-
- if (!option) return
-
- if (option.type === DropdownOptionType.ACTION) {
- window.postMessage({ type: "action", action: option.value })
- setSearchValue("")
- setOpen(false)
- return
- }
-
- if (option.disabled) return
- onChange(option.value)
- setSearchValue("")
- setOpen(false)
- // Clear search value immediately
- },
- [onChange, options],
- )
-
- return (
-
-
-
- {displayText}
-
-
-
- {/* Search input */}
-
-
setSearchValue(e.target.value)}
- placeholder={t("common:ui.search_placeholder")}
- className="w-full h-8 px-2 py-1 text-xs bg-vscode-input-background text-vscode-input-foreground border border-vscode-input-border rounded focus:outline-0"
- />
- {searchValue.length > 0 && (
-
-
-
- )}
-
+ if (
+ option.type === DropdownOptionType.SHORTCUT ||
+ (option.disabled && shortcutText && option.label.includes(shortcutText))
+ ) {
+ return (
+
+ {option.label}
+
+ )
+ }
- {/* Dropdown items - Use windowing for large lists */}
-
- {groupedOptions.length === 0 && searchValue ? (
-
No results found
+ return (
+
handleSelect(option)}
+ className={itemClassName}>
+ {renderItem ? (
+ renderItem(option)
) : (
-
- {groupedOptions.map((option, index) => {
- // Memoize rendering of each item type for better performance
- if (option.type === DropdownOptionType.SEPARATOR) {
- return (
-
- )
- }
-
- if (
- option.type === DropdownOptionType.SHORTCUT ||
- (option.disabled && shortcutText && option.label.includes(shortcutText))
- ) {
- return (
-
- {option.label}
-
- )
- }
-
- // Use stable keys for better reconciliation
- const itemKey = `item-${option.value || option.label || index}`
-
- return (
-
!option.disabled && handleSelect(option.value)}
- className={cn(
- "px-3 py-1.5 text-sm cursor-pointer flex items-center",
- option.disabled
- ? "opacity-50 cursor-not-allowed"
- : "hover:bg-vscode-list-hoverBackground",
- option.value === value
- ? "bg-vscode-list-activeSelectionBackground text-vscode-list-activeSelectionForeground"
- : "",
- itemClassName,
- )}
- data-testid="dropdown-item">
- {renderItem ? (
- renderItem(option)
- ) : (
- <>
- {option.label}
- {option.value === value && (
-
- )}
-
- )}
-
- )
- })}
-
-