New: Flip point annotation dialog if in lower half of file (#353)

src/lib/annotations/AnnotationDialog.js

@@ -4,6 +4,10 @@ import * as annotatorUtil from './annotatorUtil';
 import * as constants from './annotationConstants';
 import { ICON_CLOSE, ICON_DELETE } from '../icons/icons';
 
+const POINT_ANNOTATION_ICON_HEIGHT = 31;
+const POINT_ANNOTATION_ICON_DOT_HEIGHT = 8;
+const CLASS_FLIPPED_DIALOG = 'bp-annotation-dialog-flipped';
+
 const CLASS_BUTTON_DELETE_COMMENT = 'delete-comment-btn';
 const CLASS_CANCEL_DELETE = 'cancel-delete-btn';
 const CLASS_CANNOT_ANNOTATE = 'cannot-annotate';
@@ -186,6 +190,9 @@ class AnnotationDialog extends EventEmitter {
 
         annotatorUtil.hideElement(this.element);
         this.deactivateReply();
+
+        // Make sure entire thread icon displays for flipped dialogs
+        this.toggleFlippedThreadEl();
     }
 
     /**
@@ -691,6 +698,82 @@ class AnnotationDialog extends EventEmitter {
             </section>`.trim();
         return dialogEl;
     }
+
+    /**
+     * Flip the annotations dialog if the dialog would appear in the lower
+     * half of the viewer
+     *
+     * @private
+     * @param {number} yPos - y coordinate for the top of the dialog
+     * @param {number} containerHeight - height of the current annotation
+     * container/page
+     * @return {void}
+     */
+    flipDialog(yPos, containerHeight) {
+        let top = '';
+        let bottom = '';
+        const iconPadding = POINT_ANNOTATION_ICON_HEIGHT - POINT_ANNOTATION_ICON_DOT_HEIGHT / 2;
+        const annotationCaretEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_CARET);
+
+        if (yPos <= containerHeight / 2) {
+            // Keep dialog below the icon if in the top half of the viewport
+            top = `${yPos - POINT_ANNOTATION_ICON_DOT_HEIGHT}px`;
+            bottom = '';
+
+            this.element.classList.remove(CLASS_FLIPPED_DIALOG);
+
+            annotationCaretEl.style.bottom = '';
+        } else {
+            // Flip dialog to above the icon if in the lower half of the viewport
+            const flippedY = containerHeight - yPos - iconPadding;
+            top = '';
+            bottom = `${flippedY}px`;
+
+            this.element.classList.add(CLASS_FLIPPED_DIALOG);
+
+            // Adjust dialog caret
+            annotationCaretEl.style.top = '';
+            annotationCaretEl.style.bottom = '0px';
+        }
+
+        this.fitDialogHeightInPage();
+        this.toggleFlippedThreadEl();
+        return { top, bottom };
+    }
+
+    /**
+     * Show/hide the top portion of the annotations icon based on if the
+     * entire dialog is flipped
+     *
+     * @private
+     * @return {void}
+     */
+    toggleFlippedThreadEl() {
+        if (!this.element || !this.threadEl) {
+            return;
+        }
+
+        const isDialogFlipped = this.element.classList.contains(CLASS_FLIPPED_DIALOG);
+        if (!isDialogFlipped) {
+            return;
+        }
+
+        if (this.element.classList.contains(constants.CLASS_HIDDEN)) {
+            this.threadEl.classList.remove(CLASS_FLIPPED_DIALOG);
+        } else {
+            this.threadEl.classList.add(CLASS_FLIPPED_DIALOG);
+        }
+    }
+
+    /**
+     * Set max height for dialog to prevent the dialog from being cut off
+     *
+     * @private
+     * @return {void}
+     */
+    fitDialogHeightInPage() {
+        this.dialogEl.style.maxHeight = `${this.container.clientHeight / 2}px`;
+    }
 }
 
 export default AnnotationDialog;

src/lib/annotations/Annotator.scss

@@ -274,6 +274,17 @@ $avatar-color-9: #f22c44;
     }
 }
 
+.bp-annotation-dialog-flipped {
+    path,
+    rect {
+        display: none;
+    }
+
+    .bp-annotation-caret {
+        transform: rotate(180deg);
+    }
+}
+
 .bp-annotation-profile {
     background-color: $tendemob-grey;
     border-radius: 16px; // Circle

src/lib/annotations/__tests__/AnnotationDialog-test.js

@@ -4,6 +4,7 @@ import AnnotationDialog from '../AnnotationDialog';
 import * as annotatorUtil from '../annotatorUtil';
 import * as constants from '../annotationConstants';
 
+const CLASS_FLIPPED_DIALOG = 'bp-annotation-dialog-flipped';
 const CLASS_CANCEL_DELETE = 'cancel-delete-btn';
 const CLASS_CANNOT_ANNOTATE = 'cannot-annotate';
 const CLASS_REPLY_TEXTAREA = 'reply-textarea';
@@ -215,9 +216,18 @@ describe('lib/annotations/AnnotationDialog', () => {
     });
 
     describe('hide()', () => {
+        it('should do nothing if element is already hidden', () => {
+            dialog.element.classList.add(constants.CLASS_HIDDEN);
+            sandbox.stub(annotatorUtil, 'hideElement');
+            dialog.hide();
+            expect(annotatorUtil.hideElement).to.not.have.called;
+        });
+
         it('should hide dialog immediately', () => {
+            sandbox.stub(dialog, 'toggleFlippedThreadEl');
             dialog.hide();
             expect(dialog.element).to.have.class(constants.CLASS_HIDDEN);
+            expect(dialog.toggleFlippedThreadEl).to.be.called;
         });
 
         it('should hide the mobile dialog if using a mobile browser', () => {
@@ -891,4 +901,76 @@ describe('lib/annotations/AnnotationDialog', () => {
             expect(showSectionEl).to.not.have.class(constants.CLASS_HIDDEN);
         });
     });
+
+    describe('flipDialog()', () => {
+        const containerHeight = 5;
+
+        beforeEach(() => {
+            sandbox.stub(dialog.element, 'querySelector').returns(document.createElement('div'));
+            sandbox.stub(dialog, 'fitDialogHeightInPage');
+            sandbox.stub(dialog, 'toggleFlippedThreadEl');
+        });
+
+        afterEach(() => {
+            dialog.element = null;
+        });
+
+        it('should keep the dialog below the annotation icon if the annotation is in the top half of the viewport', () => {
+            const { top, bottom } = dialog.flipDialog(2, containerHeight);
+            expect(dialog.element).to.not.have.class(CLASS_FLIPPED_DIALOG);
+            expect(top).not.equals('');
+            expect(bottom).equals('');
+            expect(dialog.fitDialogHeightInPage).to.be.called;
+            expect(dialog.toggleFlippedThreadEl).to.be.called;
+        });
+
+        it('should flip the dialog above the annotation icon if the annotation is in the lower half of the viewport', () => {
+            const { top, bottom } = dialog.flipDialog(4, containerHeight);
+            expect(dialog.element).to.have.class(CLASS_FLIPPED_DIALOG);
+            expect(top).equals('');
+            expect(bottom).not.equals('');
+        });
+    });
+
+    describe('toggleFlippedThreadEl()', () => {
+        beforeEach(() => {
+            dialog.threadEl = document.createElement('div');
+        });
+
+        it('should do nothing if the dialog is not flipped', () => {
+            stubs.add = sandbox.stub(dialog.threadEl.classList, 'add');
+            stubs.remove = sandbox.stub(dialog.threadEl.classList, 'remove');
+            dialog.toggleFlippedThreadEl();
+            expect(stubs.add).to.not.be.called;
+            expect(stubs.remove).to.not.be.called;
+        });
+
+        it('should reset thread icon if dialog is flipped and hidden', () => {
+            dialog.element.classList.add(CLASS_FLIPPED_DIALOG);
+            stubs.add = sandbox.stub(dialog.threadEl.classList, 'add');
+            stubs.remove = sandbox.stub(dialog.threadEl.classList, 'remove');
+            dialog.toggleFlippedThreadEl();
+            expect(stubs.add).to.be.called;
+            expect(stubs.remove).to.not.be.called;
+        })
+
+        it('should flip thread icon if dialog is flipped and not hidden', () => {
+            dialog.element.classList.add(CLASS_FLIPPED_DIALOG);
+            dialog.element.classList.add(constants.CLASS_HIDDEN);
+            stubs.add = sandbox.stub(dialog.threadEl.classList, 'add');
+            stubs.remove = sandbox.stub(dialog.threadEl.classList, 'remove');
+            dialog.toggleFlippedThreadEl();
+            expect(stubs.add).to.not.be.called;
+            expect(stubs.remove).to.be.called;
+        })
+    });
+
+    describe('fitDialogHeightInPage()', () => {
+        it('should allow scrolling on annotations dialog if file is a powerpoint', () => {
+            dialog.dialogEl = { style: {} };
+            dialog.container = { clientHeight: 100 };
+            dialog.fitDialogHeightInPage();
+            expect(dialog.dialogEl.style.maxHeight).equals('50px');
+        });
+    });
 });

src/lib/annotations/doc/DocHighlightDialog.js

@@ -19,7 +19,7 @@ class DocHighlightDialog extends AnnotationDialog {
     // Public
     //--------------------------------------------------------------------------
 
-    /** 
+    /**
      * Saves an annotation with the associated text or blank if only
      * highlighting. Only adds an annotation to the dialog if it contains text.
      * The annotation is still added to the thread on the server side.
@@ -67,7 +67,7 @@ class DocHighlightDialog extends AnnotationDialog {
 
     /**
      * Set the state of the dialog so comments are hidden, if they're currently shown.
-     * 
+     *
      * @public
      * @return {void}
      */
@@ -150,7 +150,7 @@ class DocHighlightDialog extends AnnotationDialog {
 
         this.element.style.left = `${dialogX}px`;
         this.element.style.top = `${dialogY + PAGE_PADDING_TOP}px`;
-        docAnnotatorUtil.fitDialogHeightInPage(this.annotatedElement, this.element, pageDimensions.height, dialogY);
+        this.fitDialogHeightInPage();
         annotatorUtil.showElement(this.element);
     }
 

src/lib/annotations/doc/DocPointDialog.js

@@ -1,10 +1,9 @@
 import autobind from 'autobind-decorator';
 import AnnotationDialog from '../AnnotationDialog';
 import * as annotatorUtil from '../annotatorUtil';
-import * as docAnnotatorUtil from './docAnnotatorUtil';
 
 const PAGE_PADDING_TOP = 15;
-const POINT_ANNOTATION_ICON_DOT_HEIGHT = 8;
+const POINT_ANNOTATION_ICON_HEIGHT = 31;
 const POINT_ANNOTATION_ICON_WIDTH = 24;
 
 @autobind
@@ -35,7 +34,7 @@ class DocPointDialog extends AnnotationDialog {
         let dialogLeftX = threadIconLeftX - dialogWidth / 2;
 
         // Adjusts Y position for transparent top border
-        const dialogTopY = this.threadEl.offsetTop + POINT_ANNOTATION_ICON_DOT_HEIGHT;
+        const dialogTopY = this.threadEl.offsetTop + POINT_ANNOTATION_ICON_HEIGHT;
 
         // Only reposition if one side is past page boundary - if both are,
         // just center the dialog and cause scrolling since there is nothing
@@ -49,9 +48,12 @@ class DocPointDialog extends AnnotationDialog {
         );
 
         // Position the dialog
-        this.element.style.left = `${dialogLeftX}px`;
-        this.element.style.top = `${dialogTopY + PAGE_PADDING_TOP}px`;
-        docAnnotatorUtil.fitDialogHeightInPage(this.annotatedElement, this.element, pageDimensions.height, dialogTopY);
+        this.element.style.left = `${dialogLeftX - 1}px`;
+
+        const pageHeight = pageDimensions.height + PAGE_PADDING_TOP;
+        const dialogPos = this.flipDialog(dialogTopY, pageHeight + POINT_ANNOTATION_ICON_HEIGHT);
+        this.element.style.top = dialogPos.top;
+        this.element.style.bottom = dialogPos.bottom;
     }
 }
 

src/lib/annotations/doc/__tests__/DocHighlightDialog-test.js

@@ -208,7 +208,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             stubs.width = sandbox.stub(dialog, 'getDialogWidth');
             stubs.caret = sandbox.stub(annotatorUtil, 'repositionCaret');
             stubs.show = sandbox.stub(annotatorUtil, 'showElement');
-            stubs.fit = sandbox.stub(docAnnotatorUtil, 'fitDialogHeightInPage');
+            stubs.fit = sandbox.stub(dialog, 'fitDialogHeightInPage');
         });
 
         it('should position the plain highlight dialog at the right place and show it', () => {

src/lib/annotations/doc/__tests__/DocPointDialog-test.js

@@ -39,13 +39,13 @@ describe('lib/annotations/doc/DocPointDialog', () => {
         it('should position the dialog at the right place and show it', () => {
             sandbox.stub(annotatorUtil, 'showElement');
             sandbox.stub(annotatorUtil, 'repositionCaret');
-            sandbox.stub(docAnnotatorUtil, 'fitDialogHeightInPage');
+            sandbox.stub(dialog, 'flipDialog').returns([]);
 
             dialog.position();
 
             expect(annotatorUtil.repositionCaret).to.have.been.called;
             expect(annotatorUtil.showElement).to.have.been.called;
-            expect(docAnnotatorUtil.fitDialogHeightInPage).to.have.been.called;
+            expect(dialog.flipDialog).to.have.been.called;
         });
     });
 });

src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js

@@ -82,23 +82,6 @@ describe('lib/annotations/doc/docAnnotatorUtil', () => {
         });
     });
 
-    describe('fitDialogHeightInPage()', () => {
-        it('should allow scrolling on annotations dialog if file is a powerpoint', () => {
-            const docEl = document.querySelector('.annotatedElement');
-            docEl.classList.add('bp-doc-presentation');
-            docEl.style.height = 100;
-
-            const dialogEl = document.querySelector(SELECTOR_ANNOTATION_DIALOG);
-            const pageHeight = 20;
-            const yPos = 5;
-
-            docAnnotatorUtil.fitDialogHeightInPage(docEl, dialogEl, pageHeight, yPos);
-
-            const annotationsEl = dialogEl.querySelector(SELECTOR_ANNOTATION_CONTAINER);
-            expect(annotationsEl.style.maxHeight).to.not.be.undefined;
-        });
-    });
-
     describe('isPointInPolyOpt()', () => {
         it('should return true if point is inside polygon', () => {
             const polygon = [[0, 0], [100, 0], [100, 100], [0, 100]];

src/lib/annotations/doc/docAnnotatorUtil.js

@@ -2,7 +2,6 @@ import * as annotatorUtil from '../annotatorUtil';
 import {
     CLASS_ANNOTATION_DIALOG,
     CLASS_ANNOTATION_HIGHLIGHT_DIALOG,
-    SELECTOR_ANNOTATION_CONTAINER,
     PAGE_PADDING_TOP,
     PAGE_PADDING_BOTTOM,
     DATA_TYPE_ANNOTATION_DIALOG,
@@ -24,7 +23,6 @@ const HEIGHT_PADDING = 30;
 // PDF unit = 1/72 inch, CSS pixel = 1/92 inch
 const PDF_UNIT_TO_CSS_PIXEL = 4 / 3;
 const CSS_PIXEL_TO_PDF_UNIT = 3 / 4;
-const HIGHLIGHT_DIALOG_HEIGHT = 48;
 
 const DIALOG_DATATYPES = [
     DATA_TYPE_ANNOTATION_DIALOG,
@@ -69,29 +67,6 @@ export function hasActiveDialog(annotatedEl) {
     return !!(commentsDialogEl || highlightDialogEl);
 }
 
-/**
- * Set max height for dialog on powerpoint previews to prevent the
- * dialog from being cut off since the presentation viewer doesn't allow
- * the annotations dialog to overflow below the file
- *
- * @private
- * @param {HTMLElement} annotatedElement - Annotated element
- * @param {HTMLElement} dialogEl - Annotations dialog element
- * @param {number} pageHeight - Page height
- * @param {number} dialogY - Dialog y position
- * @return {void}
- */
-export function fitDialogHeightInPage(annotatedElement, dialogEl, pageHeight, dialogY) {
-    if (isPresentation(annotatedElement)) {
-        const wrapperHeight = annotatedElement.clientHeight;
-        const topPadding = (wrapperHeight - pageHeight) / 2;
-        const maxHeight = wrapperHeight - dialogY - topPadding - HIGHLIGHT_DIALOG_HEIGHT;
-
-        const annotationsEl = dialogEl.querySelector(SELECTOR_ANNOTATION_CONTAINER);
-        annotationsEl.style.maxHeight = `${maxHeight}px`;
-    }
-}
-
 //------------------------------------------------------------------------------
 // Highlight Utils
 //------------------------------------------------------------------------------