Fix: Don't trigger highlight if a new highlight is being created (#265)

box · Jul 31, 2017 · 9ab6db4 · 9ab6db4
1 parent 4de1259
commit 9ab6db4
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 1 deletion.
diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js
@@ -328,6 +328,7 @@ class DocAnnotator extends Annotator {
             return null;
         }
         this.createHighlightDialog.hide();
+        this.isCreatingHighlight = false;
 
         const location = this.getLocationFromEvent(this.lastHighlightEvent, TYPES.highlight);
         if (!location) {
@@ -576,6 +577,12 @@ class DocAnnotator extends Annotator {
             return;
         }
 
+        // Determine if user is in the middle of creating a highlight
+        // annotation and ignore hover events of any highlights below
+        if (this.isCreatingHighlight) {
+            return;
+        }
+
         // Determine if mouse is over any highlight dialog
         // and ignore hover events of any highlights below
         const event = this.mouseMoveEvent;
@@ -669,6 +676,8 @@ class DocAnnotator extends Annotator {
             this.highlighter.removeAllHighlights();
         }
         this.createHighlightDialog.hide();
+        this.isCreatingHighlight = false;
+
         // Creating highlights is disabled on mobile for now since the
         // event we would listen to, selectionchange, fires continuously and
         // is unreliable. If the mouse moved or we double clicked text,
@@ -678,7 +687,6 @@ class DocAnnotator extends Annotator {
         } else {
             this.highlightClickHandler(event);
         }
-        this.isCreatingHighlight = false;
     }
 
     /**
@@ -724,6 +732,7 @@ class DocAnnotator extends Annotator {
         if (!this.isMobile) {
             this.createHighlightDialog.setPosition(right - pageLeft, bottom - pageTop);
         }
+        this.isCreatingHighlight = true;
 
         this.lastHighlightEvent = event;
     }

diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js
@@ -694,18 +694,32 @@ describe('lib/annotations/doc/DocAnnotator', () => {
             stubs.getPageInfo = stubs.getPageInfo.returns({ pageEl: {}, page: 1 });
             stubs.getThreads = sandbox.stub(annotator, 'getHighlightThreadsOnPage');
             stubs.clock = sinon.useFakeTimers();
+            stubs.isDialog = sandbox.stub(docAnnotatorUtil, 'isDialogDataType');
 
             let timer = 0;
             window.performance = window.performance || { now: () => {} };
             sandbox.stub(window.performance, 'now', () => {
                 return (timer += 500);
             });
+
+            annotator.isCreatingHighlight = false;
         });
 
         afterEach(() => {
             stubs.clock.restore();
         });
 
+        it('should not do anything if user is creating a highlight', () => {
+            stubs.threadMock.expects('onMousemove').returns(false).never();
+            stubs.delayMock.expects('onMousemove').returns(true).never();
+            stubs.getThreads.returns([stubs.thread, stubs.delayThread]);
+            annotator.isCreatingHighlight = true;
+
+            annotator.mouseMoveEvent = { clientX: 3, clientY: 3 };
+            annotator.onHighlightCheck();
+            expect(stubs.isDialog).to.not.be.called;
+        });
+
         it('should not add any delayThreads if there are no threads on the current page', () => {
             stubs.threadMock.expects('onMousemove').returns(false).never();
             stubs.delayMock.expects('onMousemove').returns(true).never();