diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index 51162d7b86..3df3e87e9b 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/webview-ui/src/components/chat/ChatTextArea.tsx @@ -16,7 +16,7 @@ import { vscode } from "../../utils/vscode" import { WebviewMessage } from "../../../../src/shared/WebviewMessage" import { Mode, getAllModes } from "../../../../src/shared/modes" import { convertToMentionPath } from "../../utils/path-mentions" -import { SelectDropdown } from "../ui" +import { SelectDropdown, DropdownOptionType } from "../ui" interface ChatTextAreaProps { inputValue: string @@ -779,22 +779,32 @@ const ChatTextArea = forwardRef( title="Select mode for interaction" options={[ // Add the shortcut text as a disabled option at the top - { value: "shortcut", label: modeShortcutText, disabled: true }, + { + value: "shortcut", + label: modeShortcutText, + disabled: true, + type: DropdownOptionType.SHORTCUT, + }, // Add all modes ...getAllModes(customModes).map((mode) => ({ value: mode.slug, label: mode.name, + type: DropdownOptionType.ITEM, })), // Add separator - { value: "sep-1", label: "────", disabled: true }, + { + value: "sep-1", + label: "Separator", + type: DropdownOptionType.SEPARATOR, + }, // Add Edit option - { value: "prompts-action", label: "Edit..." }, + { + value: "promptsButtonClicked", + label: "Edit...", + type: DropdownOptionType.ACTION, + }, ]} onChange={(value) => { - if (value === "prompts-action") { - window.postMessage({ type: "action", action: "promptsButtonClicked" }) - return - } setMode(value as Mode) vscode.postMessage({ type: "mode", @@ -822,17 +832,22 @@ const ChatTextArea = forwardRef( ...(listApiConfigMeta || []).map((config) => ({ value: config.name, label: config.name, + type: DropdownOptionType.ITEM, })), // Add separator - { value: "sep-1", label: "────", disabled: true }, + { + value: "sep-2", + label: "Separator", + type: DropdownOptionType.SEPARATOR, + }, // Add Edit option - { value: "settings-action", label: "Edit..." }, + { + value: "settingsButtonClicked", + label: "Edit...", + type: DropdownOptionType.ACTION, + }, ]} onChange={(value) => { - if (value === "settings-action") { - window.postMessage({ type: "action", action: "settingsButtonClicked" }) - return - } vscode.postMessage({ type: "loadApiConfiguration", text: value, @@ -849,7 +864,7 @@ const ChatTextArea = forwardRef( style={{ display: "flex", alignItems: "center", - gap: "8px", // Reduced from 12px + gap: "8px", flexShrink: 0, }}>
@@ -860,7 +875,7 @@ const ChatTextArea = forwardRef( color: "var(--vscode-input-foreground)", opacity: 0.5, fontSize: 16.5, - marginRight: 6, // Reduced from 10 + marginRight: 6, }} /> ) : ( diff --git a/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx b/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx index 89f8ddc3c2..5d65eaae98 100644 --- a/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx +++ b/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx @@ -1,6 +1,13 @@ import React, { ReactNode } from "react" -import { render, screen } from "@testing-library/react" -import { SelectDropdown } from "../select-dropdown" +import { render, screen, fireEvent } from "@testing-library/react" +import { SelectDropdown, DropdownOptionType } from "../select-dropdown" + +// Mock window.postMessage +const postMessageMock = jest.fn() +Object.defineProperty(window, "postMessage", { + writable: true, + value: postMessageMock, +}) // Mock the Radix UI DropdownMenu component and its children jest.mock("../dropdown-menu", () => { @@ -107,4 +114,133 @@ describe("SelectDropdown", () => { const trigger = screen.getByTestId("dropdown-trigger") expect(trigger.classList.toString()).toContain("custom-trigger-class") }) + + // Tests for the new functionality + describe("Option types", () => { + it("renders separator options correctly", () => { + const optionsWithTypedSeparator = [ + { value: "option1", label: "Option 1" }, + { value: "sep-1", label: "Separator", type: DropdownOptionType.SEPARATOR }, + { value: "option2", label: "Option 2" }, + ] + + render() + + // Check for separator + const separators = screen.getAllByTestId("dropdown-separator") + expect(separators.length).toBe(1) + }) + + it("renders string separator (backward compatibility) correctly", () => { + const optionsWithStringSeparator = [ + { value: "option1", label: "Option 1" }, + { value: "sep-1", label: "────", disabled: true }, + { value: "option2", label: "Option 2" }, + ] + + render() + + // Check for separator + const separators = screen.getAllByTestId("dropdown-separator") + expect(separators.length).toBe(1) + }) + + it("renders shortcut options correctly", () => { + const shortcutText = "Ctrl+K" + const optionsWithShortcut = [ + { value: "shortcut", label: shortcutText, type: DropdownOptionType.SHORTCUT }, + { value: "option1", label: "Option 1" }, + ] + + render( + , + ) + + // The shortcut text should be rendered as a div, not a dropdown item + expect(screen.queryByText(shortcutText)).toBeInTheDocument() + const dropdownItems = screen.getAllByTestId("dropdown-item") + expect(dropdownItems.length).toBe(1) // Only one regular option + }) + + it("handles action options correctly", () => { + const optionsWithAction = [ + { value: "option1", label: "Option 1" }, + { value: "settingsButtonClicked", label: "Settings", type: DropdownOptionType.ACTION }, + ] + + render() + + // Get all dropdown items + const dropdownItems = screen.getAllByTestId("dropdown-item") + + // Click the action item + fireEvent.click(dropdownItems[1]) + + // Check that postMessage was called with the correct action + expect(postMessageMock).toHaveBeenCalledWith({ + type: "action", + action: "settingsButtonClicked", + }) + + // The onChange callback should not be called for action items + expect(onChangeMock).not.toHaveBeenCalled() + }) + + it("only treats options with explicit ACTION type as actions", () => { + const optionsForTest = [ + { value: "option1", label: "Option 1" }, + // This should be treated as a regular option despite the -action suffix + { value: "settings-action", label: "Regular option with action suffix" }, + // This should be treated as an action + { value: "settingsButtonClicked", label: "Settings", type: DropdownOptionType.ACTION }, + ] + + render() + + // 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]) + + // Should trigger onChange, not postMessage + expect(onChangeMock).toHaveBeenCalledWith("settings-action") + expect(postMessageMock).not.toHaveBeenCalled() + + // 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() + + // Get all dropdown items + const dropdownItems = screen.getAllByTestId("dropdown-item") + + // Click the second option (index 1) + fireEvent.click(dropdownItems[1]) + + // Check that onChange was called with the correct value + expect(onChangeMock).toHaveBeenCalledWith("option2") + + // 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 ed67f8922a..b134894d25 100644 --- a/webview-ui/src/components/ui/select-dropdown.tsx +++ b/webview-ui/src/components/ui/select-dropdown.tsx @@ -8,10 +8,18 @@ import { } from "./dropdown-menu" import { cn } from "@/lib/utils" +// Constants for option types +export enum DropdownOptionType { + ITEM = "item", + SEPARATOR = "separator", + SHORTCUT = "shortcut", + ACTION = "action", +} export interface DropdownOption { value: string label: string disabled?: boolean + type?: DropdownOptionType // Optional type to specify special behaviors } export interface SelectDropdownProps { @@ -54,13 +62,16 @@ export const SelectDropdown = React.forwardRef { - if (optionValue.endsWith("-action")) { - // Handle special actions like "settings-action" or "prompts-action" - window.postMessage({ type: "action", action: optionValue.replace("-action", "ButtonClicked") }) + const handleSelect = (option: DropdownOption) => { + // Check if this is an action option by its explicit type + if (option.type === DropdownOptionType.ACTION) { + window.postMessage({ + type: "action", + action: option.value, + }) return } - onChange(optionValue) + onChange(option.value) } return ( @@ -76,7 +87,7 @@ export const SelectDropdown = React.forwardRef @@ -106,22 +117,24 @@ export const SelectDropdown = React.forwardRef {options.map((option, index) => { - // Check if this is a separator (typically used for the "────" option) - if (option.label.includes("────")) { + // Handle separator type + if (option.type === DropdownOptionType.SEPARATOR || option.label.includes("────")) { return } - // Check if this is a disabled label (like the shortcut text) - if (option.disabled && shortcutText && option.label.includes(shortcutText)) { + // Handle shortcut text type (disabled label for keyboard shortcuts) + if ( + option.type === DropdownOptionType.SHORTCUT || + (option.disabled && shortcutText && option.label.includes(shortcutText)) + ) { return ( -
+
{option.label}
) } + // Regular menu items return ( handleSelect(option.value)}> + onClick={() => handleSelect(option)}> {option.label} )