diff --git a/packages/react/src/hooks/__tests__/useSearchHotkey.test.ts b/packages/react/src/hooks/__tests__/useSearchHotkey.test.ts index 22adb57..07bd538 100644 --- a/packages/react/src/hooks/__tests__/useSearchHotkey.test.ts +++ b/packages/react/src/hooks/__tests__/useSearchHotkey.test.ts @@ -22,6 +22,10 @@ function appendTo(parent: HTMLElement, tag: string): HTMLElement { return el } +function click(target: EventTarget): void { + target.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) +} + function pressCmdF(target: EventTarget): boolean { const e = new KeyboardEvent('keydown', { key: 'f', @@ -50,41 +54,38 @@ describe('shouldHandleSearchHotkey', () => { it('handles the key when the target is inside the canvas', () => { const node = appendTo(container, 'div') - expect(shouldHandleSearchHotkey(node, container)).toBe(true) + expect(shouldHandleSearchHotkey(node, container, false)).toBe(true) }) - it('handles the key when nothing is focused', () => { - expect(shouldHandleSearchHotkey(document.body, container)).toBe(true) + it('defers to canvasActive when nothing is focused', () => { + // Most host-app chrome is not focusable, so a click on it leaves the + // keystroke on — indistinguishable from "never clicked anything". + expect(shouldHandleSearchHotkey(document.body, container, true)).toBe(true) + expect(shouldHandleSearchHotkey(document.body, container, false)).toBe(false) }) it('ignores the key while typing in a textarea outside the canvas', () => { - // The reported bug: a chat panel rendered next to the canvas. const chat = appendTo(document.body, 'div') const textarea = appendTo(chat, 'textarea') - expect(shouldHandleSearchHotkey(textarea, container)).toBe(false) + expect(shouldHandleSearchHotkey(textarea, container, true)).toBe(false) }) it('ignores the key while typing in an input, even inside the canvas', () => { - // The canvas' own search overlay is an input — Cmd+F there is the - // browser's to handle, not another toggle. + // The canvas' own search overlay is an input — Cmd+F there belongs to the + // browser, not to another toggle. const input = appendTo(container, 'input') - expect(shouldHandleSearchHotkey(input, container)).toBe(false) + expect(shouldHandleSearchHotkey(input, container, true)).toBe(false) }) it('ignores the key in a contentEditable region', () => { const editable = appendTo(document.body, 'div') Object.defineProperty(editable, 'isContentEditable', { value: true }) - expect(shouldHandleSearchHotkey(editable, container)).toBe(false) + expect(shouldHandleSearchHotkey(editable, container, true)).toBe(false) }) - it('ignores the key when focus is elsewhere in the host app', () => { + it('ignores the key when a focusable element elsewhere has focus', () => { const sidebarButton = appendTo(appendTo(document.body, 'aside'), 'button') - expect(shouldHandleSearchHotkey(sidebarButton, container)).toBe(false) - }) - - it('falls back to handling the key when there is no container yet', () => { - const anywhere = appendTo(document.body, 'div') - expect(shouldHandleSearchHotkey(anywhere, null)).toBe(true) + expect(shouldHandleSearchHotkey(sidebarButton, container, false)).toBe(false) }) }) @@ -94,43 +95,86 @@ describe('shouldHandleSearchHotkey', () => { describe('useSearchHotkey', () => { let container: HTMLElement + let chatPanel: HTMLElement beforeEach(() => { container = makeCanvas() + chatPanel = appendTo(document.body, 'div') }) - it('toggles and preventDefaults for a keystroke inside the canvas', () => { + function mount(onToggle: () => void) { + return renderHook(() => useSearchHotkey({ current: container }, onToggle)) + } + + it('does nothing before the canvas has been clicked', () => { const onToggle = vi.fn() - renderHook(() => useSearchHotkey({ current: container }, onToggle)) + mount(onToggle) - const node = appendTo(container, 'div') - const prevented = pressCmdF(node) + const prevented = pressCmdF(document.body) + + expect(onToggle).not.toHaveBeenCalled() + // The host app keeps native find. + expect(prevented).toBe(false) + }) + + it('toggles after the canvas is clicked', () => { + const onToggle = vi.fn() + mount(onToggle) + + click(appendTo(container, 'div')) + const prevented = pressCmdF(document.body) expect(onToggle).toHaveBeenCalledTimes(1) expect(prevented).toBe(true) }) - it('does not toggle or preventDefault from a chat textarea', () => { + it('stops toggling once a non-focusable part of the host app is clicked', () => { + // The reported case: clicking anywhere in the chat panel — not just its + // textarea — must hand Cmd+F back to the browser. + const onToggle = vi.fn() + mount(onToggle) + + click(appendTo(container, 'div')) + click(appendTo(chatPanel, 'div')) + const prevented = pressCmdF(document.body) + + expect(onToggle).not.toHaveBeenCalled() + expect(prevented).toBe(false) + }) + + it('does not toggle from a chat textarea even when the canvas was clicked last', () => { const onToggle = vi.fn() - renderHook(() => useSearchHotkey({ current: container }, onToggle)) + mount(onToggle) - const textarea = appendTo(appendTo(document.body, 'div'), 'textarea') + click(appendTo(container, 'div')) + const textarea = appendTo(chatPanel, 'textarea') const prevented = pressCmdF(textarea) expect(onToggle).not.toHaveBeenCalled() - // Native find must stay available to the host app. expect(prevented).toBe(false) }) - it('ignores other keys and unmodified f', () => { + it('toggles when focus is inside the canvas regardless of click history', () => { const onToggle = vi.fn() - renderHook(() => useSearchHotkey({ current: container }, onToggle)) + mount(onToggle) + click(appendTo(chatPanel, 'div')) const node = appendTo(container, 'div') - node.dispatchEvent( + const prevented = pressCmdF(node) + + expect(onToggle).toHaveBeenCalledTimes(1) + expect(prevented).toBe(true) + }) + + it('ignores other keys and unmodified f', () => { + const onToggle = vi.fn() + mount(onToggle) + click(appendTo(container, 'div')) + + document.body.dispatchEvent( new KeyboardEvent('keydown', { key: 'f', bubbles: true, cancelable: true }), ) - node.dispatchEvent( + document.body.dispatchEvent( new KeyboardEvent('keydown', { key: 'g', metaKey: true, @@ -144,12 +188,11 @@ describe('useSearchHotkey', () => { it('stops listening after unmount', () => { const onToggle = vi.fn() - const { unmount } = renderHook(() => - useSearchHotkey({ current: container }, onToggle), - ) + const { unmount } = mount(onToggle) + click(appendTo(container, 'div')) unmount() - pressCmdF(appendTo(container, 'div')) + pressCmdF(document.body) expect(onToggle).not.toHaveBeenCalled() }) @@ -162,7 +205,8 @@ describe('useSearchHotkey', () => { ) rerender({ cb: second }) - pressCmdF(appendTo(container, 'div')) + click(appendTo(container, 'div')) + pressCmdF(document.body) expect(first).not.toHaveBeenCalled() expect(second).toHaveBeenCalledTimes(1) diff --git a/packages/react/src/hooks/useSearchHotkey.ts b/packages/react/src/hooks/useSearchHotkey.ts index 6e15e13..bb9e3d9 100644 --- a/packages/react/src/hooks/useSearchHotkey.ts +++ b/packages/react/src/hooks/useSearchHotkey.ts @@ -9,12 +9,17 @@ import { useEffect, useRef, type RefObject } from 'react' * — a chat panel, a sidebar — and hijacking Cmd+F there both opens the wrong * search and suppresses the browser's native find. * - * Handle the key when the canvas owns focus, or when nothing does; leave it - * alone when focus sits in a text field or elsewhere in the host app. + * Focus alone isn't enough to tell the two apart: most of a host app's chrome + * isn't focusable, so clicking it leaves the keystroke on ``, exactly as + * if the user had never clicked anything. Hence `canvasActive`, which the hook + * derives from where the last pointerdown landed. + * + * @param canvasActive whether the canvas was the last region the user clicked */ export function shouldHandleSearchHotkey( target: EventTarget | null, container: HTMLElement | null, + canvasActive: boolean, ): boolean { const el = target instanceof HTMLElement ? target : null @@ -26,37 +31,57 @@ export function shouldHandleSearchHotkey( return false } - // No container to compare against, or a target we can't place: assume the - // canvas is the whole page (demo, standalone build) and keep the old behaviour. - if (!container || !el) return true - - // Keydowns land on when nothing is focused. - if (el === document.body || el === document.documentElement) return true + // Focus sits inside the canvas — unambiguous, regardless of click history. + if (container && el && container.contains(el)) return true - return container.contains(el) + return canvasActive } /** * Cmd+F / Ctrl+F toggles the canvas search overlay, in both editable and - * read-only modes. + * read-only modes, when the canvas is the region the user is working in. + * + * Starts inactive: an embedded canvas never claims Cmd+F until it has been + * clicked, so the host app keeps the browser's native find until then. */ export function useSearchHotkey( containerRef: RefObject, onToggle: () => void, ): void { // Kept in a ref so a caller passing an inline closure doesn't re-subscribe - // the window listener on every render. + // the window listeners on every render. const toggleRef = useRef(onToggle) toggleRef.current = onToggle + const canvasActiveRef = useRef(false) + useEffect(() => { + const onPointerDown = (e: PointerEvent) => { + const container = containerRef.current + const el = e.target instanceof HTMLElement ? e.target : null + canvasActiveRef.current = !!container && !!el && container.contains(el) + } + const onKey = (e: KeyboardEvent) => { if (!((e.metaKey || e.ctrlKey) && e.key === 'f')) return - if (!shouldHandleSearchHotkey(e.target, containerRef.current)) return + if ( + !shouldHandleSearchHotkey( + e.target, + containerRef.current, + canvasActiveRef.current, + ) + ) { + return + } e.preventDefault() toggleRef.current() } + + window.addEventListener('pointerdown', onPointerDown, true) window.addEventListener('keydown', onKey) - return () => window.removeEventListener('keydown', onKey) + return () => { + window.removeEventListener('pointerdown', onPointerDown, true) + window.removeEventListener('keydown', onKey) + } }, [containerRef]) }