Fix: Mobile dialog doesn't properly close on delete cancel/confirm (#148

)
box · Mar 28, 2018 · 523217a · 523217a
1 parent 9c94ca6
commit 523217a
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 17 deletions.
diff --git a/src/AnnotationDialog.js b/src/AnnotationDialog.js
@@ -193,11 +193,8 @@ class AnnotationDialog extends EventEmitter {
 
         // Clear annotations from dialog
         util.hideElement(this.element);
-        this.element = util.generateMobileDialogEl();
         this.unbindDOMListeners();
-
-        // Cancel any unsaved annotations
-        this.cancelAnnotation();
+        this.element = util.generateMobileDialogEl();
     }
 
     /**
@@ -556,6 +553,10 @@ class AnnotationDialog extends EventEmitter {
     clickHandler(event) {
         event.stopPropagation();
 
+        if (this.isMobile) {
+            event.preventDefault();
+        }
+
         const eventTarget = event.target;
         const dataType = util.findClosestDataType(eventTarget);
         const annotationID = util.findClosestDataType(eventTarget, 'data-annotation-id');

diff --git a/src/__tests__/AnnotationDialog-test.js b/src/__tests__/AnnotationDialog-test.js
@@ -212,22 +212,11 @@ describe('AnnotationDialog', () => {
             dialog.element = document.querySelector(constants.SELECTOR_MOBILE_ANNOTATION_DIALOG);
             stubs.hide = sandbox.stub(util, '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.be.called;
-        });
-
-        it('should cancel unsaved annotations only if the dialog does not have annotations', () => {
-            dialog.element = document.querySelector(constants.SELECTOR_MOBILE_ANNOTATION_DIALOG);
-            stubs.cancel = sandbox.stub(dialog, 'cancelAnnotation');
-            dialog.hasAnnotations = false;
-
-            dialog.hideMobileDialog();
-            expect(stubs.cancel).to.be.called;
         });
     });
 
@@ -707,8 +696,8 @@ describe('AnnotationDialog', () => {
     describe('clickHandler()', () => {
         beforeEach(() => {
             stubs.event = {
-                stopPropagation: () => {},
-                preventDefault: () => {},
+                stopPropagation: sandbox.stub(),
+                preventDefault: sandbox.stub(),
                 target: document.createElement('div')
             };
             stubs.post = sandbox.stub(dialog, 'postAnnotation');
@@ -721,10 +710,24 @@ describe('AnnotationDialog', () => {
             stubs.delete = sandbox.stub(dialog, 'deleteAnnotation');
             stubs.reply = sandbox.stub(dialog, 'postReply');
             stubs.hideMobile = sandbox.stub(dialog, 'hideMobileDialog');
+            dialog.isMobile = false;
 
             dialog.element.classList.remove(constants.CLASS_HIDDEN);
         });
 
+        it('should only stop propogation on a desktop device', () => {
+            dialog.clickHandler(stubs.event);
+            expect(stubs.event.stopPropagation).to.be.called;
+            expect(stubs.event.preventDefault).to.not.be.called;
+        });
+
+        it('should stop propogation AND prevent default on a mobile device', () => {
+            dialog.isMobile = true;
+            dialog.clickHandler(stubs.event);
+            expect(stubs.event.stopPropagation).to.be.called;
+            expect(stubs.event.preventDefault).to.be.called;
+        });
+
         it('should post annotation when post annotation button is clicked', () => {
             stubs.findClosest.returns(constants.DATA_TYPE_POST);