fix: address PR review feedback for mode selector export/import

- Replace raw button with IconButton component for consistency
- Add proper error handling with inline error messages
- Add aria-label to export button for accessibility
- Add unique IDs to radio inputs in import dialog
- Add comprehensive test coverage for export/import functionality

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

@@ -1,5 +1,5 @@
 import React from "react"
-import { ChevronUp, Check, X, Upload, Download } from "lucide-react"
+import { ChevronUp, Check, X, Upload } from "lucide-react"
 import { cn } from "@/lib/utils"
 import { useRooPortal } from "@/components/ui/hooks/useRooPortal"
 import { Popover, PopoverContent, PopoverTrigger, StandardTooltip, Button } from "@/components/ui"
@@ -50,6 +50,8 @@ export const ModeSelector = ({
 	const [isExporting, setIsExporting] = React.useState<string | null>(null)
 	const [isImporting, setIsImporting] = React.useState(false)
 	const [showImportDialog, setShowImportDialog] = React.useState(false)
+	const [exportError, setExportError] = React.useState<string | null>(null)
+	const [importError, setImportError] = React.useState<string | null>(null)
 
 	const trackModeSelectorOpened = React.useCallback(() => {
 		// Track telemetry every time the mode selector is opened
@@ -166,22 +168,31 @@ export const ModeSelector = ({
 				setIsExporting(null)
 				if (!message.success) {
 					console.error("Failed to export mode:", message.error)
+					setExportError(message.error || t("prompts:exportMode.error"))
+					// Clear error after 5 seconds
+					setTimeout(() => setExportError(null), 5000)
+				} else {
+					setExportError(null)
 				}
 			} else if (message.type === "importModeResult") {
 				setIsImporting(false)
-				setShowImportDialog(false)
 				if (!message.success && message.error !== "cancelled") {
 					console.error("Failed to import mode:", message.error)
+					setImportError(message.error || t("prompts:importMode.error"))
+				} else {
+					setImportError(null)
+					setShowImportDialog(false)
 				}
 			}
 		}
 		window.addEventListener("message", handler)
 		return () => window.removeEventListener("message", handler)
-	}, [])
+	}, [t])
 
 	// Handle export mode
 	const handleExportMode = React.useCallback((modeSlug: string) => {
 		setIsExporting(modeSlug)
+		setExportError(null)
 		vscode.postMessage({
 			type: "exportMode",
 			slug: modeSlug,
@@ -193,6 +204,7 @@ export const ModeSelector = ({
 		const selectedLevel = (document.querySelector('input[name="importLevel"]:checked') as HTMLInputElement)
 			?.value as "global" | "project"
 		setIsImporting(true)
+		setImportError(null)
 		vscode.postMessage({
 			type: "importMode",
 			source: selectedLevel || "project",
@@ -309,6 +321,7 @@ export const ModeSelector = ({
 														handleExportMode(mode.slug)
 													}}
 													disabled={isExporting === mode.slug}
+													aria-label={t("prompts:exportMode.title")}
 													title={t("prompts:exportMode.title")}>
 													<Upload className="h-3.5 w-3.5" />
 												</Button>
@@ -320,6 +333,13 @@ export const ModeSelector = ({
 							)}
 						</div>
 
+						{/* Export error message */}
+						{exportError && (
+							<div className="px-3 py-2 text-xs text-vscode-errorForeground border-t border-vscode-dropdown-border">
+								{exportError}
+							</div>
+						)}
+
 						{/* Bottom bar with buttons on left and title on right */}
 						<div className="flex flex-row items-center justify-between px-2 py-2 border-t border-vscode-dropdown-border">
 							<div className="flex flex-row gap-1">
@@ -349,28 +369,14 @@ export const ModeSelector = ({
 										setOpen(false)
 									}}
 								/>
-								{/* Import button - matching IconButton dimensions */}
-								<StandardTooltip content={t("prompts:modes.importMode")}>
-									<button
-										onClick={() => setShowImportDialog(true)}
-										disabled={isImporting}
-										className={cn(
-											"relative inline-flex items-center justify-center",
-											"bg-transparent border-none p-1.5",
-											"rounded-md min-w-[28px] min-h-[28px]",
-											"text-vscode-foreground opacity-85",
-											"transition-all duration-150",
-											"hover:opacity-100 hover:bg-[rgba(255,255,255,0.03)] hover:border-[rgba(255,255,255,0.15)]",
-											"focus:outline-none focus-visible:ring-1 focus-visible:ring-vscode-focusBorder",
-											"active:bg-[rgba(255,255,255,0.1)]",
-											!isImporting && "cursor-pointer",
-											isImporting &&
-												"opacity-40 cursor-not-allowed grayscale-[30%] hover:bg-transparent hover:border-[rgba(255,255,255,0.08)] active:bg-transparent",
-										)}
-										style={{ fontSize: 16.5 }}>
-										<Download className="h-4 w-4" />
-									</button>
-								</StandardTooltip>
+								{/* Import button - using IconButton for consistency */}
+								<IconButton
+									iconClass="codicon-cloud-download"
+									title={t("prompts:modes.importMode")}
+									onClick={() => setShowImportDialog(true)}
+									disabled={isImporting}
+									isLoading={isImporting}
+								/>
 							</div>
 
 							{/* Info icon and title on the right - only show info icon when search bar is visible */}
@@ -398,11 +404,12 @@ export const ModeSelector = ({
 							{t("prompts:importMode.selectLevel")}
 						</p>
 						<div className="space-y-3 mb-6">
-							<label className="flex items-start gap-2 cursor-pointer">
+							<label className="flex items-start gap-2 cursor-pointer" htmlFor="import-level-project">
 								<input
 									type="radio"
 									name="importLevel"
 									value="project"
+									id="import-level-project"
 									className="mt-1"
 									defaultChecked
 								/>
@@ -413,8 +420,14 @@ export const ModeSelector = ({
 									</div>
 								</div>
 							</label>
-							<label className="flex items-start gap-2 cursor-pointer">
-								<input type="radio" name="importLevel" value="global" className="mt-1" />
+							<label className="flex items-start gap-2 cursor-pointer" htmlFor="import-level-global">
+								<input
+									type="radio"
+									name="importLevel"
+									value="global"
+									id="import-level-global"
+									className="mt-1"
+								/>
 								<div>
 									<div className="font-medium">{t("prompts:importMode.global.label")}</div>
 									<div className="text-xs text-vscode-descriptionForeground">
@@ -423,8 +436,15 @@ export const ModeSelector = ({
 								</div>
 							</label>
 						</div>
+						{/* Import error message */}
+						{importError && <div className="text-xs text-vscode-errorForeground mb-4">{importError}</div>}
 						<div className="flex justify-end gap-2">
-							<Button variant="secondary" onClick={() => setShowImportDialog(false)}>
+							<Button
+								variant="secondary"
+								onClick={() => {
+									setShowImportDialog(false)
+									setImportError(null)
+								}}>
 								{t("prompts:createModeDialog.buttons.cancel")}
 							</Button>
 							<Button variant="default" onClick={handleImportMode} disabled={isImporting}>

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

@@ -0,0 +1,439 @@
+import React from "react"
+import { render, screen, fireEvent, waitFor } from "@/utils/test-utils"
+import { describe, test, expect, vi, beforeEach } from "vitest"
+import ModeSelector from "../ModeSelector"
+import { Mode } from "@roo/modes"
+import { ModeConfig } from "@roo-code/types"
+
+// Mock the dependencies
+const mockPostMessage = vi.fn()
+vi.mock("@/utils/vscode", () => ({
+	vscode: {
+		postMessage: (message: any) => mockPostMessage(message),
+	},
+}))
+
+vi.mock("@/context/ExtensionStateContext", () => ({
+	useExtensionState: () => ({
+		hasOpenedModeSelector: false,
+		setHasOpenedModeSelector: vi.fn(),
+	}),
+}))
+
+vi.mock("@/i18n/TranslationContext", () => ({
+	useAppTranslation: () => ({
+		t: (key: string, _options?: any) => {
+			if (key === "prompts:exportMode.title") return "Export Mode"
+			if (key === "prompts:modes.importMode") return "Import Mode"
+			if (key === "prompts:importMode.selectLevel") return "Select import level"
+			if (key === "prompts:importMode.project.label") return "Project"
+			if (key === "prompts:importMode.project.description") return "Import to project"
+			if (key === "prompts:importMode.global.label") return "Global"
+			if (key === "prompts:importMode.global.description") return "Import globally"
+			if (key === "prompts:createModeDialog.buttons.cancel") return "Cancel"
+			if (key === "prompts:importMode.import") return "Import"
+			if (key === "prompts:importMode.importing") return "Importing..."
+			if (key === "prompts:exportMode.error") return "Export failed"
+			if (key === "prompts:importMode.error") return "Import failed"
+			if (key === "chat:modeSelector.marketplace") return "Marketplace"
+			if (key === "chat:modeSelector.settings") return "Settings"
+			if (key === "chat:modeSelector.searchPlaceholder") return "Search modes"
+			if (key === "chat:modeSelector.noResults") return "No results"
+			if (key === "chat:modeSelector.description") return "Select a mode"
+			if (key === "chat:modeSelector.title") return "Modes"
+			return key
+		},
+	}),
+}))
+
+vi.mock("@/components/ui/hooks/useRooPortal", () => ({
+	useRooPortal: () => document.body,
+}))
+
+vi.mock("@/utils/TelemetryClient", () => ({
+	telemetryClient: {
+		capture: vi.fn(),
+	},
+}))
+
+// Create a variable to control what getAllModes returns
+let mockModes: ModeConfig[] = []
+
+vi.mock("@roo/modes", async () => {
+	const actual = await vi.importActual<typeof import("@roo/modes")>("@roo/modes")
+	return {
+		...actual,
+		getAllModes: () => mockModes,
+	}
+})
+
+describe("ModeSelector Export/Import", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+		// Set up mock modes
+		mockModes = [
+			{
+				slug: "code",
+				name: "Code",
+				description: "Write code",
+				roleDefinition: "You are a coding assistant",
+				groups: ["read", "edit"],
+			},
+			{
+				slug: "architect",
+				name: "Architect",
+				description: "Design systems",
+				roleDefinition: "You are a system architect",
+				groups: ["read"],
+			},
+		]
+	})
+
+	describe("Export Functionality", () => {
+		test("export button is hidden by default and shows on hover", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Find the mode item
+			const modeItems = screen.getAllByTestId("mode-selector-item")
+			const codeItem = modeItems[0]
+
+			// Export button should have opacity-0 class initially
+			const exportButton = codeItem.querySelector('button[aria-label="Export Mode"]')
+			expect(exportButton).toHaveClass("opacity-0")
+
+			// Hover over the mode item
+			fireEvent.mouseEnter(codeItem)
+
+			// Export button should now have opacity-100 class
+			expect(exportButton).toHaveClass("group-hover:opacity-100")
+		})
+
+		test("clicking export button sends exportMode message", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Find and click the export button for the first mode
+			const modeItems = screen.getAllByTestId("mode-selector-item")
+			const exportButton = modeItems[0].querySelector('button[aria-label="Export Mode"]') as HTMLButtonElement
+			fireEvent.click(exportButton)
+
+			// Verify the message was sent
+			expect(mockPostMessage).toHaveBeenCalledWith({
+				type: "exportMode",
+				slug: "code",
+			})
+		})
+
+		test("export button shows loading state while exporting", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Find and click the export button
+			const modeItems = screen.getAllByTestId("mode-selector-item")
+			const exportButton = modeItems[0].querySelector('button[aria-label="Export Mode"]') as HTMLButtonElement
+			fireEvent.click(exportButton)
+
+			// Button should be disabled
+			expect(exportButton).toBeDisabled()
+		})
+
+		test("handles export success", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Click export
+			const modeItems = screen.getAllByTestId("mode-selector-item")
+			const exportButton = modeItems[0].querySelector('button[aria-label="Export Mode"]') as HTMLButtonElement
+			fireEvent.click(exportButton)
+
+			// Simulate success response
+			window.dispatchEvent(
+				new MessageEvent("message", {
+					data: {
+						type: "exportModeResult",
+						success: true,
+					},
+				}),
+			)
+
+			// Button should be enabled again
+			await waitFor(() => {
+				expect(exportButton).not.toBeDisabled()
+			})
+		})
+
+		test("handles export error", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Click export
+			const modeItems = screen.getAllByTestId("mode-selector-item")
+			const exportButton = modeItems[0].querySelector('button[aria-label="Export Mode"]') as HTMLButtonElement
+			fireEvent.click(exportButton)
+
+			// Simulate error response
+			window.dispatchEvent(
+				new MessageEvent("message", {
+					data: {
+						type: "exportModeResult",
+						success: false,
+						error: "Failed to save file",
+					},
+				}),
+			)
+
+			// Error message should be displayed
+			await waitFor(() => {
+				expect(screen.getByText("Failed to save file")).toBeInTheDocument()
+			})
+
+			// Button should be enabled again
+			expect(exportButton).not.toBeDisabled()
+		})
+	})
+
+	describe("Import Functionality", () => {
+		test("import button is visible in the footer", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Import button should be visible
+			const importButton = screen.getByRole("button", { name: "Import Mode" })
+			expect(importButton).toBeInTheDocument()
+			// Check for the icon inside the button
+			const icon = importButton.querySelector(".codicon-cloud-download")
+			expect(icon).toBeInTheDocument()
+		})
+
+		test("clicking import button opens import dialog", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Click import button
+			const importButton = screen.getByRole("button", { name: "Import Mode" })
+			fireEvent.click(importButton)
+
+			// Import dialog should be visible
+			expect(screen.getByText("Select import level")).toBeInTheDocument()
+			// Check for the text content instead of label association
+			expect(screen.getByText("Project")).toBeInTheDocument()
+			expect(screen.getByText("Global")).toBeInTheDocument()
+		})
+
+		test("import dialog has unique IDs for radio inputs", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Check for unique IDs
+			const projectRadio = document.getElementById("import-level-project") as HTMLInputElement
+			const globalRadio = document.getElementById("import-level-global") as HTMLInputElement
+
+			expect(projectRadio).toBeInTheDocument()
+			expect(globalRadio).toBeInTheDocument()
+			expect(projectRadio.id).toBe("import-level-project")
+			expect(globalRadio.id).toBe("import-level-global")
+		})
+
+		test("project level is selected by default", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Project should be checked by default
+			const projectRadio = document.getElementById("import-level-project") as HTMLInputElement
+			const globalRadio = document.getElementById("import-level-global") as HTMLInputElement
+
+			expect(projectRadio).toBeInTheDocument()
+			expect(globalRadio).toBeInTheDocument()
+			expect(projectRadio.checked).toBe(true)
+			expect(globalRadio.checked).toBe(false)
+		})
+
+		test("clicking import sends importMode message with selected level", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Select global
+			const globalRadio = document.getElementById("import-level-global") as HTMLInputElement
+			expect(globalRadio).toBeInTheDocument()
+			fireEvent.click(globalRadio)
+
+			// Click import
+			const importButton = screen.getByText("Import")
+			fireEvent.click(importButton)
+
+			// Verify the message was sent
+			expect(mockPostMessage).toHaveBeenCalledWith({
+				type: "importMode",
+				source: "global",
+			})
+		})
+
+		test("import button shows loading state while importing", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Click import
+			const importButton = screen.getByText("Import")
+			fireEvent.click(importButton)
+
+			// Button should show importing text
+			expect(screen.getByText("Importing...")).toBeInTheDocument()
+		})
+
+		test("handles import success", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Click import
+			const importButton = screen.getByText("Import")
+			fireEvent.click(importButton)
+
+			// Simulate success response
+			window.dispatchEvent(
+				new MessageEvent("message", {
+					data: {
+						type: "importModeResult",
+						success: true,
+					},
+				}),
+			)
+
+			// Dialog should be closed
+			await waitFor(() => {
+				expect(screen.queryByText("Select import level")).not.toBeInTheDocument()
+			})
+		})
+
+		test("handles import error", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Click import
+			const importButton = screen.getByText("Import")
+			fireEvent.click(importButton)
+
+			// Simulate error response
+			window.dispatchEvent(
+				new MessageEvent("message", {
+					data: {
+						type: "importModeResult",
+						success: false,
+						error: "Invalid file format",
+					},
+				}),
+			)
+
+			// Error message should be displayed
+			await waitFor(() => {
+				expect(screen.getByText("Invalid file format")).toBeInTheDocument()
+			})
+
+			// Dialog should still be open
+			expect(screen.getByText("Select import level")).toBeInTheDocument()
+		})
+
+		test("handles cancelled import", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Click import
+			const importButton = screen.getByText("Import")
+			fireEvent.click(importButton)
+
+			// Simulate cancelled response
+			window.dispatchEvent(
+				new MessageEvent("message", {
+					data: {
+						type: "importModeResult",
+						success: false,
+						error: "cancelled",
+					},
+				}),
+			)
+
+			// Dialog should be closed and no error shown
+			await waitFor(() => {
+				expect(screen.queryByText("Select import level")).not.toBeInTheDocument()
+			})
+			expect(screen.queryByText("cancelled")).not.toBeInTheDocument()
+		})
+
+		test("cancel button closes import dialog", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover and import dialog
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+			fireEvent.click(screen.getByRole("button", { name: "Import Mode" }))
+
+			// Click cancel
+			const cancelButton = screen.getByText("Cancel")
+			fireEvent.click(cancelButton)
+
+			// Dialog should be closed
+			expect(screen.queryByText("Select import level")).not.toBeInTheDocument()
+		})
+	})
+
+	describe("Accessibility", () => {
+		test("export button has proper aria-label", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Find export button
+			const modeItems = screen.getAllByTestId("mode-selector-item")
+			const exportButton = modeItems[0].querySelector('button[aria-label="Export Mode"]')
+
+			expect(exportButton).toHaveAttribute("aria-label", "Export Mode")
+		})
+
+		test("import button uses IconButton component", async () => {
+			render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />)
+
+			// Open the popover
+			fireEvent.click(screen.getByTestId("mode-selector-trigger"))
+
+			// Import button should have proper IconButton structure
+			const importButton = screen.getByRole("button", { name: "Import Mode" })
+			expect(importButton).toHaveAttribute("aria-label", "Import Mode")
+			expect(importButton.querySelector(".codicon-cloud-download")).toBeInTheDocument()
+		})
+	})
+})