From c84b6398728ff82f006e4a0107dac2301337ffed Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 2 Mar 2025 22:14:32 -0500 Subject: [PATCH 1/2] Custom dropdowns for mode/api profile --- .../src/components/chat/ChatTextArea.tsx | 148 ++++++------------ .../src/components/common/CaretIcon.tsx | 15 -- .../ui/__tests__/select-dropdown.test.tsx | 110 +++++++++++++ webview-ui/src/components/ui/index.ts | 1 + .../src/components/ui/select-dropdown.tsx | 144 +++++++++++++++++ 5 files changed, 301 insertions(+), 117 deletions(-) delete mode 100644 webview-ui/src/components/common/CaretIcon.tsx create mode 100644 webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx create mode 100644 webview-ui/src/components/ui/select-dropdown.tsx diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index a2af6a3a71..51162d7b86 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/webview-ui/src/components/chat/ChatTextArea.tsx @@ -15,8 +15,8 @@ import Thumbnails from "../common/Thumbnails" import { vscode } from "../../utils/vscode" import { WebviewMessage } from "../../../../src/shared/WebviewMessage" import { Mode, getAllModes } from "../../../../src/shared/modes" -import { CaretIcon } from "../common/CaretIcon" import { convertToMentionPath } from "../../utils/path-mentions" +import { SelectDropdown } from "../ui" interface ChatTextAreaProps { inputValue: string @@ -541,35 +541,6 @@ const ChatTextArea = forwardRef( [updateCursorPosition], ) - const selectStyle = { - fontSize: "11px", - cursor: textAreaDisabled ? "not-allowed" : "pointer", - backgroundColor: "transparent", - border: "none", - color: "var(--vscode-foreground)", - opacity: textAreaDisabled ? 0.5 : 0.8, - outline: "none", - paddingLeft: "20px", - paddingRight: "6px", - WebkitAppearance: "none" as const, - MozAppearance: "none" as const, - appearance: "none" as const, - } - - const optionStyle = { - backgroundColor: "var(--vscode-dropdown-background)", - color: "var(--vscode-dropdown-foreground)", - } - - const caretContainerStyle = { - position: "absolute" as const, - left: 6, - top: "50%", - transform: "translateY(-45%)", - pointerEvents: "none" as const, - opacity: textAreaDisabled ? 0.5 : 0.8, - } - return (
( marginTop: "auto", paddingTop: "2px", }}> + {/* Left side - dropdowns container */}
-
- -
- -
+ shortcutText={modeShortcutText} + triggerClassName="w-full" + />
+ {/* API configuration selector - flexible width */}
- -
- -
+ contentClassName="max-h-[300px] overflow-y-auto" + triggerClassName="w-full text-ellipsis overflow-hidden" + />
+ {/* Right side - action buttons */}
{isEnhancingPrompt ? ( @@ -916,7 +860,7 @@ const ChatTextArea = forwardRef( color: "var(--vscode-input-foreground)", opacity: 0.5, fontSize: 16.5, - marginRight: 10, + marginRight: 6, // Reduced from 10 }} /> ) : ( diff --git a/webview-ui/src/components/common/CaretIcon.tsx b/webview-ui/src/components/common/CaretIcon.tsx deleted file mode 100644 index 22ff52b81e..0000000000 --- a/webview-ui/src/components/common/CaretIcon.tsx +++ /dev/null @@ -1,15 +0,0 @@ -import React from "react" - -export const CaretIcon = () => ( - - - -) diff --git a/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx b/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx new file mode 100644 index 0000000000..89f8ddc3c2 --- /dev/null +++ b/webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx @@ -0,0 +1,110 @@ +import React, { ReactNode } from "react" +import { render, screen } from "@testing-library/react" +import { SelectDropdown } from "../select-dropdown" + +// Mock the Radix UI DropdownMenu component and its children +jest.mock("../dropdown-menu", () => { + return { + DropdownMenu: ({ children }: { children: ReactNode }) =>
{children}
, + + DropdownMenuTrigger: ({ + children, + disabled, + ...props + }: { + children: ReactNode + disabled?: boolean + [key: string]: any + }) => ( + + ), + + DropdownMenuContent: ({ children }: { children: ReactNode }) => ( +
{children}
+ ), + + DropdownMenuItem: ({ + children, + onClick, + disabled, + }: { + children: ReactNode + onClick?: () => void + disabled?: boolean + }) => ( +
+ {children} +
+ ), + + DropdownMenuSeparator: () =>
, + } +}) + +describe("SelectDropdown", () => { + const options = [ + { value: "option1", label: "Option 1" }, + { value: "option2", label: "Option 2" }, + { value: "option3", label: "Option 3" }, + { value: "sep-1", label: "────", disabled: true }, + { value: "action", label: "Action Item" }, + ] + + const onChangeMock = jest.fn() + + beforeEach(() => { + jest.clearAllMocks() + }) + + it("renders correctly with default props", () => { + render() + + // Check that the selected option is displayed in the trigger, not in a menu item + const trigger = screen.getByTestId("dropdown-trigger") + expect(trigger).toHaveTextContent("Option 1") + }) + + it("handles disabled state correctly", () => { + render() + + const trigger = screen.getByTestId("dropdown-trigger") + expect(trigger).toHaveAttribute("disabled") + }) + + it("renders with width: 100% for proper sizing", () => { + render() + + const trigger = screen.getByTestId("dropdown-trigger") + expect(trigger).toHaveStyle("width: 100%") + }) + + it("passes the selected value to the trigger", () => { + const { rerender } = render() + + // Check initial render using testId to be specific + const trigger = screen.getByTestId("dropdown-trigger") + expect(trigger).toHaveTextContent("Option 1") + + // Rerender with a different value + rerender() + + // Check updated render + expect(trigger).toHaveTextContent("Option 3") + }) + + it("applies custom className to trigger when provided", () => { + render( + , + ) + + const trigger = screen.getByTestId("dropdown-trigger") + expect(trigger.classList.toString()).toContain("custom-trigger-class") + }) +}) diff --git a/webview-ui/src/components/ui/index.ts b/webview-ui/src/components/ui/index.ts index 6eb8dd25ba..b444b37788 100644 --- a/webview-ui/src/components/ui/index.ts +++ b/webview-ui/src/components/ui/index.ts @@ -13,3 +13,4 @@ export * from "./separator" export * from "./slider" export * from "./textarea" export * from "./tooltip" +export * from "./select-dropdown" diff --git a/webview-ui/src/components/ui/select-dropdown.tsx b/webview-ui/src/components/ui/select-dropdown.tsx new file mode 100644 index 0000000000..ed67f8922a --- /dev/null +++ b/webview-ui/src/components/ui/select-dropdown.tsx @@ -0,0 +1,144 @@ +import * as React from "react" +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, + DropdownMenuSeparator, +} from "./dropdown-menu" +import { cn } from "@/lib/utils" + +export interface DropdownOption { + value: string + label: string + disabled?: boolean +} + +export interface SelectDropdownProps { + value: string + options: DropdownOption[] + onChange: (value: string) => void + disabled?: boolean + title?: string + className?: string + triggerClassName?: string + contentClassName?: string + sideOffset?: number + align?: "start" | "center" | "end" + shouldShowCaret?: boolean + placeholder?: string + shortcutText?: string +} + +export const SelectDropdown = React.forwardRef, SelectDropdownProps>( + ( + { + value, + options, + onChange, + disabled = false, + title = "", + className = "", + triggerClassName = "", + contentClassName = "", + sideOffset = 4, + align = "start", + shouldShowCaret = true, + placeholder = "", + shortcutText = "", + }, + ref, + ) => { + // Find the selected option label + const selectedOption = options.find((option) => option.value === value) + const displayText = selectedOption?.label || placeholder || "" + + // Handle menu item click + const handleSelect = (optionValue: string) => { + if (optionValue.endsWith("-action")) { + // Handle special actions like "settings-action" or "prompts-action" + window.postMessage({ type: "action", action: optionValue.replace("-action", "ButtonClicked") }) + return + } + onChange(optionValue) + } + + return ( + + + {shouldShowCaret && ( +
+ + + +
+ )} + {displayText} +
+ + + {options.map((option, index) => { + // Check if this is a separator (typically used for the "────" option) + if (option.label.includes("────")) { + return + } + + // Check if this is a disabled label (like the shortcut text) + if (option.disabled && shortcutText && option.label.includes(shortcutText)) { + return ( +
+ {option.label} +
+ ) + } + + return ( + handleSelect(option.value)}> + {option.label} + + ) + })} +
+
+ ) + }, +) + +SelectDropdown.displayName = "SelectDropdown" From 9c3e477bf26c6a5acabbed39f0b8ac127a736fff Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Mon, 3 Mar 2025 21:53:49 -0500 Subject: [PATCH 2/2] PR cleanup --- .../src/components/chat/ChatTextArea.tsx | 47 ++++-- .../ui/__tests__/select-dropdown.test.tsx | 140 +++++++++++++++++- .../src/components/ui/select-dropdown.tsx | 41 +++-- 3 files changed, 196 insertions(+), 32 deletions(-) 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} )