From 6d71dee7451c9085df9ac2b86b5fe8faedecb3c0 Mon Sep 17 00:00:00 2001
From: Conrad Chan <conrad@box.com>
Date: Fri, 30 Nov 2018 15:39:03 -0800
Subject: [PATCH] Fix: Point image annotations popover should be on annotated
 element (#305)

---
 src/image/ImagePointThread.js                | 16 ++++++++---
 src/image/__tests__/ImagePointThread-test.js | 28 +++++++++++++++++++-
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/image/ImagePointThread.js b/src/image/ImagePointThread.js
index 606ebf331..b5a026671 100644
--- a/src/image/ImagePointThread.js
+++ b/src/image/ImagePointThread.js
@@ -13,6 +13,17 @@ import {
 } from '../constants';
 
 class ImagePointThread extends AnnotationThread {
+    /**
+     * Gets the popover parent for image point threads. The popover parent
+     * should be not the image element but rather the annotatedElement
+     *
+     * @override
+     * @return {HTMLElement} The correct parent based on mobile view or not
+     */
+    getPopoverParent() {
+        return shouldDisplayMobileUI(this.container) ? this.container : this.annotatedElement;
+    }
+
     /** @inheritdoc */
     show() {
         const [browserX, browserY] = getBrowserCoordinatesFromLocation(this.location, this.annotatedElement);
@@ -50,9 +61,8 @@ class ImagePointThread extends AnnotationThread {
         const flippedPopoverOffset = isUpperHalf
             ? 0
             : popoverEl.getBoundingClientRect().height +
-              POINT_ANNOTATION_ICON_HEIGHT +
-              ANNOTATION_POPOVER_CARET_HEIGHT +
-              POINT_ANNOTATION_ICON_DOT_HEIGHT;
+              POINT_ANNOTATION_ICON_DOT_HEIGHT * 2 +
+              ANNOTATION_POPOVER_CARET_HEIGHT;
 
         const dialogTopY =
             this.element.offsetTop +
diff --git a/src/image/__tests__/ImagePointThread-test.js b/src/image/__tests__/ImagePointThread-test.js
index 1dee26e68..e84b0920a 100644
--- a/src/image/__tests__/ImagePointThread-test.js
+++ b/src/image/__tests__/ImagePointThread-test.js
@@ -13,16 +13,24 @@ const html = `<div class="annotated-element" data-page-number="1">
 
 describe('image/ImagePointThread', () => {
     let rootElement;
+    let container;
+    let annotatedElement;
 
     beforeEach(() => {
         rootElement = document.createElement('div');
         rootElement.innerHTML = html;
         document.body.appendChild(rootElement);
 
+        container = document.createElement('div');
+        container.classList.add('container');
+
+        annotatedElement = document.querySelector(SELECTOR_ANNOTATED_ELEMENT);
+
         thread = new ImagePointThread({
-            annotatedElement: document.querySelector(SELECTOR_ANNOTATED_ELEMENT),
+            annotatedElement,
             annotations: [],
             api: {},
+            container,
             fileVersionId: 1,
             location: {},
             threadID: 2,
@@ -73,4 +81,22 @@ describe('image/ImagePointThread', () => {
             expect(thread.renderAnnotationPopover).not.toBeCalled();
         });
     });
+
+    describe('getPopoverParent()', () => {
+        beforeEach(() => {
+            util.shouldDisplayMobileUI = jest.fn();
+        });
+
+        it('should return the container if mobile', () => {
+            util.shouldDisplayMobileUI.mockReturnValueOnce(true);
+
+            expect(thread.getPopoverParent()).toBe(container);
+        });
+
+        it('should return the annotatedEleemnt if not mobile', () => {
+            util.shouldDisplayMobileUI.mockReturnValueOnce(false);
+
+            expect(thread.getPopoverParent()).toBe(annotatedElement);
+        });
+    });
 });