New: Allowing users to add new mobile point annotations (#177)

src/lib/annotations/AnnotationDialog.js

@@ -150,6 +150,12 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
         dialogCloseButtonEl.removeEventListener('click', this.hideMobileDialog);
 
         annotatorUtil.hideElement(this.element);
+        this.unbindDOMListeners();
+
+        // Cancel any unsaved annotations
+        if (!this.hasAnnotations) {
+            this.cancelAnnotation();
+        }
     }
 
     /**
@@ -218,7 +224,7 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
     /**
      * Sets up the dialog element.
      *
-     * @param {Annotation[]} Annotations - to show in the dialog
+     * @param {Annotation[]} annotations - to show in the dialog
      * @return {void}
      * @protected
      */
@@ -370,7 +376,12 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
                 break;
             // Clicking 'Cancel' button to cancel the annotation
             case 'cancel-annotation-btn':
-                this.cancelAnnotation();
+                if (this.isMobile) {
+                    this.hideMobileDialog();
+                } else {
+                    this.cancelAnnotation();
+                }
+
                 this.deactivateReply(true);
                 break;
             // Clicking inside reply text area
@@ -529,7 +540,7 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
      * Deactivate reply textarea.
      *
      * @private
-     * @param {Boolean} clearText - Whether or not text in text area should be cleared
+     * @param {boolean} clearText - Whether or not text in text area should be cleared
      * @return {void}
      */
     deactivateReply(clearText) {

src/lib/annotations/AnnotationThread.js

@@ -58,7 +58,7 @@ import { ICON_PLACED_ANNOTATION } from '../icons/icons';
      * @return {void}
      */
     destroy() {
-        if (this.dialog) {
+        if (this.dialog && !this.isMobile) {
             this.unbindCustomListenersOnDialog();
             this.dialog.destroy();
         }
@@ -204,6 +204,10 @@ import { ICON_PLACED_ANNOTATION } from '../icons/icons';
 
             // If this annotation was the last one in the thread, destroy the thread
         } else if (this.annotations.length === 0 || annotatorUtil.isPlainHighlight(this.annotations)) {
+            if (this.isMobile) {
+                this.dialog.removeAnnotation(annotationID);
+                this.dialog.hideMobileDialog();
+            }
             this.destroy();
 
             // Otherwise, remove deleted annotation from dialog
@@ -353,7 +357,7 @@ import { ICON_PLACED_ANNOTATION } from '../icons/icons';
         }
 
         this.dialog.addListener('annotationcreate', this.createAnnotation);
-        this.dialog.addListener('annotationcancel', this.destroy);
+        this.dialog.addListener('annotationcancel', this.cancelUnsavedAnnotation);
         this.dialog.addListener('annotationdelete', this.deleteAnnotationWithID);
     }
 
@@ -373,6 +377,23 @@ import { ICON_PLACED_ANNOTATION } from '../icons/icons';
         this.dialog.removeAllListeners('annotationdelete');
     }
 
+    /**
+     * Destroys mobile and pending/pending-active annotation threads
+     *
+     * @protected
+     * @return {void}
+     */
+    cancelUnsavedAnnotation() {
+        if (
+            !this.isMobile &&
+            this.state !== constants.ANNOTATION_STATE_PENDING &&
+            this.state !== constants.ANNOTATION_STATE_PENDING_ACTIVE
+        ) {
+            return;
+        }
+        this.destroy();
+    }
+
     //--------------------------------------------------------------------------
     // Private
     //--------------------------------------------------------------------------
@@ -443,7 +464,7 @@ import { ICON_PLACED_ANNOTATION } from '../icons/icons';
      * Creates a new point annotation
      *
      * @private
-     * @param data - Annotation data
+     * @param {Object} data - Annotation data
      * @return {void}
      */
     createAnnotation(data) {
@@ -454,7 +475,7 @@ import { ICON_PLACED_ANNOTATION } from '../icons/icons';
      * Deletes annotation with annotationID from thread
      *
      * @private
-     * @param data - Annotation data
+     * @param {Object} data - Annotation data
      * @return {void}
      */
     deleteAnnotationWithID(data) {

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

@@ -182,8 +182,14 @@ describe('lib/annotations/AnnotationDialog', () => {
         it('should hide and reset the mobile annotations dialog', () => {
             dialog.element = document.querySelector('.bp-mobile-annotation-dialog');
             stubs.hide = sandbox.stub(annotatorUtil, 'hideElement');
+            stubs.unbind = sandbox.stub(dialog, 'unbindDOMListeners');
+            stubs.cancel = sandbox.stub(dialog, 'cancelAnnotation');
+            dialog.hasAnnotations = true;
+
             dialog.hideMobileDialog();
             expect(stubs.hide).to.be.called;
+            expect(stubs.unbind).to.be.called;
+            expect(stubs.cancel).to.not.be.called;
             expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.false;
         });
 
@@ -192,6 +198,15 @@ describe('lib/annotations/AnnotationDialog', () => {
             dialog.hideMobileDialog();
             expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.false;
         });
+
+        it('should cancel unsaved annotations only if the dialog does not have annotations', () => {
+            dialog.element = document.querySelector('.bp-mobile-annotation-dialog');
+            stubs.cancel = sandbox.stub(dialog, 'cancelAnnotation');
+            dialog.hasAnnotations = false;
+
+            dialog.hideMobileDialog();
+            expect(stubs.cancel).to.be.called;
+        });
     });
 
     describe('hide()', () => {
@@ -460,6 +475,7 @@ describe('lib/annotations/AnnotationDialog', () => {
             stubs.hideDelete = sandbox.stub(dialog, 'hideDeleteConfirmation');
             stubs.delete = sandbox.stub(dialog, 'deleteAnnotation');
             stubs.reply = sandbox.stub(dialog, 'postReply');
+            stubs.hideMobile = sandbox.stub(dialog, 'hideMobileDialog');
         });
 
         it('should post annotation when post annotation button is clicked', () => {
@@ -471,9 +487,21 @@ describe('lib/annotations/AnnotationDialog', () => {
 
         it('should cancel annotation when cancel annotation button is clicked', () => {
             stubs.findClosest.returns('cancel-annotation-btn');
+            dialog.isMobile = false;
 
             dialog.clickHandler(stubs.event);
             expect(stubs.cancel).to.be.called;
+            expect(stubs.hideMobile).to.not.be.called;
+            expect(stubs.deactivate).to.be.calledWith(true);
+        });
+
+        it('should only hide the mobile dialog when the cancel annotation button is clicked on mobile', () => {
+            stubs.findClosest.returns('cancel-annotation-btn');
+            dialog.isMobile = true;
+
+            dialog.clickHandler(stubs.event);
+            expect(stubs.cancel).to.not.be.called;
+            expect(stubs.hideMobile).to.be.called;
             expect(stubs.deactivate).to.be.calledWith(true);
         });
 

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

@@ -67,6 +67,17 @@ describe('lib/annotations/AnnotationThread', () => {
             expect(stubs.unbindDOM).to.be.called;
             expect(stubs.emit).to.be.calledWith('threaddeleted');
         });
+
+        it('should not destroy the dialog on mobile', () => {
+            stubs.unbindCustom = sandbox.stub(thread, 'unbindCustomListenersOnDialog');
+            stubs.destroyDialog = sandbox.stub(thread.dialog, 'destroy');
+            thread.element = null;
+            thread.isMobile = true;
+
+            thread.destroy();
+            expect(stubs.unbindCustom).to.not.be.called;
+            expect(stubs.destroyDialog).to.not.be.called;
+        });
     });
 
     describe('hide()', () => {
@@ -199,7 +210,8 @@ describe('lib/annotations/AnnotationThread', () => {
                 removeAllListeners: () => {},
                 show: () => {},
                 hide: () => {},
-                removeAnnotation: () => {}
+                removeAnnotation: () => {},
+                hideMobileDialog: () => {}
             };
             stubs.dialogMock = sandbox.mock(thread.dialog);
 
@@ -210,10 +222,20 @@ describe('lib/annotations/AnnotationThread', () => {
         });
 
         it('should destroy the thread if the deleted annotation was the last annotation in the thread', () => {
+            thread.isMobile = false;
+            stubs.dialogMock.expects('removeAnnotation').never();
+            stubs.dialogMock.expects('hideMobileDialog').never();
             thread.deleteAnnotation('someID', false);
             expect(stubs.destroy).to.be.called;
         });
 
+        it('should destroy the thread and hide the mobile dialog if the deleted annotation was the last annotation in the thread on mobile', () => {
+            thread.isMobile = true;
+            stubs.dialogMock.expects('removeAnnotation');
+            stubs.dialogMock.expects('hideMobileDialog');
+            thread.deleteAnnotation('someID', false);
+        });
+
         it('should remove the relevant annotation from its dialog if the deleted annotation was not the last one', () => {
             // Add another annotation to thread so 'someID' isn't the only annotation
             thread.annotations.push(stubs.annotation2);
@@ -437,6 +459,28 @@ describe('lib/annotations/AnnotationThread', () => {
         });
     });
 
+    describe('cancelUnsavedAnnotation()', () => {
+        it('should only destroy thread if on a mobile browser or in a pending/pending-active state', () => {
+            sandbox.stub(thread, 'destroy');
+
+            // mobile
+            thread.isMobile = true;
+            thread.cancelUnsavedAnnotation();
+            expect(thread.destroy).to.be.called;
+
+            // 'pending' state
+            thread.isMobile = false;
+            thread.state = constants.ANNOTATION_STATE_PENDING;
+            thread.cancelUnsavedAnnotation();
+            expect(thread.destroy).to.be.called;
+
+            // 'pending-active' state
+            thread.state = constants.ANNOTATION_STATE_PENDING_ACTIVE;
+            thread.cancelUnsavedAnnotation();
+            expect(thread.destroy).to.be.called;
+        });
+    });
+
     describe('createElement()', () => {
         it('should create an element with the right class and attribute', () => {
             const element = thread.createElement();

src/lib/viewers/BaseViewer.js

@@ -58,7 +58,6 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
     /**
      * [constructor]
      *
-     * @param {HTMLElement} containerEl - The container
      * @param {Object} options - some options
      * @return {BaseViewer} Instance of base viewer
      */
@@ -583,7 +582,7 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
             const { file } = this.options;
 
             // Users can currently only view annotations on mobile
-            this.canAnnotate = checkPermission(file, PERMISSION_ANNOTATE) && !this.isMobile;
+            this.canAnnotate = checkPermission(file, PERMISSION_ANNOTATE);
             if (this.canAnnotate) {
                 this.showAnnotateButton(this.getPointModeClickHandler());
             }
@@ -592,7 +591,7 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
     }
 
     /**
-     * Orient anntations to the correct scale and orientatio of the annotated document.
+     * Orient annotations to the correct scale and orientation of the annotated document.
      * @TODO(jholdstock|spramod): Remove this once we are emitting the correct messaging.
      *
      * @protected