Skip to content

Commit 52bf3c3

Browse files
committed
feat(ui): apply Bruno feedback — simplify Chat dropdown (remove sort controls, prefer saved custom order); Settings always-on reordering with Reset to Alphabetical; update tests; fix lint
1 parent 638e912 commit 52bf3c3

File tree

5 files changed

+32
-210
lines changed

5 files changed

+32
-210
lines changed

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

Lines changed: 13 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ const ConfigItem = ({ config, isPinned, index, value, onSelect, togglePinnedApiC
8888
)
8989
}
9090

91-
type SortMode = "alphabetical" | "custom"
92-
9391
interface ApiConfigSelectorProps {
9492
value: string
9593
displayName: string
@@ -100,8 +98,6 @@ interface ApiConfigSelectorProps {
10098
listApiConfigMeta: Array<{ id: string; name: string; modelId?: string }>
10199
pinnedApiConfigs?: Record<string, boolean>
102100
togglePinnedApiConfig: (id: string) => void
103-
onSortModeChange?: (mode: SortMode) => void
104-
onCustomOrderChange?: (order: Array<{ id: string; index: number; pinned: boolean }>) => void
105101
}
106102

107103
export const ApiConfigSelector = ({
@@ -114,52 +110,33 @@ export const ApiConfigSelector = ({
114110
listApiConfigMeta,
115111
pinnedApiConfigs,
116112
togglePinnedApiConfig,
117-
onSortModeChange,
118-
onCustomOrderChange,
119113
}: ApiConfigSelectorProps) => {
120114
const { t } = useAppTranslation()
121115
const { apiConfigsCustomOrder: customOrder = [] } = useExtensionState()
122116
const [open, setOpen] = useState(false)
123117
const [searchValue, setSearchValue] = useState("")
124118

125-
const [sortMode, setSortMode] = useState<SortMode>("alphabetical")
119+
// No explicit sort mode in Chat; use custom order if available, otherwise alphabetical
126120

127121
const portalContainer = useRooPortal("roo-portal")
128122

129-
// Sort configs based on sort mode
123+
// Sort configs: prefer saved custom order; otherwise alphabetical
130124
const sortedConfigs = useMemo(() => {
131125
const sorted = [...listApiConfigMeta]
132126

133-
switch (sortMode) {
134-
case "alphabetical":
135-
sorted.sort((a, b) => a.name.localeCompare(b.name))
136-
break
137-
case "custom":
138-
if (customOrder && customOrder.length > 0) {
139-
// Sort by custom order, with unordered items at the end
140-
const orderMap = new Map(customOrder.map((item) => [item.id, item.index]))
141-
sorted.sort((a, b) => {
142-
const aIndex = orderMap.get(a.id) ?? Number.MAX_SAFE_INTEGER
143-
const bIndex = orderMap.get(b.id) ?? Number.MAX_SAFE_INTEGER
144-
return (aIndex as number) - (bIndex as number)
145-
})
146-
}
147-
break
127+
if (customOrder && customOrder.length > 0) {
128+
const orderMap = new Map(customOrder.map((item) => [item.id, item.index]))
129+
sorted.sort((a, b) => {
130+
const aIndex = orderMap.get(a.id) ?? Number.MAX_SAFE_INTEGER
131+
const bIndex = orderMap.get(b.id) ?? Number.MAX_SAFE_INTEGER
132+
return (aIndex as number) - (bIndex as number)
133+
})
134+
} else {
135+
sorted.sort((a, b) => a.name.localeCompare(b.name))
148136
}
149137

150138
return sorted
151-
}, [listApiConfigMeta, sortMode, customOrder])
152-
153-
// Current visible order for callbacks when switching to custom
154-
const currentOrder = useMemo(
155-
() =>
156-
sortedConfigs.map((config, index) => ({
157-
id: config.id,
158-
index,
159-
pinned: Boolean(pinnedApiConfigs?.[config.id]),
160-
})),
161-
[sortedConfigs, pinnedApiConfigs],
162-
)
139+
}, [listApiConfigMeta, customOrder])
163140

164141
// Filter configs based on search.
165142
const filteredConfigs = useMemo(() => {
@@ -193,22 +170,6 @@ export const ApiConfigSelector = ({
193170
setOpen(false)
194171
}, [])
195172

196-
const handleSortModeChange = useCallback(
197-
(mode: SortMode) => {
198-
setSortMode(mode)
199-
onSortModeChange?.(mode)
200-
if (mode === "custom") {
201-
// Persist current visible order as custom order baseline
202-
vscode.postMessage({
203-
type: "setApiConfigsCustomOrder",
204-
values: { customOrder: currentOrder },
205-
})
206-
onCustomOrderChange?.(currentOrder)
207-
}
208-
},
209-
[onSortModeChange, onCustomOrderChange, currentOrder],
210-
)
211-
212173
return (
213174
<Popover open={open} onOpenChange={setOpen} data-testid="api-config-selector-root">
214175
<StandardTooltip content={title}>
@@ -312,34 +273,7 @@ export const ApiConfigSelector = ({
312273

313274
{/* Bottom bar with controls */}
314275
<div className="flex flex-col border-t border-vscode-dropdown-border">
315-
{/* Sort controls */}
316-
<div className="flex items-center justify-between px-2 py-1.5 border-b border-vscode-dropdown-border">
317-
<div className="flex items-center gap-2">
318-
<span className="text-xs text-vscode-descriptionForeground">
319-
{t("chat:apiConfigSelector.sort")}
320-
</span>
321-
<div className="flex items-center gap-1">
322-
{(["alphabetical", "custom"] as const).map((mode) => (
323-
<Button
324-
key={mode}
325-
variant="ghost"
326-
size="sm"
327-
aria-label={`${t("chat:apiConfigSelector.sort")} ${mode === "alphabetical" ? t("chat:apiConfigSelector.alphabetical") : t("chat:apiConfigSelector.custom")}`}
328-
aria-pressed={sortMode === mode}
329-
onClick={() => handleSortModeChange(mode)}
330-
className={cn(
331-
"h-6 px-2 text-xs",
332-
sortMode === mode &&
333-
"bg-vscode-button-background text-vscode-button-foreground",
334-
)}>
335-
{mode === "alphabetical"
336-
? t("chat:apiConfigSelector.alphabetical")
337-
: t("chat:apiConfigSelector.custom")}
338-
</Button>
339-
))}
340-
</div>
341-
</div>
342-
</div>
276+
{/* No sort controls in Chat dropdown per design feedback */}
343277

344278
{/* Bottom bar with settings and title */}
345279
<div className="flex flex-row items-center justify-between px-2 py-2">

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

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -436,60 +436,6 @@ describe("ApiConfigSelector", () => {
436436
expect(searchInput.value).toBe("Config")
437437
})
438438

439-
test("calls onSortModeChange when switching sort mode", () => {
440-
const mockOnSortModeChange = vi.fn()
441-
render(<ApiConfigSelector {...defaultProps} onSortModeChange={mockOnSortModeChange} />)
442-
443-
const trigger = screen.getByTestId("dropdown-trigger")
444-
fireEvent.click(trigger)
445-
446-
const customButton = screen.getByText("chat:apiConfigSelector.custom")
447-
fireEvent.click(customButton)
448-
449-
expect(mockOnSortModeChange).toHaveBeenCalledWith("custom")
450-
})
451-
452-
test("persists custom order when switching to custom sort", () => {
453-
render(<ApiConfigSelector {...defaultProps} />)
454-
455-
const trigger = screen.getByTestId("dropdown-trigger")
456-
fireEvent.click(trigger)
457-
458-
const customButton = screen.getByText("chat:apiConfigSelector.custom")
459-
fireEvent.click(customButton)
460-
461-
expect(vi.mocked(vscode.postMessage)).toHaveBeenCalledWith(
462-
expect.objectContaining({ type: "setApiConfigsCustomOrder" }),
463-
)
464-
})
465-
466-
test("switches between sort modes", () => {
467-
render(<ApiConfigSelector {...defaultProps} />)
468-
469-
const trigger = screen.getByTestId("dropdown-trigger")
470-
fireEvent.click(trigger)
471-
472-
// Check that alphabetical button is initially active (default state)
473-
const alphabeticalButton = screen.getByText("chat:apiConfigSelector.alphabetical")
474-
expect(alphabeticalButton).toHaveAttribute("aria-pressed", "true")
475-
476-
// Switch to custom mode
477-
const customButton = screen.getByText("chat:apiConfigSelector.custom")
478-
expect(customButton).toHaveAttribute("aria-pressed", "false")
479-
fireEvent.click(customButton)
480-
481-
// Check that custom button is now active
482-
expect(customButton).toHaveAttribute("aria-pressed", "true")
483-
expect(alphabeticalButton).toHaveAttribute("aria-pressed", "false")
484-
485-
// Switch back to alphabetical mode
486-
fireEvent.click(alphabeticalButton)
487-
488-
// Check that alphabetical button is active again
489-
expect(alphabeticalButton).toHaveAttribute("aria-pressed", "true")
490-
expect(customButton).toHaveAttribute("aria-pressed", "false")
491-
})
492-
493439
test("pinned configs remain fixed at top while unpinned configs scroll", () => {
494440
// Create a list with many configs to test scrolling
495441
const manyConfigs = Array.from({ length: 15 }, (_, i) => ({

webview-ui/src/components/settings/ApiConfigManager.tsx

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ const ApiConfigManager = ({
4545
dragOverIndex: null,
4646
})
4747
const [focusedIndex, setFocusedIndex] = useState<number | null>(null)
48-
const [isReorderingMode, setIsReorderingMode] = useState(false)
48+
// Always allow reordering in Settings per design feedback
49+
const isReorderingMode = true
4950
const newProfileInputRef = useRef<any>(null)
5051

5152
const isOnlyProfile = listApiConfigMeta?.length === 1
@@ -270,9 +271,7 @@ const ApiConfigManager = ({
270271
setIsCreating(true)
271272
}
272273

273-
const toggleReorderingMode = () => {
274-
setIsReorderingMode(!isReorderingMode)
275-
}
274+
// Removed explicit toggle; reordering is always enabled
276275

277276
const handleRename = useCallback(
278277
(oldName: string, newName: string) => {
@@ -309,21 +308,18 @@ const ApiConfigManager = ({
309308
<label className="block font-medium mb-1">{t("settings:providers.configProfile")}</label>
310309
{/* Action buttons */}
311310
<div className="flex items-center gap-1 mb-3">
312-
<StandardTooltip
313-
content={
314-
isReorderingMode
315-
? t("settings:providers.exitReorderMode")
316-
: t("settings:providers.reorderProfiles")
317-
}>
311+
<StandardTooltip content="Reset to Alphabetical">
318312
<Button
319313
variant="ghost"
320314
size="icon"
321-
onClick={toggleReorderingMode}
322-
data-testid="reorder-toggle-button"
323-
className={
324-
isReorderingMode ? "bg-vscode-button-background text-vscode-button-foreground" : ""
325-
}>
326-
<span className="codicon codicon-list-ordered" />
315+
onClick={() =>
316+
vscode.postMessage({
317+
type: "setApiConfigsCustomOrder",
318+
values: { customOrder: [] },
319+
})
320+
}
321+
data-testid="reset-to-alphabetical-button">
322+
<span className="codicon codicon-discard" />
327323
</Button>
328324
</StandardTooltip>
329325
<StandardTooltip content={t("settings:providers.addProfile")}>
@@ -369,9 +365,7 @@ const ApiConfigManager = ({
369365

370366
{/* Help text */}
371367
<div className="text-vscode-descriptionForeground text-sm mt-2">
372-
{isReorderingMode
373-
? t("settings:providers.reorderModeHelpText")
374-
: t("settings:providers.normalModeHelpText")}
368+
{t("settings:providers.reorderModeHelpText")}
375369
</div>
376370

377371
<Dialog

webview-ui/src/components/settings/ConfigListItem.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ const ConfigListItem = memo(function ConfigListItemComponent({
200200
}
201201
: undefined
202202
}
203-
onClick={isReorderingMode ? undefined : () => onSelectConfig(config.name)}
203+
onClick={() => onSelectConfig(config.name)}
204204
onKeyDown={(e) => {
205205
onKeyDown(e, index)
206206
if (!isReorderingMode && (e.key === "Enter" || e.key === " ")) {

webview-ui/src/components/settings/__tests__/ApiConfigManager.spec.tsx

Lines changed: 5 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -378,73 +378,21 @@ describe("ApiConfigManager", () => {
378378
listApiConfigMeta: mockConfigsForReordering,
379379
}
380380

381-
it("should render reorder toggle button", () => {
381+
it("shows reset-to-alphabetical button", () => {
382382
render(<ApiConfigManager {...reorderingProps} />)
383-
384-
const reorderButton = screen.getByTestId("reorder-toggle-button")
385-
expect(reorderButton).toBeInTheDocument()
386-
})
387-
388-
it("should toggle reordering mode when reorder button is clicked", () => {
389-
render(<ApiConfigManager {...reorderingProps} />)
390-
391-
const reorderButton = screen.getByTestId("reorder-toggle-button")
392-
393-
// Initially not in reordering mode
394-
expect(reorderButton).not.toHaveClass("bg-vscode-button-background")
395-
396-
// Click to enter reordering mode
397-
fireEvent.click(reorderButton)
398-
expect(reorderButton).toHaveClass("bg-vscode-button-background")
399-
400-
// Click again to exit reordering mode
401-
fireEvent.click(reorderButton)
402-
expect(reorderButton).not.toHaveClass("bg-vscode-button-background")
383+
const resetBtn = screen.getByTestId("reset-to-alphabetical-button")
384+
expect(resetBtn).toBeInTheDocument()
403385
})
404386

405-
it("should show different help text based on reordering mode", () => {
387+
it("always shows reorder help text in settings", () => {
406388
render(<ApiConfigManager {...reorderingProps} />)
407-
408-
const reorderButton = screen.getByTestId("reorder-toggle-button")
409-
410-
// Initially shows normal help text
411-
expect(screen.getByText("settings:providers.normalModeHelpText")).toBeInTheDocument()
412-
413-
// Enter reordering mode
414-
fireEvent.click(reorderButton)
415389
expect(screen.getByText("settings:providers.reorderModeHelpText")).toBeInTheDocument()
416390
})
417391

418-
it("should show checkmark for current config when not in reordering mode", () => {
419-
render(<ApiConfigManager {...reorderingProps} />)
420-
421-
// Should show checkmark for current config
422-
const checkmarks = screen.getAllByText("", { selector: ".codicon-check" })
423-
expect(checkmarks.length).toBeGreaterThan(0)
424-
})
425-
426-
it("should disable regular click behavior in reordering mode", () => {
392+
it("allows click selection while reordering", () => {
427393
render(<ApiConfigManager {...reorderingProps} />)
428-
429-
const reorderButton = screen.getByTestId("reorder-toggle-button")
430394
const configItems = screen.getAllByRole("option")
431-
432-
// Enter reordering mode
433-
fireEvent.click(reorderButton)
434-
435-
// Click on a config item should not trigger selection
436395
fireEvent.click(configItems[1])
437-
expect(mockOnSelectConfig).not.toHaveBeenCalled()
438-
})
439-
440-
it("should allow regular click behavior when not in reordering mode", () => {
441-
render(<ApiConfigManager {...reorderingProps} />)
442-
443-
const configItems = screen.getAllByRole("option")
444-
445-
// Click on a config item should trigger selection
446-
// The actual order depends on the sorting logic, so let's just test that clicking works
447-
fireEvent.click(configItems[0])
448396
expect(mockOnSelectConfig).toHaveBeenCalled()
449397
})
450398
})

0 commit comments

Comments
 (0)