Skip to content

Commit

Permalink
Chore: Remove util method
Browse files Browse the repository at this point in the history
  • Loading branch information
pramodsum committed Nov 20, 2018
1 parent f368097 commit 765b7bf
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 59 deletions.
24 changes: 21 additions & 3 deletions src/AnnotationThread.js
Expand Up @@ -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.
*
Expand All @@ -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(
<AnnotationPopover
id={this.id}
Expand Down Expand Up @@ -495,8 +511,10 @@ class AnnotationThread extends EventEmitter {
return;
}

const pageEl = util.getPopoverParent(this.container, this.annotatedElement, this.location);
pageEl.scrollIntoView();
const pageEl = util.getPageEl(this.annotatedElement, this.location.page);
if (pageEl) {
pageEl.scrollIntoView();
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/AnnotationThread-test.js
Expand Up @@ -380,7 +380,7 @@ describe('AnnotationThread', () => {
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', () => {
Expand Down
21 changes: 18 additions & 3 deletions src/doc/CreateHighlightDialog.js
Expand Up @@ -11,8 +11,8 @@ import {
findElement,
getPopoverLayer,
isInElement,
shouldDisplayMobileUI,
getPopoverParent
getPageEl,
shouldDisplayMobileUI
} from '../util';
import { getDialogCoordsFromRange } from './docUtil';
import {
Expand Down Expand Up @@ -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.
*
Expand All @@ -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(
Expand Down
12 changes: 2 additions & 10 deletions src/doc/DocDrawingThread.js
Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/doc/DocHighlightThread.js
Expand Up @@ -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(
Expand Down
12 changes: 2 additions & 10 deletions 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,
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 4 additions & 15 deletions 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,
Expand Down Expand Up @@ -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 +
Expand All @@ -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);

Expand Down
16 changes: 0 additions & 16 deletions src/util.js
Expand Up @@ -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.
Expand Down

0 comments on commit 765b7bf

Please sign in to comment.