Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/common/BaseAnnotator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
jackiejou marked this conversation as resolved.
pointer-events: auto;
}
}
2 changes: 1 addition & 1 deletion src/common/DeselectListener.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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));
};

Expand Down
7 changes: 0 additions & 7 deletions src/components/Popups/PopupDrawingToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ export type Props = {

const options: Partial<Options> = {
modifiers: [
{
name: 'eventListeners',
options: {
scroll: false,
},
},
{
name: 'offset',
options: {
Expand All @@ -48,7 +42,6 @@ const options: Partial<Options> = {
{
name: 'preventOverflow',
options: {
altAxis: true,
padding: 50,
},
},
Expand Down
2 changes: 1 addition & 1 deletion src/document/DocumentAnnotator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
33 changes: 19 additions & 14 deletions src/drawing/DrawingAnnotations.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -22,6 +23,7 @@ export type Props = {
drawnPathGroups: Array<PathGroup>;
isCreating: boolean;
location: number;
popupPortalEl?: HTMLElement | null;
redoDrawingPathGroup: () => void;
resetDrawing: () => void;
rotation: number;
Expand All @@ -46,6 +48,7 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
drawnPathGroups,
isCreating,
location,
popupPortalEl,
redoDrawingPathGroup,
resetDrawing,
rotation,
Expand Down Expand Up @@ -176,20 +179,22 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
/>
)}

{isCreating && hasPathGroups && drawingSVGGroup && canShowPopupToolbar && (
<PopupDrawingToolbar
ref={popupDrawingToolbarRef}
canComment={hasDrawnPathGroups}
canRedo={hasStashedPathGroups}
canUndo={hasDrawnPathGroups}
className={classNames('ba-DrawingAnnotations-toolbar', { 'ba-is-drawing': isDrawing })}
onDelete={handleDelete}
onRedo={handleRedo}
onReply={handleReply}
onUndo={handleUndo}
reference={drawingSVGGroup}
/>
)}
{isCreating && hasPathGroups && drawingSVGGroup && canShowPopupToolbar && popupPortalEl &&
ReactDOM.createPortal(
<PopupDrawingToolbar
ref={popupDrawingToolbarRef}
canComment={hasDrawnPathGroups}
canRedo={hasStashedPathGroups}
canUndo={hasDrawnPathGroups}
className={classNames('ba-DrawingAnnotations-toolbar', { 'ba-is-drawing': isDrawing })}
onDelete={handleDelete}
onRedo={handleRedo}
onReply={handleReply}
onUndo={handleUndo}
reference={drawingSVGGroup}
/>,
popupPortalEl,
)}
</>
);
};
Expand Down
15 changes: 13 additions & 2 deletions src/drawing/DrawingManager.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -14,6 +25,6 @@ export default class DrawingListManager extends BaseManager {
this.root = ReactDOM.createRoot(this.reactEl);
}

this.root.render(<DrawingAnnotationsContainer referenceEl={this.referenceEl} targetType={this.targetType} location={this.location} {...props} />);
this.root.render(<DrawingAnnotationsContainer referenceEl={this.referenceEl} targetType={this.targetType} location={this.location} popupPortalEl={this.popupPortalEl} {...props} />);
}
}
30 changes: 30 additions & 0 deletions src/drawing/__tests__/DrawingAnnotations-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('DrawingAnnotations', () => {
drawnPathGroups: [],
isCreating: false,
location: 0,
popupPortalEl: document.createElement('div'),
redoDrawingPathGroup: jest.fn(),
resetDrawing: jest.fn(),
rotation: 0,
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion src/image/ImageAnnotator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
}
Expand Down
7 changes: 5 additions & 2 deletions src/popup/PopupLayer.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -121,7 +122,8 @@ const PopupLayer = (props: Props): JSX.Element | null => {
}

if (showCreator) {
return (
if (!popupPortalEl) return null;
return ReactDOM.createPortal(
<div className="ba-PopupLayer-popup">
<PopupReply
isPending={isPending}
Expand All @@ -131,7 +133,8 @@ const PopupLayer = (props: Props): JSX.Element | null => {
reference={reference}
value={message}
/>
</div>
</div>,
popupPortalEl,
);
}

Expand Down
17 changes: 17 additions & 0 deletions src/popup/__tests__/PopupLayer-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('PopupLayer', () => {
});

const referenceId = '123';
const popupPortalEl = document.createElement('div');
const getDefaults = (): Props => ({
activeAnnotationId: null,
createDrawing: jest.fn(),
Expand All @@ -65,6 +66,7 @@ describe('PopupLayer', () => {
location: 1,
message: '',
mode: Mode.HIGHLIGHT,
popupPortalEl,
referenceId: '123',
resetCreator: jest.fn(),
setMessage: jest.fn(),
Expand All @@ -75,6 +77,7 @@ describe('PopupLayer', () => {

beforeEach(() => {
document.body.innerHTML = `<div data-ba-reference-id="${referenceId}"></div>`;
document.body.appendChild(popupPortalEl);
mockOnCancel = undefined;
mockOnChange = undefined;
mockOnSubmit = undefined;
Expand Down Expand Up @@ -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', () => {
Expand Down