diff --git a/src/common/BaseAnnotator.scss b/src/common/BaseAnnotator.scss index 43d86de9f..c6258c09d 100644 --- a/src/common/BaseAnnotator.scss +++ b/src/common/BaseAnnotator.scss @@ -28,3 +28,14 @@ .ba-Layer--popup { z-index: 4; } + +.ba-PopupPortal { + position: relative; + z-index: 10; + pointer-events: none; + + .ba-Popup { + z-index: 150; + pointer-events: auto; + } +} diff --git a/src/common/DeselectListener.tsx b/src/common/DeselectListener.tsx index ff3604d65..a28353372 100644 --- a/src/common/DeselectListener.tsx +++ b/src/common/DeselectListener.tsx @@ -8,7 +8,7 @@ export default function DeselectListener(): null { React.useEffect(() => { const handleMouseDown = (event: MouseEvent): void => { // Popup is portaled outside .ba-Layer; native mousedown still bubbles here. - if ((event.target as Element | null)?.closest?.('.ba-PopupV2')) return; + if ((event.target as Element | null)?.closest?.('.ba-PopupV2, .ba-Popup')) return; dispatch(setActiveAnnotationIdAction(null)); }; diff --git a/src/components/Popups/PopupDrawingToolbar.tsx b/src/components/Popups/PopupDrawingToolbar.tsx index 5e4341659..177f94131 100644 --- a/src/components/Popups/PopupDrawingToolbar.tsx +++ b/src/components/Popups/PopupDrawingToolbar.tsx @@ -33,12 +33,6 @@ export type Props = { const options: Partial = { modifiers: [ - { - name: 'eventListeners', - options: { - scroll: false, - }, - }, { name: 'offset', options: { @@ -48,7 +42,6 @@ const options: Partial = { { name: 'preventOverflow', options: { - altAxis: true, padding: 50, }, }, diff --git a/src/document/DocumentAnnotator.ts b/src/document/DocumentAnnotator.ts index 5402a2259..cd01b7fa1 100644 --- a/src/document/DocumentAnnotator.ts +++ b/src/document/DocumentAnnotator.ts @@ -103,7 +103,7 @@ export default class DocumentAnnotator extends BaseAnnotator { // Annotations mode managers.add(new PopupManager({ location: pageNumber, popupPortalEl: this.popupPortalEl, referenceEl: pageReferenceEl, resinTags })); - managers.add(new DrawingManager({ location: pageNumber, referenceEl: pageReferenceEl, resinTags })); + managers.add(new DrawingManager({ location: pageNumber, popupPortalEl: this.popupPortalEl, referenceEl: pageReferenceEl, resinTags })); const textLayer = pageEl.querySelector('.textLayer') as HTMLElement; diff --git a/src/drawing/DrawingAnnotations.tsx b/src/drawing/DrawingAnnotations.tsx index c5b1f12cc..c5b988fe8 100644 --- a/src/drawing/DrawingAnnotations.tsx +++ b/src/drawing/DrawingAnnotations.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import ReactDOM from 'react-dom'; import classNames from 'classnames'; import DecoratedDrawingPath from './DecoratedDrawingPath'; import DrawingList from './DrawingList'; @@ -22,6 +23,7 @@ export type Props = { drawnPathGroups: Array; isCreating: boolean; location: number; + popupPortalEl?: HTMLElement | null; redoDrawingPathGroup: () => void; resetDrawing: () => void; rotation: number; @@ -46,6 +48,7 @@ const DrawingAnnotations = (props: Props): JSX.Element => { drawnPathGroups, isCreating, location, + popupPortalEl, redoDrawingPathGroup, resetDrawing, rotation, @@ -176,20 +179,22 @@ const DrawingAnnotations = (props: Props): JSX.Element => { /> )} - {isCreating && hasPathGroups && drawingSVGGroup && canShowPopupToolbar && ( - - )} + {isCreating && hasPathGroups && drawingSVGGroup && canShowPopupToolbar && popupPortalEl && + ReactDOM.createPortal( + , + popupPortalEl, + )} ); }; diff --git a/src/drawing/DrawingManager.tsx b/src/drawing/DrawingManager.tsx index 7eceb31fe..2e6e1de31 100644 --- a/src/drawing/DrawingManager.tsx +++ b/src/drawing/DrawingManager.tsx @@ -1,9 +1,20 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom/client'; -import BaseManager, { Props } from '../common/BaseManager'; +import BaseManager, { Options as BaseOptions, Props } from '../common/BaseManager'; import DrawingAnnotationsContainer from './DrawingAnnotationsContainer'; +export type Options = BaseOptions & { + popupPortalEl?: HTMLElement | null; +}; + export default class DrawingListManager extends BaseManager { + popupPortalEl?: HTMLElement | null; + + constructor({ popupPortalEl, ...options }: Options) { + super(options); + this.popupPortalEl = popupPortalEl; + } + decorate(): void { this.reactEl.classList.add('ba-Layer--drawing'); this.reactEl.dataset.testid = 'ba-Layer--drawing'; @@ -14,6 +25,6 @@ export default class DrawingListManager extends BaseManager { this.root = ReactDOM.createRoot(this.reactEl); } - this.root.render(); + this.root.render(); } } diff --git a/src/drawing/__tests__/DrawingAnnotations-test.tsx b/src/drawing/__tests__/DrawingAnnotations-test.tsx index 9146f0673..5a71f5349 100644 --- a/src/drawing/__tests__/DrawingAnnotations-test.tsx +++ b/src/drawing/__tests__/DrawingAnnotations-test.tsx @@ -38,6 +38,7 @@ describe('DrawingAnnotations', () => { drawnPathGroups: [], isCreating: false, location: 0, + popupPortalEl: document.createElement('div'), redoDrawingPathGroup: jest.fn(), resetDrawing: jest.fn(), rotation: 0, @@ -319,6 +320,35 @@ describe('DrawingAnnotations', () => { ); }); + describe('popup portal', () => { + test('should render PopupDrawingToolbar into popupPortalEl', () => { + const popupPortalEl = document.createElement('div'); + document.body.appendChild(popupPortalEl); + + getWrapper({ + canShowPopupToolbar: true, + drawnPathGroups: pathGroups, + isCreating: true, + popupPortalEl, + }); + + expect(popupPortalEl.querySelector('.ba-PopupDrawingToolbar')).toBeTruthy(); + + document.body.removeChild(popupPortalEl); + }); + + test('should not render PopupDrawingToolbar when popupPortalEl is missing', () => { + const wrapper = getWrapper({ + canShowPopupToolbar: true, + drawnPathGroups: pathGroups, + isCreating: true, + popupPortalEl: null, + }); + + expect(wrapper.find(PopupDrawingToolbar).exists()).toBe(false); + }); + }); + // Use shared video annotation tests createVideoAnnotationTests({ componentName: 'DrawingAnnotations', diff --git a/src/image/ImageAnnotator.ts b/src/image/ImageAnnotator.ts index 12d785655..1bd6c2291 100644 --- a/src/image/ImageAnnotator.ts +++ b/src/image/ImageAnnotator.ts @@ -74,7 +74,7 @@ export default class ImageAnnotator extends BaseAnnotator { } this.managers.add(new PopupManager({ popupPortalEl: this.popupPortalEl, referenceEl, resinTags })); - this.managers.add(new DrawingManager({ referenceEl, resinTags })); + this.managers.add(new DrawingManager({ popupPortalEl: this.popupPortalEl, referenceEl, resinTags })); this.managers.add(new RegionManager({ referenceEl, resinTags })); this.managers.add(new RegionCreationManager({ referenceEl, resinTags })); } diff --git a/src/popup/PopupLayer.tsx b/src/popup/PopupLayer.tsx index 104c29715..39465997f 100644 --- a/src/popup/PopupLayer.tsx +++ b/src/popup/PopupLayer.tsx @@ -1,4 +1,5 @@ import * as React from 'react'; +import * as ReactDOM from 'react-dom'; import noop from 'lodash/noop'; import PopupReply from '../components/Popups/PopupReply'; import PopupV2 from '../components/Popups/PopupV2'; @@ -121,7 +122,8 @@ const PopupLayer = (props: Props): JSX.Element | null => { } if (showCreator) { - return ( + if (!popupPortalEl) return null; + return ReactDOM.createPortal(
{ reference={reference} value={message} /> -
+ , + popupPortalEl, ); } diff --git a/src/popup/__tests__/PopupLayer-test.tsx b/src/popup/__tests__/PopupLayer-test.tsx index 10933dfc3..0aa39a7d1 100644 --- a/src/popup/__tests__/PopupLayer-test.tsx +++ b/src/popup/__tests__/PopupLayer-test.tsx @@ -56,6 +56,7 @@ describe('PopupLayer', () => { }); const referenceId = '123'; + const popupPortalEl = document.createElement('div'); const getDefaults = (): Props => ({ activeAnnotationId: null, createDrawing: jest.fn(), @@ -65,6 +66,7 @@ describe('PopupLayer', () => { location: 1, message: '', mode: Mode.HIGHLIGHT, + popupPortalEl, referenceId: '123', resetCreator: jest.fn(), setMessage: jest.fn(), @@ -75,6 +77,7 @@ describe('PopupLayer', () => { beforeEach(() => { document.body.innerHTML = `
`; + document.body.appendChild(popupPortalEl); mockOnCancel = undefined; mockOnChange = undefined; mockOnSubmit = undefined; @@ -151,6 +154,20 @@ describe('PopupLayer', () => { }); }); + describe('popup portal', () => { + test('should render PopupReply into popupPortalEl', () => { + renderLayer(); + + expect(popupPortalEl.querySelector('[data-testid="popup-reply"]')).toBeTruthy(); + }); + + test('should render nothing when popupPortalEl is missing', () => { + renderLayer({ popupPortalEl: null }); + + expect(screen.queryByTestId('popup-reply')).toBeNull(); + }); + }); + describe('event handlers', () => { describe('handleCancel()', () => { test('should reset creator and reset isPromoting', () => {