diff --git a/packages/extension/src/content/components/App.modal-overlay.test.tsx b/packages/extension/src/content/components/App.modal-overlay.test.tsx new file mode 100644 index 0000000..7503d63 --- /dev/null +++ b/packages/extension/src/content/components/App.modal-overlay.test.tsx @@ -0,0 +1,266 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/preact'; +import userEvent from '@testing-library/user-event'; +import { App } from './App'; + +const runtimeState = vi.hoisted(() => ({ + annotateMode: false, +})); + +vi.mock('../hooks/useTabRuntimeState', async () => { + const hooks = await vi.importActual('preact/hooks'); + + return { + useTabRuntimeState: () => { + const [annotateMode, setAnnotateModeState] = hooks.useState(runtimeState.annotateMode); + + hooks.useEffect(() => { + runtimeState.annotateMode = annotateMode; + }, [annotateMode]); + + return { + enabled: true, + annotateMode, + isContextInvalid: false, + setEnabled: vi.fn(async () => {}), + setAnnotateMode: vi.fn(async (nextAnnotateMode: boolean) => { + runtimeState.annotateMode = nextAnnotateMode; + setAnnotateModeState(nextAnnotateMode); + }), + toggleEnabled: vi.fn(async () => {}), + toggleAnnotateMode: vi.fn(async () => { + setAnnotateModeState((previous) => { + const next = !previous; + runtimeState.annotateMode = next; + return next; + }); + }), + }; + }, + }; +}); + +vi.mock('../hooks/useAnnotations', () => ({ + useAnnotations: () => ({ + annotations: [], + addAnnotation: vi.fn(async () => ({ id: 'created-id' })), + addAnnotationsBulk: vi.fn(async () => []), + updateAnnotation: vi.fn(async () => true), + deleteAnnotation: vi.fn(async () => true), + clearAnnotations: vi.fn(async () => true), + refreshAnnotations: vi.fn(async () => {}), + isLoading: false, + error: null, + isContextInvalid: false, + }), +})); + +vi.mock('../messaging', () => ({ + getSettings: vi.fn(async () => ({ success: true, data: { clearOnCopy: false } })), + updateSettings: vi.fn(async () => ({ success: true, data: { clearOnCopy: false } })), +})); + +vi.mock('./ElementHighlight', () => ({ + ElementHighlight: () => null, +})); + +vi.mock('./AnnotationMarkers', () => ({ + AnnotationMarkers: () => null, +})); + +vi.mock('./RegionOutline', () => ({ + RegionOutline: () => null, +})); + +vi.mock('./RegionTransformHandles', () => ({ + RegionTransformHandles: () => null, +})); + +vi.mock('./OnUIDialog', () => ({ + OnUIDialog: ({ onCancel }: { onCancel: () => void }) => ( +
+
Annotation dialog mock
+ +
+ ), +})); + +vi.mock('./OnUIRegionDialog', () => ({ + OnUIRegionDialog: () => null, +})); + +vi.mock('./RegionDrawOverlay', () => ({ + RegionDrawOverlay: () => null, +})); + +vi.mock('./ErrorToast', () => ({ + ErrorToast: () => null, +})); + +function installPointerEventShim() { + if (typeof window.PointerEvent === 'function') { + return; + } + + class PointerEventShim extends MouseEvent {} + + Object.defineProperty(window, 'PointerEvent', { + configurable: true, + writable: true, + value: PointerEventShim, + }); +} + +function installElementsFromPointMock() { + const elementsFromPoint = vi.fn(() => [] as Element[]); + Object.defineProperty(document, 'elementsFromPoint', { + configurable: true, + writable: true, + value: elementsFromPoint, + }); + return elementsFromPoint; +} + +function installHostDismissListener(type: 'pointerdown' | 'keydown') { + const hostDismiss = vi.fn(); + document.addEventListener(type, hostDismiss); + + return { + hostDismiss, + cleanup: () => document.removeEventListener(type, hostDismiss), + }; +} + +function renderInOnUiHost() { + document.body.innerHTML = ''; + + const host = document.createElement('div'); + host.id = 'onui-shadow-host'; + + const appContainer = document.createElement('div'); + host.appendChild(appContainer); + document.body.appendChild(host); + + const modal = document.createElement('div'); + modal.setAttribute('role', 'dialog'); + modal.setAttribute('aria-label', 'Host dialog'); + document.body.appendChild(modal); + + render(, { container: appContainer }); + + return { host, modal }; +} + +async function flushEffects() { + await Promise.resolve(); + await Promise.resolve(); +} + +function dispatchPointerDown(target: Element, coords = { clientX: 24, clientY: 24 }) { + target.dispatchEvent( + new window.PointerEvent('pointerdown', { + bubbles: true, + cancelable: true, + button: 0, + ...coords, + }) + ); +} + +describe('App modal-safe overlay behavior', () => { + beforeAll(() => { + installPointerEventShim(); + }); + + beforeEach(() => { + runtimeState.annotateMode = false; + vi.clearAllMocks(); + installElementsFromPointMock(); + }); + + it('does not trigger host pointer dismissal when toggling annotate mode from the onUI toolbar', async () => { + const { modal } = renderInOnUiHost(); + const user = userEvent.setup(); + const { hostDismiss, cleanup } = installHostDismissListener('pointerdown'); + + await user.click(screen.getByRole('button', { name: 'Toggle onUI panel' })); + await user.click(screen.getByRole('button', { name: 'Toggle annotate mode' })); + + const annotateButton = screen.getByRole('button', { name: 'Toggle annotate mode' }); + await waitFor(() => { + expect(annotateButton.className).toContain('is-active'); + }); + + expect(hostDismiss).not.toHaveBeenCalled(); + expect(modal.getAttribute('role')).toBe('dialog'); + cleanup(); + }); + + it('selects modal content in annotate mode without invoking the host pointer dismissal handler', async () => { + runtimeState.annotateMode = true; + const elementsFromPoint = installElementsFromPointMock(); + const { modal } = renderInOnUiHost(); + const { hostDismiss, cleanup } = installHostDismissListener('pointerdown'); + + const target = document.createElement('button'); + target.textContent = 'Dialog action'; + Object.defineProperty(target, 'getBoundingClientRect', { + configurable: true, + value: () => ({ + top: 20, + left: 20, + bottom: 60, + right: 180, + width: 160, + height: 40, + x: 20, + y: 20, + toJSON: () => ({}), + }), + }); + modal.appendChild(target); + elementsFromPoint.mockReturnValue([target]); + await flushEffects(); + + dispatchPointerDown(target); + + expect(hostDismiss).not.toHaveBeenCalled(); + expect(await screen.findByText('Annotation dialog mock')).toBeTruthy(); + cleanup(); + }); + + it('owns Escape while annotate mode is active and keeps the host dialog dismissal handler from firing', async () => { + runtimeState.annotateMode = true; + renderInOnUiHost(); + const user = userEvent.setup(); + const { hostDismiss, cleanup } = installHostDismissListener('keydown'); + + await user.click(screen.getByRole('button', { name: 'Toggle onUI panel' })); + const annotateButton = screen.getByRole('button', { name: 'Toggle annotate mode' }); + expect(annotateButton.className).toContain('is-active'); + + await user.keyboard('{Escape}'); + + await waitFor(() => { + expect(annotateButton.className).not.toContain('is-active'); + }); + expect(hostDismiss).not.toHaveBeenCalled(); + cleanup(); + }); + + it('still allows host pointer dismissal when annotate mode is inactive and the click is outside onUI', () => { + renderInOnUiHost(); + const { hostDismiss, cleanup } = installHostDismissListener('pointerdown'); + + const outsideButton = document.createElement('button'); + outsideButton.textContent = 'Outside target'; + document.body.appendChild(outsideButton); + + dispatchPointerDown(outsideButton); + + expect(hostDismiss).toHaveBeenCalledTimes(1); + cleanup(); + }); +}); diff --git a/packages/extension/src/content/components/App.tsx b/packages/extension/src/content/components/App.tsx index 99def6c..6937bd7 100644 --- a/packages/extension/src/content/components/App.tsx +++ b/packages/extension/src/content/components/App.tsx @@ -18,6 +18,7 @@ import { useAnnotations } from '../hooks/useAnnotations'; import { useTabRuntimeState } from '../hooks/useTabRuntimeState'; import { createAnnotationFromElement } from '../utils/create-annotation'; import { createAnnotationFromRegion } from '../utils/create-region-annotation'; +import { ONUI_SHADOW_HOST_ID, stopEventPropagation } from '../utils/overlay-events'; import { OnUIRegionDialog } from './OnUIRegionDialog'; import { RegionDrawOverlay } from './RegionDrawOverlay'; import { RegionOutline } from './RegionOutline'; @@ -33,6 +34,8 @@ function createBatchId(): string { return `batch-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; } +type SelectionEvent = MouseEvent | PointerEvent; + interface SaveDialogData { comment: string; intent?: AnnotationIntent | undefined; @@ -163,7 +166,7 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { }, [isDrawMode]); // Handle element click in annotation mode - const handleElementClick = useCallback((element: Element, event: MouseEvent) => { + const handleElementClick = useCallback((element: Element, event: SelectionEvent) => { setEditingAnnotation(null); setPendingRegion(null); @@ -192,6 +195,27 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { setSelectedElement(element); }, []); + useEffect(() => { + const host = document.getElementById(ONUI_SHADOW_HOST_ID); + if (!host) { + return; + } + + const stopOverlayPointerEvent = (event: Event) => { + stopEventPropagation(event); + }; + + host.addEventListener('pointerdown', stopOverlayPointerEvent); + host.addEventListener('mousedown', stopOverlayPointerEvent); + host.addEventListener('click', stopOverlayPointerEvent); + + return () => { + host.removeEventListener('pointerdown', stopOverlayPointerEvent); + host.removeEventListener('mousedown', stopOverlayPointerEvent); + host.removeEventListener('click', stopOverlayPointerEvent); + }; + }, []); + // Open multi-target dialog when shift is released useEffect(() => { const handleShiftRelease = (event: KeyboardEvent) => { @@ -221,30 +245,6 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { multiDialogTargets.length, ]); - // Esc while selecting clears pending multi targets before toolbar exits annotate mode - useEffect(() => { - const handleEscapeClearSelection = (event: KeyboardEvent) => { - if (event.key !== 'Escape') return; - if (!annotateMode) return; - if (pendingMultiSelection.length === 0) return; - if (selectedElement || editingAnnotation || multiDialogTargets.length > 0) return; - - event.preventDefault(); - event.stopPropagation(); - event.stopImmediatePropagation(); - setPendingMultiSelection([]); - }; - - document.addEventListener('keydown', handleEscapeClearSelection, true); - return () => document.removeEventListener('keydown', handleEscapeClearSelection, true); - }, [ - annotateMode, - pendingMultiSelection.length, - selectedElement, - editingAnnotation, - multiDialogTargets.length, - ]); - const { hoveredElement } = useElementHover({ enabled: annotateMode && @@ -496,8 +496,8 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { return; } - if (pendingRegion) { - setPendingRegion(null); + if (pendingRegion || selectedElement || editingAnnotation || multiDialogTargets.length > 0) { + handleCancelPopup(); return; } @@ -507,11 +507,6 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { return; } - if (editingAnnotation && editingRegionGeometry) { - setEditingRegionGeometry(null); - return; - } - if (pendingMultiSelection.length > 0) { setPendingMultiSelection([]); return; @@ -523,14 +518,44 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { }, [ annotateMode, editingAnnotation, - editingRegionGeometry, + handleCancelPopup, isDrawMode, isDrawingDraft, + multiDialogTargets.length, onToggleAnnotateMode, pendingMultiSelection.length, pendingRegion, + selectedElement, ]); + const shouldOwnEscape = + annotateMode || + isDrawMode || + isDrawingDraft || + pendingRegion !== null || + selectedElement !== null || + editingAnnotation !== null || + multiDialogTargets.length > 0 || + pendingMultiSelection.length > 0; + + useEffect(() => { + if (!shouldOwnEscape) { + return; + } + + const handleEscapeKeyDown = (event: KeyboardEvent) => { + if (event.key !== 'Escape') { + return; + } + + stopEventPropagation(event, true); + handleEscape(); + }; + + window.addEventListener('keydown', handleEscapeKeyDown, true); + return () => window.removeEventListener('keydown', handleEscapeKeyDown, true); + }, [handleEscape, shouldOwnEscape]); + // Handle clear all annotations const handleClearAnnotations = useCallback(async () => { await clearAnnotations(); @@ -595,7 +620,6 @@ function EnabledApp({ annotateMode, onToggleAnnotateMode }: EnabledAppProps) { onToggleAnnotateMode={handleToggleAnnotateExclusive} onToggleDrawMode={handleToggleDrawMode} onSelectDrawShape={setDrawShape} - onEscape={handleEscape} annotations={annotations} outputLevel={outputLevel} onOutputLevelChange={setOutputLevel} diff --git a/packages/extension/src/content/components/OnUIAnnotationForm.tsx b/packages/extension/src/content/components/OnUIAnnotationForm.tsx index fb552a0..e2121d5 100644 --- a/packages/extension/src/content/components/OnUIAnnotationForm.tsx +++ b/packages/extension/src/content/components/OnUIAnnotationForm.tsx @@ -81,56 +81,56 @@ const SEVERITY_OPTIONS: Option[] = [ interface OnUIAnnotationDialogProps { title: string; - subtitle?: string; - subtitleTitle?: string; + subtitle?: string | undefined; + subtitleTitle?: string | undefined; onCancel: () => void; - position?: JSX.CSSProperties; - className?: string; - width?: string; - showBackdrop?: boolean; - dialogRef?: Ref; - children: ComponentChildren; + position?: JSX.CSSProperties | undefined; + className?: string | undefined; + width?: string | undefined; + showBackdrop?: boolean | undefined; + dialogRef?: Ref | undefined; + children?: ComponentChildren; } type OnUIAnnotationDialogShellProps = OnUIAnnotationDialogProps; interface OnUIAnnotationFormProps { comment: string; - intent?: AnnotationIntent; - severity?: AnnotationSeverity; + intent?: AnnotationIntent | undefined; + severity?: AnnotationSeverity | undefined; onCommentChange: (value: string) => void; onIntentChange: (value: AnnotationIntent | undefined) => void; onSeverityChange: (value: AnnotationSeverity | undefined) => void; onSave: () => void; onCancel: () => void; - onDelete?: () => void; - commentLabel?: string; - commentPlaceholder?: string; - intentLabel?: string; - severityLabel?: string; - cancelLabel?: string; - saveLabel?: string; - deleteLabel?: string; - saveDisabled?: boolean; - isSaving?: boolean; - textareaRef?: Ref; + onDelete?: (() => void) | undefined; + commentLabel?: string | undefined; + commentPlaceholder?: string | undefined; + intentLabel?: string | undefined; + severityLabel?: string | undefined; + cancelLabel?: string | undefined; + saveLabel?: string | undefined; + deleteLabel?: string | undefined; + saveDisabled?: boolean | undefined; + isSaving?: boolean | undefined; + textareaRef?: Ref | undefined; children?: ComponentChildren; } interface OnUIAnnotationFormDialogProps extends OnUIAnnotationDialogProps { comment: string; - intent?: AnnotationIntent; - severity?: AnnotationSeverity; + intent?: AnnotationIntent | undefined; + severity?: AnnotationSeverity | undefined; onCommentChange: (value: string) => void; onIntentChange: (value: AnnotationIntent | undefined) => void; onSeverityChange: (value: AnnotationSeverity | undefined) => void; onSave: () => void; - onDelete?: () => void; - commentPlaceholder?: string; + onDelete?: (() => void) | undefined; + commentPlaceholder?: string | undefined; saveLabel: string; - saveDisabled?: boolean; - isSaving?: boolean; - textareaRef?: Ref; + saveDisabled?: boolean | undefined; + isSaving?: boolean | undefined; + textareaRef?: Ref | undefined; children?: ComponentChildren; } @@ -169,7 +169,7 @@ export function OnUIAnnotationDialogShell({ return ( <> {showBackdrop &&
} -
+
{title}
@@ -256,7 +256,7 @@ export function OnUIAnnotationForm({ {commentLabel}