Skip to content

Commit 9c3e477

Browse files
committed
PR cleanup
1 parent c84b639 commit 9c3e477

File tree

3 files changed

+196
-32
lines changed

3 files changed

+196
-32
lines changed

webview-ui/src/components/chat/ChatTextArea.tsx

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { vscode } from "../../utils/vscode"
1616
import { WebviewMessage } from "../../../../src/shared/WebviewMessage"
1717
import { Mode, getAllModes } from "../../../../src/shared/modes"
1818
import { convertToMentionPath } from "../../utils/path-mentions"
19-
import { SelectDropdown } from "../ui"
19+
import { SelectDropdown, DropdownOptionType } from "../ui"
2020

2121
interface ChatTextAreaProps {
2222
inputValue: string
@@ -779,22 +779,32 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
779779
title="Select mode for interaction"
780780
options={[
781781
// Add the shortcut text as a disabled option at the top
782-
{ value: "shortcut", label: modeShortcutText, disabled: true },
782+
{
783+
value: "shortcut",
784+
label: modeShortcutText,
785+
disabled: true,
786+
type: DropdownOptionType.SHORTCUT,
787+
},
783788
// Add all modes
784789
...getAllModes(customModes).map((mode) => ({
785790
value: mode.slug,
786791
label: mode.name,
792+
type: DropdownOptionType.ITEM,
787793
})),
788794
// Add separator
789-
{ value: "sep-1", label: "────", disabled: true },
795+
{
796+
value: "sep-1",
797+
label: "Separator",
798+
type: DropdownOptionType.SEPARATOR,
799+
},
790800
// Add Edit option
791-
{ value: "prompts-action", label: "Edit..." },
801+
{
802+
value: "promptsButtonClicked",
803+
label: "Edit...",
804+
type: DropdownOptionType.ACTION,
805+
},
792806
]}
793807
onChange={(value) => {
794-
if (value === "prompts-action") {
795-
window.postMessage({ type: "action", action: "promptsButtonClicked" })
796-
return
797-
}
798808
setMode(value as Mode)
799809
vscode.postMessage({
800810
type: "mode",
@@ -822,17 +832,22 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
822832
...(listApiConfigMeta || []).map((config) => ({
823833
value: config.name,
824834
label: config.name,
835+
type: DropdownOptionType.ITEM,
825836
})),
826837
// Add separator
827-
{ value: "sep-1", label: "────", disabled: true },
838+
{
839+
value: "sep-2",
840+
label: "Separator",
841+
type: DropdownOptionType.SEPARATOR,
842+
},
828843
// Add Edit option
829-
{ value: "settings-action", label: "Edit..." },
844+
{
845+
value: "settingsButtonClicked",
846+
label: "Edit...",
847+
type: DropdownOptionType.ACTION,
848+
},
830849
]}
831850
onChange={(value) => {
832-
if (value === "settings-action") {
833-
window.postMessage({ type: "action", action: "settingsButtonClicked" })
834-
return
835-
}
836851
vscode.postMessage({
837852
type: "loadApiConfiguration",
838853
text: value,
@@ -849,7 +864,7 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
849864
style={{
850865
display: "flex",
851866
alignItems: "center",
852-
gap: "8px", // Reduced from 12px
867+
gap: "8px",
853868
flexShrink: 0,
854869
}}>
855870
<div style={{ display: "flex", alignItems: "center" }}>
@@ -860,7 +875,7 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
860875
color: "var(--vscode-input-foreground)",
861876
opacity: 0.5,
862877
fontSize: 16.5,
863-
marginRight: 6, // Reduced from 10
878+
marginRight: 6,
864879
}}
865880
/>
866881
) : (

webview-ui/src/components/ui/__tests__/select-dropdown.test.tsx

Lines changed: 138 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
import React, { ReactNode } from "react"
2-
import { render, screen } from "@testing-library/react"
3-
import { SelectDropdown } from "../select-dropdown"
2+
import { render, screen, fireEvent } from "@testing-library/react"
3+
import { SelectDropdown, DropdownOptionType } from "../select-dropdown"
4+
5+
// Mock window.postMessage
6+
const postMessageMock = jest.fn()
7+
Object.defineProperty(window, "postMessage", {
8+
writable: true,
9+
value: postMessageMock,
10+
})
411

512
// Mock the Radix UI DropdownMenu component and its children
613
jest.mock("../dropdown-menu", () => {
@@ -107,4 +114,133 @@ describe("SelectDropdown", () => {
107114
const trigger = screen.getByTestId("dropdown-trigger")
108115
expect(trigger.classList.toString()).toContain("custom-trigger-class")
109116
})
117+
118+
// Tests for the new functionality
119+
describe("Option types", () => {
120+
it("renders separator options correctly", () => {
121+
const optionsWithTypedSeparator = [
122+
{ value: "option1", label: "Option 1" },
123+
{ value: "sep-1", label: "Separator", type: DropdownOptionType.SEPARATOR },
124+
{ value: "option2", label: "Option 2" },
125+
]
126+
127+
render(<SelectDropdown value="option1" options={optionsWithTypedSeparator} onChange={onChangeMock} />)
128+
129+
// Check for separator
130+
const separators = screen.getAllByTestId("dropdown-separator")
131+
expect(separators.length).toBe(1)
132+
})
133+
134+
it("renders string separator (backward compatibility) correctly", () => {
135+
const optionsWithStringSeparator = [
136+
{ value: "option1", label: "Option 1" },
137+
{ value: "sep-1", label: "────", disabled: true },
138+
{ value: "option2", label: "Option 2" },
139+
]
140+
141+
render(<SelectDropdown value="option1" options={optionsWithStringSeparator} onChange={onChangeMock} />)
142+
143+
// Check for separator
144+
const separators = screen.getAllByTestId("dropdown-separator")
145+
expect(separators.length).toBe(1)
146+
})
147+
148+
it("renders shortcut options correctly", () => {
149+
const shortcutText = "Ctrl+K"
150+
const optionsWithShortcut = [
151+
{ value: "shortcut", label: shortcutText, type: DropdownOptionType.SHORTCUT },
152+
{ value: "option1", label: "Option 1" },
153+
]
154+
155+
render(
156+
<SelectDropdown
157+
value="option1"
158+
options={optionsWithShortcut}
159+
onChange={onChangeMock}
160+
shortcutText={shortcutText}
161+
/>,
162+
)
163+
164+
// The shortcut text should be rendered as a div, not a dropdown item
165+
expect(screen.queryByText(shortcutText)).toBeInTheDocument()
166+
const dropdownItems = screen.getAllByTestId("dropdown-item")
167+
expect(dropdownItems.length).toBe(1) // Only one regular option
168+
})
169+
170+
it("handles action options correctly", () => {
171+
const optionsWithAction = [
172+
{ value: "option1", label: "Option 1" },
173+
{ value: "settingsButtonClicked", label: "Settings", type: DropdownOptionType.ACTION },
174+
]
175+
176+
render(<SelectDropdown value="option1" options={optionsWithAction} onChange={onChangeMock} />)
177+
178+
// Get all dropdown items
179+
const dropdownItems = screen.getAllByTestId("dropdown-item")
180+
181+
// Click the action item
182+
fireEvent.click(dropdownItems[1])
183+
184+
// Check that postMessage was called with the correct action
185+
expect(postMessageMock).toHaveBeenCalledWith({
186+
type: "action",
187+
action: "settingsButtonClicked",
188+
})
189+
190+
// The onChange callback should not be called for action items
191+
expect(onChangeMock).not.toHaveBeenCalled()
192+
})
193+
194+
it("only treats options with explicit ACTION type as actions", () => {
195+
const optionsForTest = [
196+
{ value: "option1", label: "Option 1" },
197+
// This should be treated as a regular option despite the -action suffix
198+
{ value: "settings-action", label: "Regular option with action suffix" },
199+
// This should be treated as an action
200+
{ value: "settingsButtonClicked", label: "Settings", type: DropdownOptionType.ACTION },
201+
]
202+
203+
render(<SelectDropdown value="option1" options={optionsForTest} onChange={onChangeMock} />)
204+
205+
// Get all dropdown items
206+
const dropdownItems = screen.getAllByTestId("dropdown-item")
207+
208+
// Click the second option (with action suffix but no ACTION type)
209+
fireEvent.click(dropdownItems[1])
210+
211+
// Should trigger onChange, not postMessage
212+
expect(onChangeMock).toHaveBeenCalledWith("settings-action")
213+
expect(postMessageMock).not.toHaveBeenCalled()
214+
215+
// Reset mocks
216+
onChangeMock.mockReset()
217+
postMessageMock.mockReset()
218+
219+
// Click the third option (ACTION type)
220+
fireEvent.click(dropdownItems[2])
221+
222+
// Should trigger postMessage with "settingsButtonClicked", not onChange
223+
expect(postMessageMock).toHaveBeenCalledWith({
224+
type: "action",
225+
action: "settingsButtonClicked",
226+
})
227+
expect(onChangeMock).not.toHaveBeenCalled()
228+
})
229+
230+
it("calls onChange for regular menu items", () => {
231+
render(<SelectDropdown value="option1" options={options} onChange={onChangeMock} />)
232+
233+
// Get all dropdown items
234+
const dropdownItems = screen.getAllByTestId("dropdown-item")
235+
236+
// Click the second option (index 1)
237+
fireEvent.click(dropdownItems[1])
238+
239+
// Check that onChange was called with the correct value
240+
expect(onChangeMock).toHaveBeenCalledWith("option2")
241+
242+
// postMessage should not be called for regular items
243+
expect(postMessageMock).not.toHaveBeenCalled()
244+
})
245+
})
110246
})

webview-ui/src/components/ui/select-dropdown.tsx

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,18 @@ import {
88
} from "./dropdown-menu"
99
import { cn } from "@/lib/utils"
1010

11+
// Constants for option types
12+
export enum DropdownOptionType {
13+
ITEM = "item",
14+
SEPARATOR = "separator",
15+
SHORTCUT = "shortcut",
16+
ACTION = "action",
17+
}
1118
export interface DropdownOption {
1219
value: string
1320
label: string
1421
disabled?: boolean
22+
type?: DropdownOptionType // Optional type to specify special behaviors
1523
}
1624

1725
export interface SelectDropdownProps {
@@ -54,13 +62,16 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
5462
const displayText = selectedOption?.label || placeholder || ""
5563

5664
// Handle menu item click
57-
const handleSelect = (optionValue: string) => {
58-
if (optionValue.endsWith("-action")) {
59-
// Handle special actions like "settings-action" or "prompts-action"
60-
window.postMessage({ type: "action", action: optionValue.replace("-action", "ButtonClicked") })
65+
const handleSelect = (option: DropdownOption) => {
66+
// Check if this is an action option by its explicit type
67+
if (option.type === DropdownOptionType.ACTION) {
68+
window.postMessage({
69+
type: "action",
70+
action: option.value,
71+
})
6172
return
6273
}
63-
onChange(optionValue)
74+
onChange(option.value)
6475
}
6576

6677
return (
@@ -76,7 +87,7 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
7687
triggerClassName,
7788
)}
7889
style={{
79-
width: "100%", // Changed to take full width of parent
90+
width: "100%", // Take full width of parent
8091
minWidth: "0",
8192
maxWidth: "100%",
8293
}}>
@@ -106,22 +117,24 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
106117
contentClassName,
107118
)}>
108119
{options.map((option, index) => {
109-
// Check if this is a separator (typically used for the "────" option)
110-
if (option.label.includes("────")) {
120+
// Handle separator type
121+
if (option.type === DropdownOptionType.SEPARATOR || option.label.includes("────")) {
111122
return <DropdownMenuSeparator key={`sep-${index}`} />
112123
}
113124

114-
// Check if this is a disabled label (like the shortcut text)
115-
if (option.disabled && shortcutText && option.label.includes(shortcutText)) {
125+
// Handle shortcut text type (disabled label for keyboard shortcuts)
126+
if (
127+
option.type === DropdownOptionType.SHORTCUT ||
128+
(option.disabled && shortcutText && option.label.includes(shortcutText))
129+
) {
116130
return (
117-
<div
118-
key={`label-${index}`}
119-
className="px-2 py-1.5 text-xs font-semibold opacity-60 italic">
131+
<div key={`label-${index}`} className="px-2 py-1.5 text-xs opacity-50">
120132
{option.label}
121133
</div>
122134
)
123135
}
124136

137+
// Regular menu items
125138
return (
126139
<DropdownMenuItem
127140
key={`item-${option.value}`}
@@ -130,7 +143,7 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
130143
"cursor-pointer text-xs focus:bg-vscode-list-hoverBackground focus:text-vscode-list-hoverForeground",
131144
option.value === value && "bg-vscode-list-focusBackground",
132145
)}
133-
onClick={() => handleSelect(option.value)}>
146+
onClick={() => handleSelect(option)}>
134147
{option.label}
135148
</DropdownMenuItem>
136149
)

0 commit comments

Comments
 (0)