PR cleanup

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<HTMLTextAreaElement, ChatTextAreaProps>(
 								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<HTMLTextAreaElement, ChatTextAreaProps>(
 									...(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<HTMLTextAreaElement, ChatTextAreaProps>(
 						style={{
 							display: "flex",
 							alignItems: "center",
-							gap: "8px", // Reduced from 12px
+							gap: "8px",
 							flexShrink: 0,
 						}}>
 						<div style={{ display: "flex", alignItems: "center" }}>
@@ -860,7 +875,7 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
 										color: "var(--vscode-input-foreground)",
 										opacity: 0.5,
 										fontSize: 16.5,
-										marginRight: 6, // Reduced from 10
+										marginRight: 6,
 									}}
 								/>
 							) : (

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(<SelectDropdown value="option1" options={optionsWithTypedSeparator} onChange={onChangeMock} />)
+
+			// 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(<SelectDropdown value="option1" options={optionsWithStringSeparator} onChange={onChangeMock} />)
+
+			// 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(
+				<SelectDropdown
+					value="option1"
+					options={optionsWithShortcut}
+					onChange={onChangeMock}
+					shortcutText={shortcutText}
+				/>,
+			)
+
+			// 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(<SelectDropdown value="option1" options={optionsWithAction} onChange={onChangeMock} />)
+
+			// 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(<SelectDropdown value="option1" options={optionsForTest} onChange={onChangeMock} />)
+
+			// 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(<SelectDropdown value="option1" options={options} onChange={onChangeMock} />)
+
+			// 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()
+		})
+	})
 })

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<React.ElementRef<typeof DropdownM
 		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") })
+		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<React.ElementRef<typeof DropdownM
 						triggerClassName,
 					)}
 					style={{
-						width: "100%", // Changed to take full width of parent
+						width: "100%", // Take full width of parent
 						minWidth: "0",
 						maxWidth: "100%",
 					}}>
@@ -106,22 +117,24 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
 						contentClassName,
 					)}>
 					{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 <DropdownMenuSeparator key={`sep-${index}`} />
 						}
 
-						// 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 (
-								<div
-									key={`label-${index}`}
-									className="px-2 py-1.5 text-xs font-semibold opacity-60 italic">
+								<div key={`label-${index}`} className="px-2 py-1.5 text-xs opacity-50">
 									{option.label}
 								</div>
 							)
 						}
 
+						// Regular menu items
 						return (
 							<DropdownMenuItem
 								key={`item-${option.value}`}
@@ -130,7 +143,7 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
 									"cursor-pointer text-xs focus:bg-vscode-list-hoverBackground focus:text-vscode-list-hoverForeground",
 									option.value === value && "bg-vscode-list-focusBackground",
 								)}
-								onClick={() => handleSelect(option.value)}>
+								onClick={() => handleSelect(option)}>
 								{option.label}
 							</DropdownMenuItem>
 						)