-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add sorting and pinning for modes and API profiles #7497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { cn } from "@/lib/utils" | |
| import { useRooPortal } from "@/components/ui/hooks/useRooPortal" | ||
| import { Popover, PopoverContent, PopoverTrigger, StandardTooltip } from "@/components/ui" | ||
| import { useAppTranslation } from "@/i18n/TranslationContext" | ||
| import { useExtensionState } from "@/context/ExtensionStateContext" | ||
| import { vscode } from "@/utils/vscode" | ||
| import { Button } from "@/components/ui" | ||
|
|
||
|
|
@@ -39,6 +40,9 @@ export const ApiConfigSelector = ({ | |
| const [searchValue, setSearchValue] = useState("") | ||
| const portalContainer = useRooPortal("roo-portal") | ||
|
|
||
| // Get sorting preferences from extension state | ||
| const { apiProfileSortingMode, customApiProfileOrder } = useExtensionState() | ||
|
|
||
| // Create searchable items for fuzzy search. | ||
| const searchableItems = useMemo( | ||
| () => | ||
|
|
@@ -65,12 +69,40 @@ export const ApiConfigSelector = ({ | |
| return matchingItems | ||
| }, [listApiConfigMeta, searchValue, fzfInstance]) | ||
|
|
||
| // Sort configs based on sorting preferences | ||
| const sortedConfigs = useMemo(() => { | ||
| const sorted = [...filteredConfigs] | ||
|
|
||
| if (apiProfileSortingMode === "manual" && customApiProfileOrder && customApiProfileOrder.length > 0) { | ||
| // Sort based on custom order | ||
| sorted.sort((a, b) => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sorting logic is nearly identical to the one in ModeSelector.tsx. Could we extract it into a shared utility function to reduce duplication? |
||
| const aIndex = customApiProfileOrder.indexOf(a.id) | ||
| const bIndex = customApiProfileOrder.indexOf(b.id) | ||
|
|
||
| // If both are in custom order, sort by their position | ||
| if (aIndex !== -1 && bIndex !== -1) { | ||
| return aIndex - bIndex | ||
| } | ||
| // If only one is in custom order, it comes first | ||
| if (aIndex !== -1) return -1 | ||
| if (bIndex !== -1) return 1 | ||
| // Otherwise maintain original order | ||
| return 0 | ||
| }) | ||
| } else { | ||
| // Alphabetical sorting (default) | ||
| sorted.sort((a, b) => a.name.localeCompare(b.name)) | ||
| } | ||
|
|
||
| return sorted | ||
| }, [filteredConfigs, apiProfileSortingMode, customApiProfileOrder]) | ||
|
|
||
| // Separate pinned and unpinned configs. | ||
| const { pinnedConfigs, unpinnedConfigs } = useMemo(() => { | ||
| const pinned = filteredConfigs.filter((config) => pinnedApiConfigs?.[config.id]) | ||
| const unpinned = filteredConfigs.filter((config) => !pinnedApiConfigs?.[config.id]) | ||
| const pinned = sortedConfigs.filter((config) => pinnedApiConfigs?.[config.id]) | ||
| const unpinned = sortedConfigs.filter((config) => !pinnedApiConfigs?.[config.id]) | ||
| return { pinnedConfigs: pinned, unpinnedConfigs: unpinned } | ||
| }, [filteredConfigs, pinnedApiConfigs]) | ||
| }, [sortedConfigs, pinnedApiConfigs]) | ||
|
|
||
| const handleSelect = useCallback( | ||
| (configId: string) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ import { cn } from "@/lib/utils" | |
| import { useExtensionState } from "@/context/ExtensionStateContext" | ||
| import { useAppTranslation } from "@/i18n/TranslationContext" | ||
| import { useRooPortal } from "@/components/ui/hooks/useRooPortal" | ||
| import { Popover, PopoverContent, PopoverTrigger, StandardTooltip } from "@/components/ui" | ||
| import { Popover, PopoverContent, PopoverTrigger, StandardTooltip, Button } from "@/components/ui" | ||
|
|
||
| import { IconButton } from "./IconButton" | ||
|
|
||
|
|
@@ -45,7 +45,14 @@ export const ModeSelector = ({ | |
| const [searchValue, setSearchValue] = React.useState("") | ||
| const searchInputRef = React.useRef<HTMLInputElement>(null) | ||
| const portalContainer = useRooPortal("roo-portal") | ||
| const { hasOpenedModeSelector, setHasOpenedModeSelector } = useExtensionState() | ||
| const { | ||
| hasOpenedModeSelector, | ||
| setHasOpenedModeSelector, | ||
| modeSortingMode, | ||
| pinnedModes, | ||
| togglePinnedMode, | ||
| customModeOrder, | ||
| } = useExtensionState() | ||
| const { t } = useAppTranslation() | ||
|
|
||
| const trackModeSelectorOpened = React.useCallback(() => { | ||
|
|
@@ -99,9 +106,44 @@ export const ModeSelector = ({ | |
| [descriptionSearchItems], | ||
| ) | ||
|
|
||
| // Sort modes based on sorting preferences | ||
| const sortedModes = React.useMemo(() => { | ||
| let sorted = [...modes] | ||
|
|
||
| if (modeSortingMode === "manual" && customModeOrder && customModeOrder.length > 0) { | ||
| // Sort based on custom order | ||
| sorted.sort((a, b) => { | ||
| const aIndex = customModeOrder.indexOf(a.slug) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| const bIndex = customModeOrder.indexOf(b.slug) | ||
|
|
||
| // If both are in custom order, sort by their position | ||
| if (aIndex !== -1 && bIndex !== -1) { | ||
| return aIndex - bIndex | ||
| } | ||
| // If only one is in custom order, it comes first | ||
| if (aIndex !== -1) return -1 | ||
| if (bIndex !== -1) return 1 | ||
| // Otherwise maintain original order | ||
| return 0 | ||
| }) | ||
| } else { | ||
| // Alphabetical sorting (default) | ||
| sorted.sort((a, b) => a.name.localeCompare(b.name)) | ||
| } | ||
|
|
||
| // Apply pinning - pinned modes come first | ||
| if (pinnedModes) { | ||
| const pinned = sorted.filter((mode) => pinnedModes[mode.slug]) | ||
| const unpinned = sorted.filter((mode) => !pinnedModes[mode.slug]) | ||
| sorted = [...pinned, ...unpinned] | ||
| } | ||
|
|
||
| return sorted | ||
| }, [modes, modeSortingMode, pinnedModes, customModeOrder]) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing test coverage for the new sorting and pinning functionality. Could we add tests to verify that:
|
||
|
|
||
| // Filter modes based on search value using fuzzy search with priority. | ||
| const filteredModes = React.useMemo(() => { | ||
| if (!searchValue) return modes | ||
| if (!searchValue) return sortedModes | ||
|
|
||
| // First search in names/slugs. | ||
| const nameMatches = nameFzfInstance.find(searchValue) | ||
|
|
@@ -118,8 +160,13 @@ export const ModeSelector = ({ | |
| .map((result) => result.item.original), | ||
| ] | ||
|
|
||
| return combinedResults | ||
| }, [modes, searchValue, nameFzfInstance, descriptionFzfInstance]) | ||
| // Preserve the sorting order after filtering | ||
| const sortedFilteredResults = sortedModes.filter((mode) => | ||
| combinedResults.some((result) => result.slug === mode.slug), | ||
| ) | ||
|
|
||
| return sortedFilteredResults | ||
| }, [sortedModes, searchValue, nameFzfInstance, descriptionFzfInstance]) | ||
|
|
||
| const onClearSearch = React.useCallback(() => { | ||
| setSearchValue("") | ||
|
|
@@ -230,29 +277,24 @@ export const ModeSelector = ({ | |
| </div> | ||
| ) : ( | ||
| <div className="py-1"> | ||
| {filteredModes.map((mode) => ( | ||
| <div | ||
| key={mode.slug} | ||
| onClick={() => handleSelect(mode.slug)} | ||
| className={cn( | ||
| "px-3 py-1.5 text-sm cursor-pointer flex items-center", | ||
| "hover:bg-vscode-list-hoverBackground", | ||
| mode.slug === value | ||
| ? "bg-vscode-list-activeSelectionBackground text-vscode-list-activeSelectionForeground" | ||
| : "", | ||
| )} | ||
| data-testid="mode-selector-item"> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-bold truncate">{mode.name}</div> | ||
| {mode.description && ( | ||
| <div className="text-xs text-vscode-descriptionForeground truncate"> | ||
| {mode.description} | ||
| </div> | ||
| {/* Show pinned modes first if any */} | ||
| {pinnedModes && | ||
| Object.keys(pinnedModes).length > 0 && | ||
| filteredModes.filter((mode) => pinnedModes[mode.slug]).length > 0 && ( | ||
| <> | ||
| {filteredModes | ||
| .filter((mode) => pinnedModes[mode.slug]) | ||
| .map((mode) => renderModeItem(mode, true))} | ||
| {/* Separator between pinned and unpinned */} | ||
| {filteredModes.filter((mode) => !pinnedModes[mode.slug]).length > 0 && ( | ||
| <div className="mx-1 my-1 h-px bg-vscode-dropdown-foreground/10" /> | ||
| )} | ||
| </div> | ||
| {mode.slug === value && <Check className="ml-auto size-4 p-0.5" />} | ||
| </div> | ||
| ))} | ||
| </> | ||
| )} | ||
| {/* Show unpinned modes */} | ||
| {filteredModes | ||
| .filter((mode) => !pinnedModes || !pinnedModes[mode.slug]) | ||
| .map((mode) => renderModeItem(mode, false))} | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
@@ -301,4 +343,53 @@ export const ModeSelector = ({ | |
| </PopoverContent> | ||
| </Popover> | ||
| ) | ||
|
|
||
| function renderModeItem(mode: (typeof modes)[0], isPinned: boolean) { | ||
| const isSelected = mode.slug === value | ||
|
|
||
| return ( | ||
| <div | ||
| key={mode.slug} | ||
| onClick={() => handleSelect(mode.slug)} | ||
| className={cn( | ||
| "px-3 py-1.5 text-sm cursor-pointer flex items-center group", | ||
| "hover:bg-vscode-list-hoverBackground", | ||
| isSelected | ||
| ? "bg-vscode-list-activeSelectionBackground text-vscode-list-activeSelectionForeground" | ||
| : "", | ||
| )} | ||
| data-testid="mode-selector-item"> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-bold truncate">{mode.name}</div> | ||
| {mode.description && ( | ||
| <div className="text-xs text-vscode-descriptionForeground truncate">{mode.description}</div> | ||
| )} | ||
| </div> | ||
| <div className="flex items-center gap-1"> | ||
| {isSelected && ( | ||
| <div className="size-5 p-1 flex items-center justify-center"> | ||
| <Check className="size-3" /> | ||
| </div> | ||
| )} | ||
| <StandardTooltip content={isPinned ? t("chat:unpin") : t("chat:pin")}> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| tabIndex={-1} | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| togglePinnedMode?.(mode.slug) | ||
| vscode.postMessage({ type: "toggleModePin", text: mode.slug }) | ||
| }} | ||
| className={cn("size-5 flex items-center justify-center", { | ||
| "opacity-0 group-hover:opacity-100": !isPinned && !isSelected, | ||
| "bg-accent opacity-100": isPinned, | ||
| })}> | ||
| <span className="codicon codicon-pin text-xs opacity-50" /> | ||
| </Button> | ||
| </StandardTooltip> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR mentions future UI controls for toggling between sorting modes, but currently there's no way for users to switch from "alphabetical" to "manual" mode. Should we add a basic toggle in the Settings view, or is this intentionally left for a follow-up PR?