diff --git a/src/AnnotationThread.js b/src/AnnotationThread.js index 737d6f789..ba7fc4d0c 100644 --- a/src/AnnotationThread.js +++ b/src/AnnotationThread.js @@ -151,6 +151,22 @@ class AnnotationThread extends EventEmitter { throw new Error('Implement me!'); }; + /** + * Returns the parent element for the annotation popover + * + * @return {HTMLElement} Parent element for the annotation popover + */ + getPopoverParent() { + if (!this.location || !this.location.page) { + return this.annotatedElement; + } + + return util.shouldDisplayMobileUI(this.container) + ? this.container + : // $FlowFixMe + util.getPageEl(this.annotatedElement, this.location.page); + } + /** * Shows the appropriate annotation dialog for this thread. * @@ -165,7 +181,7 @@ class AnnotationThread extends EventEmitter { const isPending = this.state === STATES.pending; - const pageEl = util.getPopoverParent(this.container, this.annotatedElement, this.location); + const pageEl = this.getPopoverParent(); this.popoverComponent = render( { pageEl = { scrollIntoView: jest.fn() }; - thread.getPopoverParent = jest.fn().mockReturnValue(pageEl); + util.getPageEl = jest.fn().mockReturnValue(pageEl); }); it('should do nothing if annotation does not have a location or page', () => { diff --git a/src/doc/CreateHighlightDialog.js b/src/doc/CreateHighlightDialog.js index 5148c544a..ce32d2276 100644 --- a/src/doc/CreateHighlightDialog.js +++ b/src/doc/CreateHighlightDialog.js @@ -11,8 +11,8 @@ import { findElement, getPopoverLayer, isInElement, - shouldDisplayMobileUI, - getPopoverParent + getPageEl, + shouldDisplayMobileUI } from '../util'; import { getDialogCoordsFromRange } from './docUtil'; import { @@ -123,6 +123,21 @@ class CreateHighlightDialog extends EventEmitter { this.renderAnnotationPopover(type); } + /** + * Returns the parent element for the create annotation popover + * + * @return {HTMLElement} Parent element for the annotation popover + */ + getPopoverParent(): HTMLElement { + if (!this.pageInfo || !this.pageInfo.page) { + return this.annotatedElement; + } + + return shouldDisplayMobileUI(this.container) + ? this.container + : getPageEl(this.annotatedElement, this.pageInfo.page); + } + /** * Shows the appropriate annotation dialog for this thread. * @@ -131,7 +146,7 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ renderAnnotationPopover = (type: AnnotationType = TYPES.highlight) => { - const pageEl = getPopoverParent(this.container, this.annotatedElement, this.pageInfo); + const pageEl = this.getPopoverParent(); const popoverLayer = getPopoverLayer(pageEl); this.createPopoverComponent = render( diff --git a/src/doc/DocDrawingThread.js b/src/doc/DocDrawingThread.js index becbf5301..75a786e2a 100644 --- a/src/doc/DocDrawingThread.js +++ b/src/doc/DocDrawingThread.js @@ -10,15 +10,7 @@ import { SELECTOR_CLASS_ANNOTATION_POPOVER } from '../constants'; import { getBrowserCoordinatesFromLocation, getContext } from './docUtil'; -import { - createLocation, - getScale, - repositionCaret, - findElement, - getPageEl, - shouldDisplayMobileUI, - getPopoverParent -} from '../util'; +import { createLocation, getScale, repositionCaret, findElement, getPageEl, shouldDisplayMobileUI } from '../util'; class DocDrawingThread extends DrawingThread { /** @property {HTMLElement} - Page element being observed */ @@ -333,7 +325,7 @@ class DocDrawingThread extends DrawingThread { } if (!this.pageEl) { - this.pageEl = getPopoverParent(this.container, this.annotatedElement, this.location); + this.pageEl = this.getPopoverParent(); } // Render popover so we can get width diff --git a/src/doc/DocHighlightThread.js b/src/doc/DocHighlightThread.js index 8db96c6fe..369a76502 100644 --- a/src/doc/DocHighlightThread.js +++ b/src/doc/DocHighlightThread.js @@ -463,7 +463,7 @@ class DocHighlightThread extends AnnotationThread { // Position it below lower right corner or center of the highlight - we need // to reposition every time since the DOM could have changed from // zooming - const pageEl = util.getPopoverParent(this.container, this.annotatedElement, this.location); + const pageEl = this.popoverParent; const pageDimensions = pageEl.getBoundingClientRect(); const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM; const [browserX, browserY] = docUtil.getScaledPDFCoordinates( diff --git a/src/doc/DocPointThread.js b/src/doc/DocPointThread.js index 4a38fbcd2..a2858c823 100644 --- a/src/doc/DocPointThread.js +++ b/src/doc/DocPointThread.js @@ -1,14 +1,6 @@ // @flow import AnnotationThread from '../AnnotationThread'; -import { - getPageEl, - showElement, - findElement, - repositionCaret, - shouldDisplayMobileUI, - isInUpperHalf, - getPopoverParent -} from '../util'; +import { getPageEl, showElement, findElement, repositionCaret, shouldDisplayMobileUI, isInUpperHalf } from '../util'; import { getBrowserCoordinatesFromLocation } from './docUtil'; import { STATES, @@ -61,7 +53,7 @@ class DocPointThread extends AnnotationThread { return; } - const pageEl = getPopoverParent(this.container, this.annotatedElement, this.location); + const pageEl = this.popoverParent; const popoverEl = findElement( this.annotatedElement, SELECTOR_CLASS_ANNOTATION_POPOVER, diff --git a/src/image/ImagePointThread.js b/src/image/ImagePointThread.js index c35766546..f05b8ba06 100644 --- a/src/image/ImagePointThread.js +++ b/src/image/ImagePointThread.js @@ -1,13 +1,6 @@ // @flow import AnnotationThread from '../AnnotationThread'; -import { - showElement, - shouldDisplayMobileUI, - repositionCaret, - findElement, - isInUpperHalf, - getPopoverParent -} from '../util'; +import { showElement, shouldDisplayMobileUI, repositionCaret, findElement, isInUpperHalf } from '../util'; import { getBrowserCoordinatesFromLocation } from './imageUtil'; import { STATES, @@ -52,15 +45,11 @@ class ImagePointThread extends AnnotationThread { const dialogDimensions = popoverEl.getBoundingClientRect(); const dialogWidth = dialogDimensions.width; - // Get image tag inside viewer, based on page number. All images are page 1 by default. - const imageEl = getPopoverParent(this.container, this.annotatedElement); - // Center middle of dialog with point - this coordinate is with respect to the page const threadIconLeftX = this.element.offsetLeft + POINT_ANNOTATION_ICON_WIDTH / 2; let dialogLeftX = threadIconLeftX - dialogWidth / 2; - const isUpperHalf = isInUpperHalf(this.element, imageEl); - + const isUpperHalf = isInUpperHalf(this.element, this.popoverParent); const flippedPopoverOffset = isUpperHalf ? 0 : popoverEl.getBoundingClientRect().height + @@ -82,8 +71,8 @@ class ImagePointThread extends AnnotationThread { // just center the dialog and cause scrolling since there is nothing // else we can do const pageWidth = - imageEl.clientWidth > this.annotatedElement.clientWidth - ? imageEl.clientWidth + this.popoverParent.clientWidth > this.annotatedElement.clientWidth + ? this.popoverParent.clientWidth : this.annotatedElement.clientWidth; dialogLeftX = repositionCaret(popoverEl, dialogLeftX, dialogWidth, threadIconLeftX, pageWidth, !isUpperHalf); diff --git a/src/util.js b/src/util.js index 47f388c39..a5d26d140 100644 --- a/src/util.js +++ b/src/util.js @@ -108,22 +108,6 @@ export function getPageEl(annotatedEl, pageNum) { return annotatedEl.querySelector(`[data-page-number="${pageNum}"]`); } -/** - * Returns the parent element for the annotation popover - * - * @param {HTMLElement} container - Preview container - * @param {HTMLElement} annotatedElement - Annotated element - * @param {Object} location - event/annotation location container page number - * @return {HTMLElement} Parent element for the annotation popover - */ -export function getPopoverParent(container, annotatedElement, location) { - if (!location || !location.page) { - return this.annotatedElement; - } - - return shouldDisplayMobileUI(container) ? container : getPageEl(annotatedElement, location.page); -} - /** * Finds an existing annotation popover layer or creates one if it does * not already exist and appends the layer to the page.