Fix: Clear highlight selection on mousedown and hideAnnotations() (#293)

box · Nov 20, 2018 · b60bc56 · b60bc56
1 parent 4abd611
commit b60bc56
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 0 deletions.
diff --git a/src/doc/DocAnnotator.js b/src/doc/DocAnnotator.js
@@ -653,6 +653,7 @@ class DocAnnotator extends Annotator {
         }
 
         this.isCreatingHighlight = true;
+        this.resetHighlightSelection(this.mouseDownEvent);
 
         if (this.plainHighlightEnabled) {
             this.modeControllers[TYPES.highlight].destroyPendingThreads();
@@ -801,6 +802,16 @@ class DocAnnotator extends Annotator {
         return false;
     }
 
+    /** @inheritdoc */
+    hideAnnotations(event: ?Event) {
+        if (event && util.isInDialog(event, this.container)) {
+            return;
+        }
+
+        this.resetHighlightSelection(event);
+        super.hideAnnotations();
+    }
+
     /**
      * Delegates click event to click handlers for threads on the page.
      *

diff --git a/src/doc/__tests__/DocAnnotator-test.js b/src/doc/__tests__/DocAnnotator-test.js
@@ -733,16 +733,27 @@ describe('doc/DocAnnotator', () => {
             annotator.modeControllers = {
                 highlight: controller
             };
+            annotator.resetHighlightSelection = jest.fn();
+            util.isInAnnotationOrMarker = jest.fn().mockReturnValue(false);
+        });
+
+        it('should do nothing if event occured inside an annotation or marker', () => {
+            util.isInAnnotationOrMarker = jest.fn().mockReturnValue(true);
+            annotator.highlightMousedownHandler({ clientX: 1, clientY: 1 });
+            expect(annotator.mouseDownEvent).toBeNull();
+            expect(annotator.resetHighlightSelection).not.toBeCalled();
         });
 
         it('should do nothing if highlights are disabled', () => {
             annotator.highlightMousedownHandler({ clientX: 1, clientY: 1 });
+            expect(annotator.resetHighlightSelection).toBeCalled();
             expect(controller.destroyPendingThreads).not.toBeCalled();
         });
 
         it('should get highlights on page and call their onMouse down method', () => {
             annotator.plainHighlightEnabled = true;
             annotator.highlightMousedownHandler({ clientX: 1, clientY: 1 });
+            expect(annotator.resetHighlightSelection).toBeCalled();
             expect(controller.destroyPendingThreads).toBeCalled();
         });
     });
@@ -1026,6 +1037,23 @@ describe('doc/DocAnnotator', () => {
         });
     });
 
+    describe('hideAnnotations()', () => {
+        beforeEach(() => {
+            annotator.resetHighlightSelection = jest.fn();
+            util.isInDialog = jest.fn().mockReturnValue(false);
+        });
+
+        it('should reset the highlight selection', () => {
+            util.isInDialog = jest.fn().mockReturnValue(true);
+            annotator.hideAnnotations({});
+            expect(annotator.resetHighlightSelection).not.toBeCalled();
+        });
+        it('should reset the highlight selection', () => {
+            annotator.hideAnnotations({});
+            expect(annotator.resetHighlightSelection).toBeCalled();
+        });
+    });
+
     describe('clickThread()', () => {
         beforeEach(() => {
             thread.type = 'something';