Skip to content

Commit

Permalink
PR cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrubens
mrubens committed Mar 4, 2025
1 parent c84b639 commit 084ea26
Show file tree
Hide file tree
Showing 3 changed files with 196 additions and 32 deletions.
47 changes: 31 additions & 16 deletions webview-ui/src/components/chat/ChatTextArea.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -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" }}>
Expand All @@ -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,
}}
/>
) : (
Expand Down
140 changes: 138 additions & 2 deletions webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -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: "settings", 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: "settings", 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()
})
})
})
41 changes: 27 additions & 14 deletions webview-ui/src/components/ui/select-dropdown.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 (
Expand All @@ -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%",
}}>
Expand Down Expand Up @@ -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}`}
Expand All @@ -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>
)
Expand Down

0 comments on commit 084ea26

Please sign in to comment.