Skip to content

Commit 73570e0

Browse files
committed
refactor: improve ModeSelector based on PR review feedback
- Extract magic number 6 to SEARCH_THRESHOLD constant for clarity - Add optional disableSearch prop for consistency with SelectDropdown - Use showSearch variable to control search visibility logic - Add comprehensive tests for disableSearch prop behavior - Fix redundant tooltip when search is hidden These changes improve maintainability and provide more flexible control over the search functionality while maintaining backward compatibility.
1 parent 4ea6164 commit 73570e0

File tree

2 files changed

+63
-3
lines changed

2 files changed

+63
-3
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import { telemetryClient } from "@/utils/TelemetryClient"
1313
import { TelemetryEventName } from "@roo-code/types"
1414
import { Fzf } from "fzf"
1515

16+
// Minimum number of modes required to show search functionality
17+
const SEARCH_THRESHOLD = 6
18+
1619
interface ModeSelectorProps {
1720
value: Mode
1821
onChange: (value: Mode) => void
@@ -22,6 +25,7 @@ interface ModeSelectorProps {
2225
modeShortcutText: string
2326
customModes?: ModeConfig[]
2427
customModePrompts?: CustomModePrompts
28+
disableSearch?: boolean
2529
}
2630

2731
export const ModeSelector = ({
@@ -33,6 +37,7 @@ export const ModeSelector = ({
3337
modeShortcutText,
3438
customModes,
3539
customModePrompts,
40+
disableSearch = false,
3641
}: ModeSelectorProps) => {
3742
const [open, setOpen] = React.useState(false)
3843
const [searchValue, setSearchValue] = React.useState("")
@@ -148,6 +153,9 @@ export const ModeSelector = ({
148153
}
149154
}, [open])
150155

156+
// Determine if search should be shown
157+
const showSearch = !disableSearch && modes.length > SEARCH_THRESHOLD
158+
151159
// Combine instruction text for tooltip
152160
const instructionText = `${t("chat:modeSelector.description")} ${modeShortcutText}`
153161

@@ -182,8 +190,8 @@ export const ModeSelector = ({
182190
container={portalContainer}
183191
className="p-0 overflow-hidden min-w-80 max-w-9/10">
184192
<div className="flex flex-col w-full">
185-
{/* Show search bar only when there are more than 6 items, otherwise show info blurb */}
186-
{modes.length > 6 ? (
193+
{/* Show search bar only when there are more than SEARCH_THRESHOLD items, otherwise show info blurb */}
194+
{showSearch ? (
187195
<div className="relative p-2 border-b border-vscode-dropdown-border">
188196
<input
189197
aria-label="Search modes"
@@ -277,7 +285,7 @@ export const ModeSelector = ({
277285

278286
{/* Info icon and title on the right - only show info icon when search bar is visible */}
279287
<div className="flex items-center gap-1 pr-1">
280-
{modes.length > 6 && (
288+
{showSearch && (
281289
<StandardTooltip content={instructionText}>
282290
<span className="codicon codicon-info text-xs text-vscode-descriptionForeground opacity-70 hover:opacity-100 cursor-help" />
283291
</StandardTooltip>

webview-ui/src/components/chat/__tests__/ModeSelector.spec.tsx

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,4 +147,56 @@ describe("ModeSelector", () => {
147147
const modeItems = screen.getAllByTestId("mode-selector-item")
148148
expect(modeItems.length).toBeLessThan(7) // Should have filtered some out
149149
})
150+
151+
test("respects disableSearch prop even when there are more than 6 modes", () => {
152+
// Set up mock to return 10 modes
153+
mockModes = Array.from({ length: 10 }, (_, i) => ({
154+
slug: `mode-${i}`,
155+
name: `Mode ${i}`,
156+
description: `Description for mode ${i}`,
157+
roleDefinition: "Role definition",
158+
groups: ["read", "edit"],
159+
}))
160+
161+
render(
162+
<ModeSelector value={"mode-0" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" disableSearch={true} />,
163+
)
164+
165+
// Click to open the popover
166+
fireEvent.click(screen.getByTestId("mode-selector-trigger"))
167+
168+
// Search input should NOT be visible even with 10 modes
169+
expect(screen.queryByTestId("mode-search-input")).not.toBeInTheDocument()
170+
171+
// Info blurb should be visible instead
172+
expect(screen.getByText(/chat:modeSelector.description/)).toBeInTheDocument()
173+
174+
// Info icon should NOT be visible
175+
const infoIcon = document.querySelector(".codicon-info")
176+
expect(infoIcon).not.toBeInTheDocument()
177+
})
178+
179+
test("shows search when disableSearch is false (default) and modes > 6", () => {
180+
// Set up mock to return 8 modes
181+
mockModes = Array.from({ length: 8 }, (_, i) => ({
182+
slug: `mode-${i}`,
183+
name: `Mode ${i}`,
184+
description: `Description for mode ${i}`,
185+
roleDefinition: "Role definition",
186+
groups: ["read", "edit"],
187+
}))
188+
189+
// Don't pass disableSearch prop (should default to false)
190+
render(<ModeSelector value={"mode-0" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
191+
192+
// Click to open the popover
193+
fireEvent.click(screen.getByTestId("mode-selector-trigger"))
194+
195+
// Search input should be visible
196+
expect(screen.getByTestId("mode-search-input")).toBeInTheDocument()
197+
198+
// Info icon should be visible
199+
const infoIcon = document.querySelector(".codicon-info")
200+
expect(infoIcon).toBeInTheDocument()
201+
})
150202
})

0 commit comments

Comments
 (0)