Browse Source

feat: add pinning and sorting for API configurations dropdown (#2001)

* feat: add pinning functionality for API configurations

Added the ability to pin/unpin API configurations, enabling prioritized appearance in dropdown menus.
- modified state management to support storing pinned configurations persistently.
- Updated UI components to display and toggle pin states
- Adjusted backend handling for syncing pinned configuration states across sessions.

* refactor: add support for loading API configurations by ID

- Introduced a new method `loadConfigById` in `ConfigManager` to load API configurations using their unique ID.
- Refactored the dropdown and pin logic to work with ID
- Added tests to verify the new functionality for loading configurations by ID.

* fix: preserve existing API config ID on updates

- Ensure that the existing ID is retained when updating an API config.
- Prevents unintentional ID changes during configuration updates.

* Fix theme issues

---------

Co-authored-by: james <[email protected]>
Co-authored-by: cte <[email protected]>
jwc11 9 months ago
parent
commit
55bbe88e39

+ 37 - 3
src/core/config/ConfigManager.ts

@@ -89,17 +89,23 @@ export class ConfigManager {
 	}
 
 	/**
-	 * Save a config with the given name
+	 * Save a config with the given name.
+	 * Preserves the ID from the input 'config' object if it exists,
+	 * otherwise generates a new one (for creation scenarios).
 	 */
 	async saveConfig(name: string, config: ApiConfiguration): Promise<void> {
 		try {
 			return await this.lock(async () => {
 				const currentConfig = await this.readConfig()
-				const existingConfig = currentConfig.apiConfigs[name]
+
+				// Preserve the existing ID if this is an update to an existing config
+				const existingId = currentConfig.apiConfigs[name]?.id
+
 				currentConfig.apiConfigs[name] = {
 					...config,
-					id: existingConfig?.id || this.generateId(),
+					id: config.id || existingId || this.generateId(),
 				}
+
 				await this.writeConfig(currentConfig)
 			})
 		} catch (error) {
@@ -130,6 +136,34 @@ export class ConfigManager {
 		}
 	}
 
+	/**
+	 * Load a config by ID
+	 */
+	async loadConfigById(id: string): Promise<{ config: ApiConfiguration; name: string }> {
+		try {
+			return await this.lock(async () => {
+				const config = await this.readConfig()
+
+				// Find the config with the matching ID
+				const entry = Object.entries(config.apiConfigs).find(([_, apiConfig]) => apiConfig.id === id)
+
+				if (!entry) {
+					throw new Error(`Config with ID '${id}' not found`)
+				}
+
+				const [name, apiConfig] = entry
+
+				// Update current config name
+				config.currentApiConfigName = name
+				await this.writeConfig(config)
+
+				return { config: apiConfig, name }
+			})
+		} catch (error) {
+			throw new Error(`Failed to load config by ID: ${error}`)
+		}
+	}
+
 	/**
 	 * Delete a config by name
 	 */

+ 57 - 4
src/core/webview/ClineProvider.ts

@@ -1675,6 +1675,25 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 						await this.updateGlobalState("maxReadFileLine", message.value)
 						await this.postStateToWebview()
 						break
+					case "toggleApiConfigPin":
+						if (message.text) {
+							const currentPinned = ((await this.getGlobalState("pinnedApiConfigs")) || {}) as Record<
+								string,
+								boolean
+							>
+							const updatedPinned: Record<string, boolean> = { ...currentPinned }
+
+							// Toggle the pinned state
+							if (currentPinned[message.text]) {
+								delete updatedPinned[message.text]
+							} else {
+								updatedPinned[message.text] = true
+							}
+
+							await this.updateGlobalState("pinnedApiConfigs", updatedPinned)
+							await this.postStateToWebview()
+						}
+						break
 					case "enhancementApiConfigId":
 						await this.updateGlobalState("enhancementApiConfigId", message.text)
 						await this.postStateToWebview()
@@ -1848,16 +1867,24 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 									break
 								}
 
-								await this.configManager.saveConfig(newName, message.apiConfiguration)
+								// Load the old configuration to get its ID
+								const oldConfig = await this.configManager.loadConfig(oldName)
+
+								// Create a new configuration with the same ID
+								const newConfig = {
+									...message.apiConfiguration,
+									id: oldConfig.id, // Preserve the ID
+								}
+
+								// Save with the new name but same ID
+								await this.configManager.saveConfig(newName, newConfig)
 								await this.configManager.deleteConfig(oldName)
 
 								const listApiConfig = await this.configManager.listConfig()
-								const config = listApiConfig?.find((c) => c.name === newName)
 
 								// Update listApiConfigMeta first to ensure UI has latest data
 								await this.updateGlobalState("listApiConfigMeta", listApiConfig)
-
-								await Promise.all([this.updateGlobalState("currentApiConfigName", newName)])
+								await this.updateGlobalState("currentApiConfigName", newName)
 
 								await this.postStateToWebview()
 							} catch (error) {
@@ -1889,6 +1916,29 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 							}
 						}
 						break
+					case "loadApiConfigurationById":
+						if (message.text) {
+							try {
+								const { config: apiConfig, name } = await this.configManager.loadConfigById(
+									message.text,
+								)
+								const listApiConfig = await this.configManager.listConfig()
+
+								await Promise.all([
+									this.updateGlobalState("listApiConfigMeta", listApiConfig),
+									this.updateGlobalState("currentApiConfigName", name),
+									this.updateApiConfiguration(apiConfig),
+								])
+
+								await this.postStateToWebview()
+							} catch (error) {
+								this.outputChannel.appendLine(
+									`Error load api configuration by ID: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
+								)
+								vscode.window.showErrorMessage(t("common:errors.load_api_config"))
+							}
+						}
+						break
 					case "deleteApiConfiguration":
 						if (message.text) {
 							const answer = await vscode.window.showInformationMessage(
@@ -2610,6 +2660,9 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 			language,
 			renderContext: this.renderContext,
 			maxReadFileLine: maxReadFileLine ?? 500,
+			pinnedApiConfigs:
+				((await this.getGlobalState("pinnedApiConfigs")) as Record<string, boolean>) ??
+				({} as Record<string, boolean>),
 		}
 	}
 

+ 32 - 0
src/core/webview/__tests__/ClineProvider.test.ts

@@ -700,6 +700,9 @@ describe("ClineProvider", () => {
 
 		provider.configManager = {
 			loadConfig: jest.fn().mockResolvedValue({ apiProvider: "anthropic", id: "new-id" }),
+			loadConfigById: jest
+				.fn()
+				.mockResolvedValue({ config: { apiProvider: "anthropic", id: "new-id" }, name: "new-config" }),
 			listConfig: jest.fn().mockResolvedValue([{ name: "new-config", id: "new-id", apiProvider: "anthropic" }]),
 			setModeConfig: jest.fn(),
 			getModeConfigId: jest.fn().mockResolvedValue(undefined),
@@ -715,6 +718,35 @@ describe("ClineProvider", () => {
 		expect(provider.configManager.setModeConfig).toHaveBeenCalledWith("architect", "new-id")
 	})
 
+	test("load API configuration by ID works and updates mode config", async () => {
+		await provider.resolveWebviewView(mockWebviewView)
+		const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]
+
+		provider.configManager = {
+			loadConfigById: jest.fn().mockResolvedValue({
+				config: { apiProvider: "anthropic", id: "config-id-123" },
+				name: "config-by-id",
+			}),
+			listConfig: jest
+				.fn()
+				.mockResolvedValue([{ name: "config-by-id", id: "config-id-123", apiProvider: "anthropic" }]),
+			setModeConfig: jest.fn(),
+			getModeConfigId: jest.fn().mockResolvedValue(undefined),
+		} as any
+
+		// First set the mode
+		await messageHandler({ type: "mode", text: "architect" })
+
+		// Then load the config by ID
+		await messageHandler({ type: "loadApiConfigurationById", text: "config-id-123" })
+
+		// Should save new config as default for architect mode
+		expect(provider.configManager.setModeConfig).toHaveBeenCalledWith("architect", "config-id-123")
+
+		// Ensure the loadConfigById method was called with the correct ID
+		expect(provider.configManager.loadConfigById).toHaveBeenCalledWith("config-id-123")
+	})
+
 	test("handles browserToolEnabled setting", async () => {
 		await provider.resolveWebviewView(mockWebviewView)
 		const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0]

+ 1 - 0
src/exports/roo-code.d.ts

@@ -259,6 +259,7 @@ export type GlobalStateKey =
 	| "language"
 	| "maxReadFileLine"
 	| "fakeAi"
+	| "pinnedApiConfigs" // Record of API config names that should be pinned to the top of the API provides dropdown
 
 export type ConfigurationKey = GlobalStateKey | SecretKey
 

+ 2 - 0
src/shared/ExtensionMessage.ts

@@ -58,6 +58,7 @@ export interface ExtensionMessage {
 		| "ttsStop"
 		| "maxReadFileLine"
 		| "fileSearchResults"
+		| "toggleApiConfigPin"
 	text?: string
 	action?:
 		| "chatButtonClicked"
@@ -168,6 +169,7 @@ export interface ExtensionState {
 	machineId?: string
 	showRooIgnoredFiles: boolean // Whether to show .rooignore'd files in listings
 	renderContext: "sidebar" | "editor"
+	pinnedApiConfigs?: Record<string, boolean> // Map of API config names to pinned state
 	maxReadFileLine: number // Maximum number of lines to read from a file before truncating
 }
 

+ 2 - 0
src/shared/WebviewMessage.ts

@@ -17,6 +17,7 @@ export interface WebviewMessage {
 		| "upsertApiConfiguration"
 		| "deleteApiConfiguration"
 		| "loadApiConfiguration"
+		| "loadApiConfigurationById"
 		| "renameApiConfiguration"
 		| "getListApiConfiguration"
 		| "customInstructions"
@@ -117,6 +118,7 @@ export interface WebviewMessage {
 		| "language"
 		| "maxReadFileLine"
 		| "searchFiles"
+		| "toggleApiConfigPin"
 	text?: string
 	disabled?: boolean
 	askResponse?: ClineAskResponse

+ 1 - 0
src/shared/globalState.ts

@@ -126,6 +126,7 @@ export const GLOBAL_STATE_KEYS = [
 	"maxWorkspaceFiles",
 	"maxReadFileLine",
 	"fakeAi",
+	"pinnedApiConfigs",
 ] as const
 
 export const PASS_THROUGH_STATE_KEYS = ["taskHistory"] as const

+ 93 - 9
webview-ui/src/components/chat/ChatTextArea.tsx

@@ -24,7 +24,7 @@ import { SelectDropdown, DropdownOptionType, Button } from "@/components/ui"
 import Thumbnails from "../common/Thumbnails"
 import { MAX_IMAGES_PER_MESSAGE } from "./ChatView"
 import ContextMenu from "./ContextMenu"
-import { VolumeX } from "lucide-react"
+import { VolumeX, Pin, Check } from "lucide-react"
 import { IconButton } from "./IconButton"
 import { cn } from "@/lib/utils"
 
@@ -64,7 +64,26 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
 		ref,
 	) => {
 		const { t } = useAppTranslation()
-		const { filePaths, openedTabs, currentApiConfigName, listApiConfigMeta, customModes, cwd } = useExtensionState()
+		const {
+			filePaths,
+			openedTabs,
+			currentApiConfigName,
+			listApiConfigMeta,
+			customModes,
+			cwd,
+			pinnedApiConfigs,
+			togglePinnedApiConfig,
+		} = useExtensionState()
+
+		// Find the ID and display text for the currently selected API configuration
+		const { currentConfigId, displayName } = useMemo(() => {
+			const currentConfig = listApiConfigMeta?.find((config) => config.name === currentApiConfigName)
+			return {
+				currentConfigId: currentConfig?.id || "",
+				displayName: currentApiConfigName || "", // Use the name directly for display
+			}
+		}, [listApiConfigMeta, currentApiConfigName])
+
 		const [gitCommits, setGitCommits] = useState<any[]>([])
 		const [showDropdown, setShowDropdown] = useState(false)
 		const [fileSearchResults, setFileSearchResults] = useState<SearchResult[]>([])
@@ -955,15 +974,45 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
 						{/* API configuration selector - flexible width */}
 						<div className={cn("flex-1", "min-w-0", "overflow-hidden")}>
 							<SelectDropdown
-								value={currentApiConfigName || ""}
+								value={currentConfigId}
 								disabled={textAreaDisabled}
 								title={t("chat:selectApiConfig")}
+								placeholder={displayName} // Always show the current name
 								options={[
-									...(listApiConfigMeta || []).map((config) => ({
-										value: config.name,
-										label: config.name,
-										type: DropdownOptionType.ITEM,
-									})),
+									// Pinned items first
+									...(listApiConfigMeta || [])
+										.filter((config) => pinnedApiConfigs && pinnedApiConfigs[config.id])
+										.map((config) => ({
+											value: config.id,
+											label: config.name,
+											name: config.name, // Keep name for comparison with currentApiConfigName
+											type: DropdownOptionType.ITEM,
+											pinned: true,
+										}))
+										.sort((a, b) => a.label.localeCompare(b.label)),
+									// If we have pinned items and unpinned items, add a separator
+									...(pinnedApiConfigs &&
+									Object.keys(pinnedApiConfigs).length > 0 &&
+									(listApiConfigMeta || []).some((config) => !pinnedApiConfigs[config.id])
+										? [
+												{
+													value: "sep-pinned",
+													label: t("chat:separator"),
+													type: DropdownOptionType.SEPARATOR,
+												},
+											]
+										: []),
+									// Unpinned items sorted alphabetically
+									...(listApiConfigMeta || [])
+										.filter((config) => !pinnedApiConfigs || !pinnedApiConfigs[config.id])
+										.map((config) => ({
+											value: config.id,
+											label: config.name,
+											name: config.name, // Keep name for comparison with currentApiConfigName
+											type: DropdownOptionType.ITEM,
+											pinned: false,
+										}))
+										.sort((a, b) => a.label.localeCompare(b.label)),
 									{
 										value: "sep-2",
 										label: t("chat:separator"),
@@ -975,9 +1024,44 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
 										type: DropdownOptionType.ACTION,
 									},
 								]}
-								onChange={(value) => vscode.postMessage({ type: "loadApiConfiguration", text: value })}
+								onChange={(value) => {
+									if (value === "settingsButtonClicked") {
+										vscode.postMessage({ type: "loadApiConfiguration", text: value })
+									} else {
+										vscode.postMessage({ type: "loadApiConfigurationById", text: value })
+									}
+								}}
 								contentClassName="max-h-[300px] overflow-y-auto"
 								triggerClassName="w-full text-ellipsis overflow-hidden"
+								renderItem={({ type, value, label, pinned }) => {
+									if (type !== DropdownOptionType.ITEM) {
+										return label
+									}
+
+									const config = listApiConfigMeta?.find((c) => c.id === value)
+									const isCurrentConfig = config?.name === currentApiConfigName
+
+									return (
+										<div className="flex items-center justify-between gap-2 w-full">
+											<div className={cn({ "font-medium": isCurrentConfig })}>{label}</div>
+											<div className="flex items-center gap-1">
+												<Button
+													variant="ghost"
+													size="icon"
+													title={pinned ? t("chat:unpin") : t("chat:pin")}
+													onClick={(e) => {
+														e.stopPropagation()
+														togglePinnedApiConfig(value)
+														vscode.postMessage({ type: "toggleApiConfigPin", text: value })
+													}}
+													className={cn("w-5 h-5", { "bg-accent": pinned })}>
+													<Pin className="size-3 p-0.5 opacity-50" />
+												</Button>
+												{isCurrentConfig && <Check className="size-3" />}
+											</div>
+										</div>
+									)
+								}}
 							/>
 						</div>
 					</div>

+ 17 - 6
webview-ui/src/components/ui/select-dropdown.tsx

@@ -25,6 +25,7 @@ export interface DropdownOption {
 	label: string
 	disabled?: boolean
 	type?: DropdownOptionType
+	pinned?: boolean
 }
 
 export interface SelectDropdownProps {
@@ -39,6 +40,7 @@ export interface SelectDropdownProps {
 	align?: "start" | "center" | "end"
 	placeholder?: string
 	shortcutText?: string
+	renderItem?: (option: DropdownOption) => React.ReactNode
 }
 
 export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownMenuTrigger>, SelectDropdownProps>(
@@ -55,14 +57,17 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
 			align = "start",
 			placeholder = "",
 			shortcutText = "",
+			renderItem,
 		},
 		ref,
 	) => {
 		const [open, setOpen] = React.useState(false)
 		const portalContainer = useRooPortal("roo-portal")
 
+		// If the selected option isn't in the list yet, but we have a placeholder, prioritize showing the placeholder
 		const selectedOption = options.find((option) => option.value === value)
-		const displayText = selectedOption?.label || placeholder || ""
+		const displayText =
+			value && !selectedOption && placeholder ? placeholder : selectedOption?.label || placeholder || ""
 
 		const handleSelect = (option: DropdownOption) => {
 			if (option.type === DropdownOptionType.ACTION) {
@@ -121,11 +126,17 @@ export const SelectDropdown = React.forwardRef<React.ElementRef<typeof DropdownM
 								key={`item-${option.value}`}
 								disabled={option.disabled}
 								onClick={() => handleSelect(option)}>
-								{option.label}
-								{option.value === value && (
-									<DropdownMenuShortcut>
-										<Check className="size-4 p-0.5" />
-									</DropdownMenuShortcut>
+								{renderItem ? (
+									renderItem(option)
+								) : (
+									<>
+										{option.label}
+										{option.value === value && (
+											<DropdownMenuShortcut>
+												<Check className="size-4 p-0.5" />
+											</DropdownMenuShortcut>
+										)}
+									</>
 								)}
 							</DropdownMenuItem>
 						)

+ 20 - 0
webview-ui/src/context/ExtensionStateContext.tsx

@@ -81,6 +81,9 @@ export interface ExtensionStateContextType extends ExtensionState {
 	maxReadFileLine: number
 	setMaxReadFileLine: (value: number) => void
 	machineId?: string
+	pinnedApiConfigs?: Record<string, boolean>
+	setPinnedApiConfigs: (value: Record<string, boolean>) => void
+	togglePinnedApiConfig: (configName: string) => void
 }
 
 export const ExtensionStateContext = createContext<ExtensionStateContextType | undefined>(undefined)
@@ -155,6 +158,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
 		showRooIgnoredFiles: true, // Default to showing .rooignore'd files with lock symbol (current behavior).
 		renderContext: "sidebar",
 		maxReadFileLine: 500, // Default max read file line limit
+		pinnedApiConfigs: {}, // Empty object for pinned API configs
 	})
 
 	const [didHydrateState, setDidHydrateState] = useState(false)
@@ -307,6 +311,22 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
 		setShowRooIgnoredFiles: (value) => setState((prevState) => ({ ...prevState, showRooIgnoredFiles: value })),
 		setRemoteBrowserEnabled: (value) => setState((prevState) => ({ ...prevState, remoteBrowserEnabled: value })),
 		setMaxReadFileLine: (value) => setState((prevState) => ({ ...prevState, maxReadFileLine: value })),
+		setPinnedApiConfigs: (value) => setState((prevState) => ({ ...prevState, pinnedApiConfigs: value })),
+		togglePinnedApiConfig: (configId) =>
+			setState((prevState) => {
+				const currentPinned = prevState.pinnedApiConfigs || {}
+				const newPinned = {
+					...currentPinned,
+					[configId]: !currentPinned[configId],
+				}
+
+				// If the config is now unpinned, remove it from the object
+				if (!newPinned[configId]) {
+					delete newPinned[configId]
+				}
+
+				return { ...prevState, pinnedApiConfigs: newPinned }
+			}),
 	}
 
 	return <ExtensionStateContext.Provider value={contextValue}>{children}</ExtensionStateContext.Provider>