diff --git a/web/app/components/goto-anything/__tests__/index.spec.tsx b/web/app/components/goto-anything/__tests__/index.spec.tsx index 6ad36b47bc..5c704c2268 100644 --- a/web/app/components/goto-anything/__tests__/index.spec.tsx +++ b/web/app/components/goto-anything/__tests__/index.spec.tsx @@ -2,6 +2,7 @@ import type { ReactNode } from 'react' import type { ActionItem, SearchResult } from '../actions/types' import { act, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { createStore, Provider } from 'jotai' import * as React from 'react' import { GotoAnything } from '../index' @@ -49,7 +50,6 @@ vi.mock('@tanstack/react-hotkeys', async (importOriginal) => { const HOTKEY_ALIAS: Record = { 'ctrl.k': 'Mod+K', - 'esc': 'Escape', } const triggerKeyPress = (combo: string) => { @@ -135,6 +135,16 @@ vi.mock('../../plugins/install-plugin/install-from-marketplace', () => ({ ), })) +const renderGotoAnything = (ui: React.ReactElement) => { + const store = createStore() + + return render( + + {ui} + , + ) +} + describe('GotoAnything', () => { beforeEach(() => { routerPush.mockClear() @@ -147,7 +157,7 @@ describe('GotoAnything', () => { describe('modal behavior', () => { it('should open modal via Ctrl+K shortcut', async () => { - render() + renderGotoAnything() triggerKeyPress('ctrl.k') @@ -157,21 +167,22 @@ describe('GotoAnything', () => { }) it('should close modal via ESC key', async () => { - render() + const user = userEvent.setup() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { expect(screen.getByPlaceholderText('app.gotoAnything.searchPlaceholder')).toBeInTheDocument() }) - triggerKeyPress('esc') + await user.keyboard('{Escape}') await waitFor(() => { expect(screen.queryByPlaceholderText('app.gotoAnything.searchPlaceholder')).not.toBeInTheDocument() }) }) it('should toggle modal when pressing Ctrl+K twice', async () => { - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -185,15 +196,16 @@ describe('GotoAnything', () => { }) it('should call onHide when modal closes', async () => { + const user = userEvent.setup() const onHide = vi.fn() - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { expect(screen.getByPlaceholderText('app.gotoAnything.searchPlaceholder')).toBeInTheDocument() }) - triggerKeyPress('esc') + await user.keyboard('{Escape}') await waitFor(() => { expect(onHide).toHaveBeenCalled() }) @@ -201,7 +213,7 @@ describe('GotoAnything', () => { it('should reset search query when modal opens', async () => { const user = userEvent.setup() - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -211,7 +223,7 @@ describe('GotoAnything', () => { const input = screen.getByPlaceholderText('app.gotoAnything.searchPlaceholder') await user.type(input, 'test') - triggerKeyPress('esc') + await user.keyboard('{Escape}') await waitFor(() => { expect(screen.queryByPlaceholderText('app.gotoAnything.searchPlaceholder')).not.toBeInTheDocument() }) @@ -242,7 +254,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -260,7 +272,7 @@ describe('GotoAnything', () => { it('should clear selection when typing without prefix', async () => { const user = userEvent.setup() - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -284,7 +296,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -308,7 +320,7 @@ describe('GotoAnything', () => { error: testError, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -322,7 +334,7 @@ describe('GotoAnything', () => { }) it('should show default state when no query', async () => { - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -341,7 +353,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -376,7 +388,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -412,7 +424,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -453,7 +465,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -485,7 +497,7 @@ describe('GotoAnything', () => { isAvailable: () => true, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -508,7 +520,7 @@ describe('GotoAnything', () => { isAvailable: () => false, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -530,7 +542,7 @@ describe('GotoAnything', () => { execute: executeMock, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -552,7 +564,7 @@ describe('GotoAnything', () => { isAvailable: () => true, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -587,7 +599,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { @@ -620,7 +632,7 @@ describe('GotoAnything', () => { error: null, } - render() + renderGotoAnything() triggerKeyPress('ctrl.k') await waitFor(() => { diff --git a/web/app/components/goto-anything/atoms.ts b/web/app/components/goto-anything/atoms.ts new file mode 100644 index 0000000000..50ea4c6ffb --- /dev/null +++ b/web/app/components/goto-anything/atoms.ts @@ -0,0 +1,13 @@ +'use client' + +import { atom, useAtomValue, useSetAtom } from 'jotai' + +const gotoAnythingOpenAtom = atom(false) + +export function useGotoAnythingOpen() { + return useAtomValue(gotoAnythingOpenAtom) +} + +export function useSetGotoAnythingOpen() { + return useSetAtom(gotoAnythingOpenAtom) +} diff --git a/web/app/components/goto-anything/hooks/__tests__/use-goto-anything-modal.spec.ts b/web/app/components/goto-anything/hooks/__tests__/use-goto-anything-modal.spec.ts index 27d283a38b..67af966b99 100644 --- a/web/app/components/goto-anything/hooks/__tests__/use-goto-anything-modal.spec.ts +++ b/web/app/components/goto-anything/hooks/__tests__/use-goto-anything-modal.spec.ts @@ -1,4 +1,7 @@ +import type { ReactNode } from 'react' import { act, renderHook } from '@testing-library/react' +import { createStore, Provider } from 'jotai' +import { createElement } from 'react' import { useGotoAnythingModal } from '../use-goto-anything-modal' type KeyPressEvent = { @@ -31,6 +34,14 @@ const triggerHotkey = (hotkey: string, event: KeyPressEvent) => { registration?.handler(event) } +const renderGotoAnythingModalHook = () => { + const store = createStore() + const wrapper = ({ children }: { children: ReactNode }) => + createElement(Provider, { store }, children) + + return renderHook(() => useGotoAnythingModal(), { wrapper }) +} + describe('useGotoAnythingModal', () => { beforeEach(() => { Object.keys(hotkeyHandlers).forEach(key => delete hotkeyHandlers[key]) @@ -42,92 +53,64 @@ describe('useGotoAnythingModal', () => { }) describe('initialization', () => { - it('should initialize with show=false', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - expect(result.current.show).toBe(false) + it('should initialize with open=false', () => { + const { result } = renderGotoAnythingModalHook() + expect(result.current.open).toBe(false) }) it('should provide inputRef initialized to null', () => { - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() expect(result.current.inputRef).toBeDefined() expect(result.current.inputRef.current).toBe(null) }) - it('should provide setShow function', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - expect(typeof result.current.setShow).toBe('function') - }) - - it('should provide handleClose function', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - expect(typeof result.current.handleClose).toBe('function') + it('should provide onOpenChange function', () => { + const { result } = renderGotoAnythingModalHook() + expect(typeof result.current.onOpenChange).toBe('function') }) }) describe('keyboard shortcuts', () => { - it('should toggle show state when Mod+K is triggered', () => { - const { result } = renderHook(() => useGotoAnythingModal()) + it('should toggle open state when Mod+K is triggered', () => { + const { result } = renderGotoAnythingModalHook() - expect(result.current.show).toBe(false) + expect(result.current.open).toBe(false) act(() => { triggerHotkey('Mod+K', { preventDefault: vi.fn(), target: document.body }) }) - expect(result.current.show).toBe(true) + expect(result.current.open).toBe(true) }) it('should toggle back to closed when Mod+K is triggered twice', () => { - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() act(() => { triggerHotkey('Mod+K', { preventDefault: vi.fn(), target: document.body }) }) - expect(result.current.show).toBe(true) + expect(result.current.open).toBe(true) act(() => { triggerHotkey('Mod+K', { preventDefault: vi.fn(), target: document.body }) }) - expect(result.current.show).toBe(false) + expect(result.current.open).toBe(false) }) it('should let the hotkey library ignore inputs when the modal is closed', () => { - renderHook(() => useGotoAnythingModal()) + renderGotoAnythingModalHook() expect(hotkeyHandlers['Mod+K']?.options?.ignoreInputs).toBe(true) }) - it('should close modal when escape is pressed and modal is open', () => { - const { result } = renderHook(() => useGotoAnythingModal()) + it('should not register a separate escape hotkey', () => { + renderGotoAnythingModalHook() - act(() => { - result.current.setShow(true) - }) - expect(result.current.show).toBe(true) - - act(() => { - triggerHotkey('Escape', { preventDefault: vi.fn() }) - }) - - expect(result.current.show).toBe(false) - }) - - it('should NOT do anything when escape is pressed and modal is already closed', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - - expect(result.current.show).toBe(false) - - const preventDefaultMock = vi.fn() - act(() => { - triggerHotkey('Escape', { preventDefault: preventDefaultMock }) - }) - - expect(result.current.show).toBe(false) - expect(preventDefaultMock).not.toHaveBeenCalled() + expect(hotkeyHandlers.Escape).toBeUndefined() }) it('should call preventDefault when Mod+K is triggered', () => { - renderHook(() => useGotoAnythingModal()) + renderGotoAnythingModalHook() const preventDefaultMock = vi.fn() act(() => { @@ -138,72 +121,29 @@ describe('useGotoAnythingModal', () => { }) }) - describe('handleClose', () => { - it('should close modal when handleClose is called', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - - act(() => { - result.current.setShow(true) - }) - expect(result.current.show).toBe(true) - - act(() => { - result.current.handleClose() - }) - - expect(result.current.show).toBe(false) - }) - - it('should be safe to call handleClose when modal is already closed', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - - expect(result.current.show).toBe(false) - - act(() => { - result.current.handleClose() - }) - - expect(result.current.show).toBe(false) - }) - }) - - describe('setShow', () => { + describe('onOpenChange', () => { it('should accept boolean value', () => { - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() act(() => { - result.current.setShow(true) + result.current.onOpenChange(true) }) - expect(result.current.show).toBe(true) + expect(result.current.open).toBe(true) act(() => { - result.current.setShow(false) + result.current.onOpenChange(false) }) - expect(result.current.show).toBe(false) - }) - - it('should accept function value', () => { - const { result } = renderHook(() => useGotoAnythingModal()) - - act(() => { - result.current.setShow(prev => !prev) - }) - expect(result.current.show).toBe(true) - - act(() => { - result.current.setShow(prev => !prev) - }) - expect(result.current.show).toBe(false) + expect(result.current.open).toBe(false) }) }) describe('focus management', () => { it('should call requestAnimationFrame when modal opens', () => { const rafSpy = vi.spyOn(window, 'requestAnimationFrame') - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() act(() => { - result.current.setShow(true) + result.current.onOpenChange(true) }) expect(rafSpy).toHaveBeenCalled() @@ -211,16 +151,16 @@ describe('useGotoAnythingModal', () => { }) it('should not call requestAnimationFrame when modal closes', () => { - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() act(() => { - result.current.setShow(true) + result.current.onOpenChange(true) }) const rafSpy = vi.spyOn(window, 'requestAnimationFrame') act(() => { - result.current.setShow(false) + result.current.onOpenChange(false) }) expect(rafSpy).not.toHaveBeenCalled() @@ -234,7 +174,7 @@ describe('useGotoAnythingModal', () => { return 0 } - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() const mockFocus = vi.fn() const mockInput = { focus: mockFocus } as unknown as HTMLInputElement @@ -245,7 +185,7 @@ describe('useGotoAnythingModal', () => { }) act(() => { - result.current.setShow(true) + result.current.onOpenChange(true) }) expect(mockFocus).toHaveBeenCalled() @@ -260,13 +200,13 @@ describe('useGotoAnythingModal', () => { return 0 } - const { result } = renderHook(() => useGotoAnythingModal()) + const { result } = renderGotoAnythingModalHook() act(() => { - result.current.setShow(true) + result.current.onOpenChange(true) }) - expect(result.current.show).toBe(true) + expect(result.current.open).toBe(true) window.requestAnimationFrame = originalRAF }) diff --git a/web/app/components/goto-anything/hooks/use-goto-anything-modal.ts b/web/app/components/goto-anything/hooks/use-goto-anything-modal.ts index 6a06f781e3..2da09bad98 100644 --- a/web/app/components/goto-anything/hooks/use-goto-anything-modal.ts +++ b/web/app/components/goto-anything/hooks/use-goto-anything-modal.ts @@ -2,54 +2,38 @@ import type { RefObject } from 'react' import { useHotkey } from '@tanstack/react-hotkeys' -import { useCallback, useEffect, useRef, useState } from 'react' +import { useEffect, useRef } from 'react' +import { useGotoAnythingOpen, useSetGotoAnythingOpen } from '../atoms' type UseGotoAnythingModalReturn = { - show: boolean - setShow: (show: boolean | ((prev: boolean) => boolean)) => void + open: boolean + onOpenChange: (open: boolean) => void inputRef: RefObject - handleClose: () => void } export const useGotoAnythingModal = (): UseGotoAnythingModalReturn => { - const [show, setShow] = useState(false) + const open = useGotoAnythingOpen() + const setOpen = useSetGotoAnythingOpen() const inputRef = useRef(null) - // Handle keyboard shortcuts - const handleToggleModal = useCallback((e: KeyboardEvent) => { + useHotkey('Mod+K', (e) => { e.preventDefault() - setShow(prev => !prev) - }, []) - - useHotkey('Mod+K', handleToggleModal, { - ignoreInputs: !show, - }) - - useHotkey('Escape', (e) => { - e.preventDefault() - setShow(false) + setOpen(prev => !prev) }, { - enabled: show, - ignoreInputs: false, + ignoreInputs: !open, }) - const handleClose = useCallback(() => { - setShow(false) - }, []) - - // Focus input when modal opens useEffect(() => { - if (show) { + if (open) { requestAnimationFrame(() => { inputRef.current?.focus() }) } - }, [show]) + }, [open]) return { - show, - setShow, + open, + onOpenChange: setOpen, inputRef, - handleClose, } } diff --git a/web/app/components/goto-anything/index.tsx b/web/app/components/goto-anything/index.tsx index af6f0a7a9b..a9214534b8 100644 --- a/web/app/components/goto-anything/index.tsx +++ b/web/app/components/goto-anything/index.tsx @@ -44,26 +44,25 @@ const GotoAnythingDialog: FC = ({ // Modal state management const { - show, - setShow, + open, + onOpenChange, inputRef, - handleClose: modalClose, } = useGotoAnythingModal() // Reset state when modal opens/closes useEffect(() => { - if (show && !prevShowRef.current) { + if (open && !prevShowRef.current) { // Modal just opened - reset search setSearchQuery('') } - else if (!show && prevShowRef.current) { + else if (!open && prevShowRef.current) { // Modal just closed setSearchQuery('') clearSelection() onHide?.() } - prevShowRef.current = show - }, [show, setSearchQuery, clearSelection, onHide]) + prevShowRef.current = open + }, [open, setSearchQuery, clearSelection, onHide]) // Results fetching and processing const { @@ -94,7 +93,7 @@ const GotoAnythingDialog: FC = ({ setSearchQuery, clearSelection, inputRef, - onClose: () => setShow(false), + onClose: () => onOpenChange(false), }) // Handle search input change @@ -118,12 +117,12 @@ const GotoAnythingDialog: FC = ({ if (handler?.mode === 'direct' && handler.execute && isAvailable) { e.preventDefault() handler.execute() - setShow(false) + onOpenChange(false) setSearchQuery('') } } } - }, [searchQuery, setShow, setSearchQuery]) + }, [searchQuery, onOpenChange, setSearchQuery]) // Determine which empty state to show const emptyStateVariant = useMemo(() => { @@ -144,11 +143,8 @@ const GotoAnythingDialog: FC = ({ <> { - if (!open) - modalClose() - }} + open={open} + onOpenChange={onOpenChange} >