From 55bbe88e392a392dbd789aa854847a98955ad6e5 Mon Sep 17 00:00:00 2001 From: jwc11 Date: Wed, 26 Mar 2025 19:29:24 +0100 Subject: [PATCH] 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 Co-authored-by: cte --- src/core/config/ConfigManager.ts | 40 ++++++- src/core/webview/ClineProvider.ts | 61 ++++++++++- .../webview/__tests__/ClineProvider.test.ts | 32 ++++++ src/exports/roo-code.d.ts | 1 + src/shared/ExtensionMessage.ts | 2 + src/shared/WebviewMessage.ts | 2 + src/shared/globalState.ts | 1 + .../src/components/chat/ChatTextArea.tsx | 102 ++++++++++++++++-- .../src/components/ui/select-dropdown.tsx | 23 ++-- .../src/context/ExtensionStateContext.tsx | 20 ++++ 10 files changed, 262 insertions(+), 22 deletions(-) diff --git a/src/core/config/ConfigManager.ts b/src/core/config/ConfigManager.ts index dd4262ee2e..0ff8b0d400 100644 --- a/src/core/config/ConfigManager.ts +++ b/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 { 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 */ diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index addd020da2..db0491ad27 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1675,6 +1675,25 @@ export class ClineProvider extends EventEmitter 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 = { ...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 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 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 implements language, renderContext: this.renderContext, maxReadFileLine: maxReadFileLine ?? 500, + pinnedApiConfigs: + ((await this.getGlobalState("pinnedApiConfigs")) as Record) ?? + ({} as Record), } } diff --git a/src/core/webview/__tests__/ClineProvider.test.ts b/src/core/webview/__tests__/ClineProvider.test.ts index c77e677f6f..0c1e7d9d42 100644 --- a/src/core/webview/__tests__/ClineProvider.test.ts +++ b/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] diff --git a/src/exports/roo-code.d.ts b/src/exports/roo-code.d.ts index ddbd8b2d40..775f579889 100644 --- a/src/exports/roo-code.d.ts +++ b/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 diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index f42381701a..fc1dc6f833 100644 --- a/src/shared/ExtensionMessage.ts +++ b/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 // Map of API config names to pinned state maxReadFileLine: number // Maximum number of lines to read from a file before truncating } diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 7a4cb38c67..7209eabec8 100644 --- a/src/shared/WebviewMessage.ts +++ b/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 diff --git a/src/shared/globalState.ts b/src/shared/globalState.ts index 71d990906a..fa8b6f864d 100644 --- a/src/shared/globalState.ts +++ b/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 diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index a6cacfe7cd..b01dca3b37 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/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( 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([]) const [showDropdown, setShowDropdown] = useState(false) const [fileSearchResults, setFileSearchResults] = useState([]) @@ -955,15 +974,45 @@ const ChatTextArea = forwardRef( {/* API configuration selector - flexible width */}
({ - 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( 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 ( +
+
{label}
+
+ + {isCurrentConfig && } +
+
+ ) + }} />
diff --git a/webview-ui/src/components/ui/select-dropdown.tsx b/webview-ui/src/components/ui/select-dropdown.tsx index 5360eba9d9..4018418439 100644 --- a/webview-ui/src/components/ui/select-dropdown.tsx +++ b/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, SelectDropdownProps>( @@ -55,14 +57,17 @@ export const SelectDropdown = React.forwardRef { 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 handleSelect(option)}> - {option.label} - {option.value === value && ( - - - + {renderItem ? ( + renderItem(option) + ) : ( + <> + {option.label} + {option.value === value && ( + + + + )} + )} ) diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index b181e50e01..b8c8b3316b 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/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 + setPinnedApiConfigs: (value: Record) => void + togglePinnedApiConfig: (configName: string) => void } export const ExtensionStateContext = createContext(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 {children}