mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-07 02:58:15 +00:00
feat: add ESC key handling for modes, API provider, and indexing settings popovers (#6175)
Co-authored-by: Roo Code <roomote@roocode.com> Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
This commit is contained in:
roomote[bot]
GitHub
parent
41aaf07861
commit
e3a8e03905
9 changed files with 324 additions and 381 deletions
|
|
@ -253,4 +253,67 @@ describe("SearchableSelect", () => {
|
|||
expect(within(dropdown).queryByText("Option 3")).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it("closes the dropdown when ESC key is pressed", async () => {
|
||||
const user = userEvent.setup()
|
||||
render(<SearchableSelect {...defaultProps} />)
|
||||
|
||||
// Open the dropdown
|
||||
const trigger = screen.getByRole("combobox")
|
||||
await user.click(trigger)
|
||||
|
||||
// Verify dropdown is open
|
||||
expect(trigger).toHaveAttribute("aria-expanded", "true")
|
||||
expect(screen.getByRole("listbox")).toBeInTheDocument()
|
||||
|
||||
// Press ESC key
|
||||
fireEvent.keyDown(window, { key: "Escape" })
|
||||
|
||||
// Verify dropdown is closed
|
||||
await waitFor(() => {
|
||||
expect(trigger).toHaveAttribute("aria-expanded", "false")
|
||||
expect(screen.queryByRole("listbox")).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it("does not close the dropdown when ESC is pressed while dropdown is closed", async () => {
|
||||
render(<SearchableSelect {...defaultProps} />)
|
||||
|
||||
const trigger = screen.getByRole("combobox")
|
||||
|
||||
// Ensure dropdown is closed
|
||||
expect(trigger).toHaveAttribute("aria-expanded", "false")
|
||||
|
||||
// Press ESC key
|
||||
fireEvent.keyDown(window, { key: "Escape" })
|
||||
|
||||
// Verify dropdown remains closed
|
||||
expect(trigger).toHaveAttribute("aria-expanded", "false")
|
||||
expect(screen.queryByRole("listbox")).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("prevents default and stops propagation when ESC is pressed", async () => {
|
||||
const user = userEvent.setup()
|
||||
render(<SearchableSelect {...defaultProps} />)
|
||||
|
||||
// Open the dropdown
|
||||
const trigger = screen.getByRole("combobox")
|
||||
await user.click(trigger)
|
||||
|
||||
// Create a mock event to track preventDefault and stopPropagation
|
||||
const escapeEvent = new KeyboardEvent("keydown", {
|
||||
key: "Escape",
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
})
|
||||
const preventDefaultSpy = vi.spyOn(escapeEvent, "preventDefault")
|
||||
const stopPropagationSpy = vi.spyOn(escapeEvent, "stopPropagation")
|
||||
|
||||
// Dispatch the event
|
||||
window.dispatchEvent(escapeEvent)
|
||||
|
||||
// Verify preventDefault and stopPropagation were called
|
||||
expect(preventDefaultSpy).toHaveBeenCalled()
|
||||
expect(stopPropagationSpy).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import {
|
|||
} from "@src/components/ui"
|
||||
import { AlertTriangle } from "lucide-react"
|
||||
import { useRooPortal } from "@src/components/ui/hooks/useRooPortal"
|
||||
import { useEscapeKey } from "@src/hooks/useEscapeKey"
|
||||
import type { EmbedderProvider } from "@roo/embeddingModels"
|
||||
import type { IndexingStatus } from "@roo/ExtensionMessage"
|
||||
import { CODEBASE_INDEX_DEFAULTS } from "@roo-code/types"
|
||||
|
|
@ -440,6 +441,9 @@ export const CodeIndexPopover: React.FC<CodeIndexPopoverProps> = ({
|
|||
})
|
||||
}, [checkUnsavedChanges])
|
||||
|
||||
// Use the shared ESC key handler hook - respects unsaved changes logic
|
||||
useEscapeKey(open, handlePopoverClose)
|
||||
|
||||
const handleSaveSettings = () => {
|
||||
// Validate settings before saving
|
||||
if (!validateSettings()) {
|
||||
|
|
|
|||
|
|
@ -1,381 +0,0 @@
|
|||
import React from "react"
|
||||
import { render, screen, fireEvent, waitFor } from "@testing-library/react"
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest"
|
||||
import { CodeIndexPopover } from "../CodeIndexPopover"
|
||||
|
||||
// Mock the vscode API
|
||||
vi.mock("@src/utils/vscode", () => ({
|
||||
vscode: {
|
||||
postMessage: vi.fn(),
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock the extension state context
|
||||
vi.mock("@src/context/ExtensionStateContext", () => ({
|
||||
useExtensionState: vi.fn(),
|
||||
}))
|
||||
|
||||
// Mock the translation context
|
||||
vi.mock("@src/i18n/TranslationContext", () => ({
|
||||
useAppTranslation: () => ({ t: vi.fn((key: string) => key) }),
|
||||
}))
|
||||
|
||||
// Mock react-i18next
|
||||
vi.mock("react-i18next", () => ({
|
||||
Trans: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
}))
|
||||
|
||||
// Mock the doc links utility
|
||||
vi.mock("@src/utils/docLinks", () => ({
|
||||
buildDocLink: vi.fn(() => "https://docs.roocode.com"),
|
||||
}))
|
||||
|
||||
// Mock the portal hook
|
||||
vi.mock("@src/components/ui/hooks/useRooPortal", () => ({
|
||||
useRooPortal: () => ({ portalContainer: document.body }),
|
||||
}))
|
||||
|
||||
// Mock Radix UI components to avoid portal issues
|
||||
vi.mock("@src/components/ui", () => ({
|
||||
Popover: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
PopoverContent: ({ children }: { children: React.ReactNode }) => <div role="dialog">{children}</div>,
|
||||
PopoverTrigger: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
Select: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
SelectContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
SelectItem: ({ children, value }: { children: React.ReactNode; value: string }) => (
|
||||
<div role="option" data-value={value}>
|
||||
{children}
|
||||
</div>
|
||||
),
|
||||
SelectTrigger: ({ children }: { children: React.ReactNode }) => <div role="combobox">{children}</div>,
|
||||
SelectValue: ({ placeholder }: { placeholder?: string }) => <span>{placeholder}</span>,
|
||||
AlertDialog: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
AlertDialogAction: ({ children }: { children: React.ReactNode }) => <button>{children}</button>,
|
||||
AlertDialogCancel: ({ children }: { children: React.ReactNode }) => <button>{children}</button>,
|
||||
AlertDialogContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
AlertDialogDescription: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
AlertDialogFooter: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
AlertDialogHeader: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
AlertDialogTitle: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
AlertDialogTrigger: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
Slider: ({ value, onValueChange }: { value: number[]; onValueChange: (value: number[]) => void }) => (
|
||||
<input type="range" value={value[0]} onChange={(e) => onValueChange([parseInt(e.target.value)])} />
|
||||
),
|
||||
StandardTooltip: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
cn: (...classes: string[]) => classes.join(" "),
|
||||
}))
|
||||
|
||||
// Mock VSCode web components to behave like regular HTML inputs
|
||||
vi.mock("@vscode/webview-ui-toolkit/react", () => ({
|
||||
VSCodeTextField: ({ value, onInput, placeholder, className, ...rest }: any) => (
|
||||
<input
|
||||
type="text"
|
||||
value={value || ""}
|
||||
onChange={(e) => onInput && onInput(e)}
|
||||
placeholder={placeholder}
|
||||
className={className}
|
||||
aria-label="Text field"
|
||||
{...rest}
|
||||
/>
|
||||
),
|
||||
VSCodeButton: ({ children, onClick, ...rest }: any) => (
|
||||
<button onClick={onClick} {...rest}>
|
||||
{children}
|
||||
</button>
|
||||
),
|
||||
VSCodeDropdown: ({ value, onChange, children, className, ...rest }: any) => (
|
||||
<select
|
||||
value={value || ""}
|
||||
onChange={(e) => onChange && onChange(e)}
|
||||
className={className}
|
||||
role="combobox"
|
||||
{...rest}>
|
||||
{children}
|
||||
</select>
|
||||
),
|
||||
VSCodeOption: ({ value, children, ...rest }: any) => (
|
||||
<option value={value} {...rest}>
|
||||
{children}
|
||||
</option>
|
||||
),
|
||||
VSCodeLink: ({ href, children, ...rest }: any) => (
|
||||
<a href={href} {...rest}>
|
||||
{children}
|
||||
</a>
|
||||
),
|
||||
VSCodeCheckbox: ({ checked, onChange, children, ...rest }: any) => (
|
||||
<label>
|
||||
<input type="checkbox" checked={checked || false} onChange={(e) => onChange && onChange(e)} {...rest} />
|
||||
{children}
|
||||
</label>
|
||||
),
|
||||
}))
|
||||
|
||||
// Helper function to simulate input on form elements
|
||||
const simulateInput = (element: Element, value: string) => {
|
||||
// Now that we're mocking VSCode components as regular HTML inputs,
|
||||
// we can use standard fireEvent.change
|
||||
fireEvent.change(element, { target: { value } })
|
||||
}
|
||||
|
||||
describe("CodeIndexPopover Validation", () => {
|
||||
let mockUseExtensionState: any
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Get the mocked function
|
||||
const { useExtensionState } = await import("@src/context/ExtensionStateContext")
|
||||
mockUseExtensionState = vi.mocked(useExtensionState)
|
||||
|
||||
// Setup default extension state
|
||||
mockUseExtensionState.mockReturnValue({
|
||||
codebaseIndexConfig: {
|
||||
codebaseIndexEnabled: false,
|
||||
codebaseIndexQdrantUrl: "",
|
||||
codebaseIndexEmbedderProvider: "openai",
|
||||
codebaseIndexEmbedderBaseUrl: "",
|
||||
codebaseIndexEmbedderModelId: "",
|
||||
codebaseIndexSearchMaxResults: 10,
|
||||
codebaseIndexSearchMinScore: 0.7,
|
||||
codebaseIndexOpenAiCompatibleBaseUrl: "",
|
||||
codebaseIndexEmbedderModelDimension: undefined,
|
||||
},
|
||||
codebaseIndexModels: {
|
||||
openai: [{ dimension: 1536 }],
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
const renderComponent = () => {
|
||||
return render(
|
||||
<CodeIndexPopover indexingStatus={{ systemStatus: "idle", message: "", processedItems: 0, totalItems: 0 }}>
|
||||
<button>Test Trigger</button>
|
||||
</CodeIndexPopover>,
|
||||
)
|
||||
}
|
||||
|
||||
const openPopover = async () => {
|
||||
const trigger = screen.getByText("Test Trigger")
|
||||
fireEvent.click(trigger)
|
||||
|
||||
// Wait for popover to open
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole("dialog")).toBeInTheDocument()
|
||||
})
|
||||
}
|
||||
|
||||
const expandSetupSection = async () => {
|
||||
const setupButton = screen.getByText("settings:codeIndex.setupConfigLabel")
|
||||
fireEvent.click(setupButton)
|
||||
|
||||
// Wait for section to expand - look for vscode-text-field elements
|
||||
await waitFor(() => {
|
||||
const textFields = screen.getAllByLabelText("Text field")
|
||||
expect(textFields.length).toBeGreaterThan(0)
|
||||
})
|
||||
}
|
||||
|
||||
describe("OpenAI Provider Validation", () => {
|
||||
it("should show validation error when OpenAI API key is empty", async () => {
|
||||
renderComponent()
|
||||
await openPopover()
|
||||
await expandSetupSection()
|
||||
|
||||
// First, make a change to enable the save button by modifying the Qdrant URL
|
||||
const qdrantUrlField = screen.getByPlaceholderText(/settings:codeIndex.qdrantUrlPlaceholder/i)
|
||||
fireEvent.change(qdrantUrlField, { target: { value: "http://localhost:6333" } })
|
||||
|
||||
// Wait for the save button to become enabled
|
||||
await waitFor(() => {
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
expect(saveButton).not.toBeDisabled()
|
||||
})
|
||||
|
||||
// Now clear the OpenAI API key to create a validation error
|
||||
const apiKeyField = screen.getByPlaceholderText(/settings:codeIndex.openAiKeyPlaceholder/i)
|
||||
fireEvent.change(apiKeyField, { target: { value: "" } })
|
||||
|
||||
// Click the save button to trigger validation
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
// Should show specific field error
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("settings:codeIndex.validation.openaiApiKeyRequired")).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it("should show validation error when model is not selected", async () => {
|
||||
renderComponent()
|
||||
await openPopover()
|
||||
await expandSetupSection()
|
||||
|
||||
// First, make a change to enable the save button
|
||||
const qdrantUrlField = screen.getByPlaceholderText(/settings:codeIndex.qdrantUrlPlaceholder/i)
|
||||
fireEvent.change(qdrantUrlField, { target: { value: "http://localhost:6333" } })
|
||||
|
||||
// Set API key but leave model empty
|
||||
const apiKeyField = screen.getByPlaceholderText(/settings:codeIndex.openAiKeyPlaceholder/i)
|
||||
fireEvent.change(apiKeyField, { target: { value: "test-api-key" } })
|
||||
|
||||
// Wait for the save button to become enabled
|
||||
await waitFor(() => {
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
expect(saveButton).not.toBeDisabled()
|
||||
})
|
||||
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("settings:codeIndex.validation.modelSelectionRequired")).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("Qdrant URL Validation", () => {
|
||||
it("should show validation error when Qdrant URL is empty", async () => {
|
||||
renderComponent()
|
||||
await openPopover()
|
||||
await expandSetupSection()
|
||||
|
||||
// First, make a change to enable the save button by setting API key
|
||||
const apiKeyField = screen.getByPlaceholderText(/settings:codeIndex.openAiKeyPlaceholder/i)
|
||||
fireEvent.change(apiKeyField, { target: { value: "test-api-key" } })
|
||||
|
||||
// Clear the Qdrant URL to create validation error
|
||||
const qdrantUrlField = screen.getByPlaceholderText(/settings:codeIndex.qdrantUrlPlaceholder/i)
|
||||
fireEvent.change(qdrantUrlField, { target: { value: "" } })
|
||||
|
||||
// Wait for the save button to become enabled
|
||||
await waitFor(() => {
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
expect(saveButton).not.toBeDisabled()
|
||||
})
|
||||
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("settings:codeIndex.validation.invalidQdrantUrl")).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it("should show validation error when Qdrant URL is invalid", async () => {
|
||||
renderComponent()
|
||||
await openPopover()
|
||||
await expandSetupSection()
|
||||
|
||||
// First, make a change to enable the save button by setting API key
|
||||
const apiKeyField = screen.getByPlaceholderText(/settings:codeIndex.openAiKeyPlaceholder/i)
|
||||
fireEvent.change(apiKeyField, { target: { value: "test-api-key" } })
|
||||
|
||||
// Set invalid Qdrant URL
|
||||
const qdrantUrlField = screen.getByPlaceholderText(/settings:codeIndex.qdrantUrlPlaceholder/i)
|
||||
fireEvent.change(qdrantUrlField, { target: { value: "invalid-url" } })
|
||||
|
||||
// Wait for the save button to become enabled
|
||||
await waitFor(() => {
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
expect(saveButton).not.toBeDisabled()
|
||||
})
|
||||
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("settings:codeIndex.validation.invalidQdrantUrl")).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("Common Field Validation", () => {
|
||||
it("should not show validation error for optional Qdrant API key", async () => {
|
||||
renderComponent()
|
||||
await openPopover()
|
||||
await expandSetupSection()
|
||||
|
||||
// Set required fields to make form valid
|
||||
const qdrantUrlField = screen.getByPlaceholderText(/settings:codeIndex.qdrantUrlPlaceholder/i)
|
||||
fireEvent.change(qdrantUrlField, { target: { value: "http://localhost:6333" } })
|
||||
|
||||
const apiKeyField = screen.getByPlaceholderText(/settings:codeIndex.openAiKeyPlaceholder/i)
|
||||
fireEvent.change(apiKeyField, { target: { value: "test-api-key" } })
|
||||
|
||||
// Select a model - this is required (get the select element specifically)
|
||||
const modelSelect = screen.getAllByRole("combobox").find((el) => el.tagName === "SELECT")
|
||||
if (modelSelect) {
|
||||
fireEvent.change(modelSelect, { target: { value: "0" } })
|
||||
}
|
||||
|
||||
// Leave Qdrant API key empty (it's optional)
|
||||
const qdrantApiKeyField = screen.getByPlaceholderText(/settings:codeIndex.qdrantApiKeyPlaceholder/i)
|
||||
fireEvent.change(qdrantApiKeyField, { target: { value: "" } })
|
||||
|
||||
// Wait for the save button to become enabled
|
||||
await waitFor(() => {
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
expect(saveButton).not.toBeDisabled()
|
||||
})
|
||||
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
// Should not show validation errors since Qdrant API key is optional
|
||||
})
|
||||
|
||||
it("should clear validation errors when fields are corrected", async () => {
|
||||
renderComponent()
|
||||
await openPopover()
|
||||
await expandSetupSection()
|
||||
|
||||
// First make an invalid change to enable the save button and trigger validation
|
||||
const textFields = screen.getAllByLabelText("Text field")
|
||||
const qdrantField = textFields.find((field) =>
|
||||
field.getAttribute("placeholder")?.toLowerCase().includes("qdrant"),
|
||||
)
|
||||
|
||||
if (qdrantField) {
|
||||
simulateInput(qdrantField, "invalid-url") // Invalid URL to trigger validation
|
||||
}
|
||||
|
||||
// Wait for save button to be enabled
|
||||
const saveButton = screen.getByText("settings:codeIndex.saveSettings")
|
||||
await waitFor(() => {
|
||||
expect(saveButton).not.toBeDisabled()
|
||||
})
|
||||
|
||||
// Click save to trigger validation errors
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
// Now fix the errors with valid values
|
||||
const apiKeyField = textFields.find(
|
||||
(field) =>
|
||||
field.getAttribute("placeholder")?.toLowerCase().includes("openai") ||
|
||||
field.getAttribute("placeholder")?.toLowerCase().includes("key"),
|
||||
)
|
||||
|
||||
// Set valid Qdrant URL
|
||||
if (qdrantField) {
|
||||
simulateInput(qdrantField, "http://localhost:6333")
|
||||
}
|
||||
|
||||
// Set API key
|
||||
if (apiKeyField) {
|
||||
simulateInput(apiKeyField, "test-api-key")
|
||||
}
|
||||
|
||||
// Select a model - this is required (get the select element specifically)
|
||||
const modelSelect = screen.getAllByRole("combobox").find((el) => el.tagName === "SELECT")
|
||||
if (modelSelect) {
|
||||
fireEvent.change(modelSelect, { target: { value: "0" } })
|
||||
}
|
||||
|
||||
// Try to save again
|
||||
fireEvent.click(saveButton)
|
||||
|
||||
// Validation errors should be cleared (specific field errors are checked elsewhere)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -48,6 +48,7 @@ import {
|
|||
StandardTooltip,
|
||||
} from "@src/components/ui"
|
||||
import { DeleteModeDialog } from "@src/components/modes/DeleteModeDialog"
|
||||
import { useEscapeKey } from "@src/hooks/useEscapeKey"
|
||||
|
||||
// Get all available groups that should show in prompts view
|
||||
const availableGroups = (Object.keys(TOOL_GROUPS) as ToolGroup[]).filter((group) => !TOOL_GROUPS[group].alwaysAvailable)
|
||||
|
|
@ -194,6 +195,9 @@ const ModesView = ({ onDone }: ModesViewProps) => {
|
|||
}
|
||||
}, [])
|
||||
|
||||
// Use the shared ESC key handler hook
|
||||
useEscapeKey(open, () => setOpen(false))
|
||||
|
||||
// Handler for clearing search input
|
||||
const onClearSearch = useCallback(() => {
|
||||
setSearchValue("")
|
||||
|
|
|
|||
|
|
@ -231,4 +231,37 @@ describe("PromptsView", () => {
|
|||
text: undefined,
|
||||
})
|
||||
})
|
||||
|
||||
it("closes the mode selection popover when ESC key is pressed", async () => {
|
||||
renderPromptsView()
|
||||
const selectTrigger = screen.getByTestId("mode-select-trigger")
|
||||
|
||||
// Open the popover
|
||||
fireEvent.click(selectTrigger)
|
||||
await waitFor(() => {
|
||||
expect(selectTrigger).toHaveAttribute("aria-expanded", "true")
|
||||
})
|
||||
|
||||
// Press ESC key
|
||||
fireEvent.keyDown(window, { key: "Escape" })
|
||||
|
||||
// Verify popover is closed
|
||||
await waitFor(() => {
|
||||
expect(selectTrigger).toHaveAttribute("aria-expanded", "false")
|
||||
})
|
||||
})
|
||||
|
||||
it("does not close the popover when ESC is pressed while popover is closed", async () => {
|
||||
renderPromptsView()
|
||||
const selectTrigger = screen.getByTestId("mode-select-trigger")
|
||||
|
||||
// Ensure popover is closed
|
||||
expect(selectTrigger).toHaveAttribute("aria-expanded", "false")
|
||||
|
||||
// Press ESC key
|
||||
fireEvent.keyDown(window, { key: "Escape" })
|
||||
|
||||
// Verify popover remains closed
|
||||
expect(selectTrigger).toHaveAttribute("aria-expanded", "false")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import {
|
|||
PopoverTrigger,
|
||||
Button,
|
||||
} from "@src/components/ui"
|
||||
import { useEscapeKey } from "@src/hooks/useEscapeKey"
|
||||
|
||||
import { ModelInfoView } from "./ModelInfoView"
|
||||
import { ApiErrorMessage } from "./ApiErrorMessage"
|
||||
|
|
@ -133,6 +134,9 @@ export const ModelPicker = ({
|
|||
}
|
||||
}, [])
|
||||
|
||||
// Use the shared ESC key handler hook
|
||||
useEscapeKey(open, () => setOpen(false))
|
||||
|
||||
return (
|
||||
<>
|
||||
<div>
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import {
|
|||
PopoverContent,
|
||||
PopoverTrigger,
|
||||
} from "@/components/ui"
|
||||
import { useEscapeKey } from "@/hooks/useEscapeKey"
|
||||
|
||||
export interface SearchableSelectOption {
|
||||
value: string
|
||||
|
|
@ -79,6 +80,9 @@ export function SearchableSelect({
|
|||
return () => clearTimeout(timeoutId)
|
||||
}, [value])
|
||||
|
||||
// Use the shared ESC key handler hook
|
||||
useEscapeKey(open, () => setOpen(false))
|
||||
|
||||
const handleOpenChange = (open: boolean) => {
|
||||
setOpen(open)
|
||||
// Reset search when closing
|
||||
|
|
|
|||
168
webview-ui/src/hooks/useEscapeKey.spec.ts
Normal file
168
webview-ui/src/hooks/useEscapeKey.spec.ts
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
import { renderHook } from "@testing-library/react"
|
||||
import { vi } from "vitest"
|
||||
import { useEscapeKey } from "./useEscapeKey"
|
||||
|
||||
describe("useEscapeKey", () => {
|
||||
let mockOnEscape: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
mockOnEscape = vi.fn()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it("should call onEscape when Escape key is pressed and isOpen is true", () => {
|
||||
renderHook(() => useEscapeKey(true, mockOnEscape))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(mockOnEscape).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it("should not call onEscape when Escape key is pressed and isOpen is false", () => {
|
||||
renderHook(() => useEscapeKey(false, mockOnEscape))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(mockOnEscape).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should not call onEscape when a different key is pressed", () => {
|
||||
renderHook(() => useEscapeKey(true, mockOnEscape))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Enter" })
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(mockOnEscape).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should prevent default when preventDefault option is true", () => {
|
||||
renderHook(() => useEscapeKey(true, mockOnEscape, { preventDefault: true }))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
const preventDefaultSpy = vi.spyOn(event, "preventDefault")
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(preventDefaultSpy).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should not prevent default when preventDefault option is false", () => {
|
||||
renderHook(() => useEscapeKey(true, mockOnEscape, { preventDefault: false }))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
const preventDefaultSpy = vi.spyOn(event, "preventDefault")
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(preventDefaultSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should stop propagation when stopPropagation option is true", () => {
|
||||
renderHook(() => useEscapeKey(true, mockOnEscape, { stopPropagation: true }))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
const stopPropagationSpy = vi.spyOn(event, "stopPropagation")
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(stopPropagationSpy).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should not stop propagation when stopPropagation option is false", () => {
|
||||
renderHook(() => useEscapeKey(true, mockOnEscape, { stopPropagation: false }))
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
const stopPropagationSpy = vi.spyOn(event, "stopPropagation")
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(stopPropagationSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should remove event listener on unmount", () => {
|
||||
const addEventListenerSpy = vi.spyOn(window, "addEventListener")
|
||||
const removeEventListenerSpy = vi.spyOn(window, "removeEventListener")
|
||||
|
||||
const { unmount } = renderHook(() => useEscapeKey(true, mockOnEscape))
|
||||
|
||||
expect(addEventListenerSpy).toHaveBeenCalledWith("keydown", expect.any(Function))
|
||||
|
||||
unmount()
|
||||
|
||||
expect(removeEventListenerSpy).toHaveBeenCalledWith("keydown", expect.any(Function))
|
||||
})
|
||||
|
||||
it("should always add event listener regardless of isOpen state", () => {
|
||||
const addEventListenerSpy = vi.spyOn(window, "addEventListener")
|
||||
const removeEventListenerSpy = vi.spyOn(window, "removeEventListener")
|
||||
|
||||
// Test with isOpen = false
|
||||
const { rerender } = renderHook(({ isOpen }) => useEscapeKey(isOpen, mockOnEscape), {
|
||||
initialProps: { isOpen: false },
|
||||
})
|
||||
|
||||
expect(addEventListenerSpy).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Change to isOpen = true
|
||||
rerender({ isOpen: true })
|
||||
|
||||
// The listener is re-added because handleKeyDown changes when isOpen changes
|
||||
// This is expected behavior - the old listener is removed and a new one is added
|
||||
expect(addEventListenerSpy).toHaveBeenCalledTimes(2)
|
||||
expect(removeEventListenerSpy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it("should handle rapid isOpen state changes without memory leaks", () => {
|
||||
const addEventListenerSpy = vi.spyOn(window, "addEventListener")
|
||||
const removeEventListenerSpy = vi.spyOn(window, "removeEventListener")
|
||||
|
||||
const { rerender, unmount } = renderHook(({ isOpen }) => useEscapeKey(isOpen, mockOnEscape), {
|
||||
initialProps: { isOpen: false },
|
||||
})
|
||||
|
||||
// Initial render
|
||||
expect(addEventListenerSpy).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Rapid state changes
|
||||
rerender({ isOpen: true })
|
||||
rerender({ isOpen: false })
|
||||
rerender({ isOpen: true })
|
||||
|
||||
// Each rerender causes the effect to re-run because handleKeyDown changes
|
||||
expect(addEventListenerSpy).toHaveBeenCalledTimes(4)
|
||||
// Each re-run also removes the previous listener
|
||||
expect(removeEventListenerSpy).toHaveBeenCalledTimes(3)
|
||||
|
||||
// Unmount while isOpen is true
|
||||
unmount()
|
||||
|
||||
// Should properly clean up the final listener
|
||||
expect(removeEventListenerSpy).toHaveBeenCalledTimes(4)
|
||||
|
||||
// Verify that all listeners were properly cleaned up
|
||||
expect(addEventListenerSpy).toHaveBeenCalledTimes(removeEventListenerSpy.mock.calls.length)
|
||||
})
|
||||
|
||||
it("should update callback when dependencies change", () => {
|
||||
const { rerender } = renderHook(({ isOpen, onEscape }) => useEscapeKey(isOpen, onEscape), {
|
||||
initialProps: { isOpen: true, onEscape: mockOnEscape },
|
||||
})
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "Escape" })
|
||||
window.dispatchEvent(event)
|
||||
|
||||
expect(mockOnEscape).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Change the callback
|
||||
const newMockOnEscape = vi.fn()
|
||||
rerender({ isOpen: true, onEscape: newMockOnEscape })
|
||||
|
||||
window.dispatchEvent(event)
|
||||
|
||||
// Old callback should not be called again
|
||||
expect(mockOnEscape).toHaveBeenCalledTimes(1)
|
||||
// New callback should be called
|
||||
expect(newMockOnEscape).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
44
webview-ui/src/hooks/useEscapeKey.ts
Normal file
44
webview-ui/src/hooks/useEscapeKey.ts
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
import { useEffect, useCallback } from "react"
|
||||
|
||||
/**
|
||||
* Custom hook for handling ESC key press events
|
||||
* @param isOpen - Whether the component is currently open/visible
|
||||
* @param onEscape - Callback function to execute when ESC is pressed
|
||||
* @param options - Additional options for the hook
|
||||
*/
|
||||
export function useEscapeKey(
|
||||
isOpen: boolean,
|
||||
onEscape: () => void,
|
||||
options: {
|
||||
preventDefault?: boolean
|
||||
stopPropagation?: boolean
|
||||
} = {},
|
||||
) {
|
||||
const { preventDefault = true, stopPropagation = true } = options
|
||||
|
||||
const handleKeyDown = useCallback(
|
||||
(event: KeyboardEvent) => {
|
||||
// Check isOpen inside the handler to ensure proper cleanup
|
||||
if (event.key === "Escape" && isOpen) {
|
||||
if (preventDefault) {
|
||||
event.preventDefault()
|
||||
}
|
||||
if (stopPropagation) {
|
||||
event.stopPropagation()
|
||||
}
|
||||
onEscape()
|
||||
}
|
||||
},
|
||||
[isOpen, onEscape, preventDefault, stopPropagation],
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
// Always add the event listener to ensure proper cleanup on unmount
|
||||
// The isOpen check is now inside the handler
|
||||
window.addEventListener("keydown", handleKeyDown)
|
||||
|
||||
return () => {
|
||||
window.removeEventListener("keydown", handleKeyDown)
|
||||
}
|
||||
}, [handleKeyDown])
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue