Fix: Exiting annotations modes properly re-binds handlers

box · Nov 2, 2018 · 2c0b162 · 2c0b162
1 parent d339a26
commit 2c0b162
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 24 deletions.
diff --git a/src/controllers/AnnotationModeController.js b/src/controllers/AnnotationModeController.js
@@ -211,12 +211,19 @@ class AnnotationModeController extends EventEmitter {
         this.emit(CONTROLLER_EVENT.toggleMode);
     }
 
+    /**
+     * @return {void}
+     */
+    resetMode() {}
+
     /**
      * Disables the specified annotation mode
      *
      * @return {void}
      */
     exit(): void {
+        this.resetMode();
+
         if (this.currentThread) {
             this.currentThread.unmountPopover();
         }

diff --git a/src/controllers/DrawingModeController.js b/src/controllers/DrawingModeController.js
@@ -256,7 +256,7 @@ class DrawingModeController extends AnnotationModeController {
     }
 
     /** @inheritdoc */
-    exit() {
+    resetMode() {
         if (this.currentThread) {
             this.currentThread.clearBoundary();
         }
@@ -272,7 +272,6 @@ class DrawingModeController extends AnnotationModeController {
         pageElements.forEach((pageEl) => clearCanvas(pageEl, CLASS_ANNOTATION_LAYER_DRAW_IN_PROGRESS));
 
         this.annotatedElement.classList.remove(CLASS_ANNOTATION_DRAW_MODE);
-        super.exit();
     }
 
     /**

diff --git a/src/controllers/HighlightModeController.js b/src/controllers/HighlightModeController.js
@@ -34,7 +34,7 @@ class HighlightModeController extends AnnotationModeController {
     }
 
     /** @inheritdoc */
-    exit(): void {
+    resetMode(): void {
         this.destroyPendingThreads();
         window.getSelection().removeAllRanges();
         this.unbindListeners(); // Disable mode

diff --git a/src/controllers/PointModeController.js b/src/controllers/PointModeController.js
@@ -57,7 +57,7 @@ class PointModeController extends AnnotationModeController {
     }
 
     /** @inheritdoc */
-    exit(): void {
+    resetMode(): void {
         if (this.buttonEl) {
             this.buttonEl.classList.remove(CLASS_ACTIVE);
         }
@@ -70,7 +70,6 @@ class PointModeController extends AnnotationModeController {
         }
 
         this.pendingThreadID = null;
-        super.exit();
     }
 
     /** @inheritdoc */

diff --git a/src/controllers/__tests__/HighlightModeController-test.js b/src/controllers/__tests__/HighlightModeController-test.js
@@ -62,7 +62,7 @@ describe('controllers/HighlightModeController', () => {
         });
     });
 
-    describe('exit()', () => {
+    describe('resetMode()', () => {
         it('should exit annotation mode', () => {
             controller.destroyPendingThreads = jest.fn();
             controller.unbindListeners = jest.fn();
@@ -75,7 +75,7 @@ describe('controllers/HighlightModeController', () => {
             controller.annotatedElement = document.createElement('div');
             controller.annotatedElement.classList.add(CLASS_ANNOTATION_MODE);
 
-            controller.exit();
+            controller.resetMode();
             expect(controller.destroyPendingThreads).toBeCalled();
             expect(controller.emit).toBeCalledWith(CONTROLLER_EVENT.bindDOMListeners);
             expect(controller.unbindListeners).toBeCalled();

diff --git a/src/controllers/__tests__/PointModeController-test.js b/src/controllers/__tests__/PointModeController-test.js
@@ -90,11 +90,8 @@ describe('controllers/PointModeController', () => {
         });
     });
 
-    describe('exit()', () => {
+    describe('resetMode()', () => {
         beforeEach(() => {
-            controller.destroyPendingThreads = jest.fn();
-            controller.unbindListeners = jest.fn();
-
             // Set up annotation mode
             controller.annotatedElement = document.createElement('div');
             controller.annotatedElement.classList.add(CLASS_ANNOTATION_MODE);
@@ -104,19 +101,12 @@ describe('controllers/PointModeController', () => {
             controller.buttonEl.classList.add(CLASS_ACTIVE);
         });
 
-        it('should exit annotation mode', () => {
-            controller.exit();
-            expect(controller.destroyPendingThreads).toBeCalled();
-            expect(controller.emit).toBeCalledWith(CONTROLLER_EVENT.exit, expect.any(Object));
-            expect(controller.unbindListeners).toBeCalled();
-            expect(controller.hadPendingThreads).toBeFalsy();
-        });
-
-        it('should deactive mode button if available', () => {
+        it('should reset annotation mode', () => {
             controller.buttonEl = document.createElement('button');
             controller.buttonEl.classList.add(CLASS_ACTIVE);
-            controller.exit();
+            controller.resetMode();
             expect(controller.buttonEl.classList).not.toContain(CLASS_ACTIVE);
+            expect(controller.hadPendingThreads).toBeFalsy();
         });
     });
 

diff --git a/src/doc/DocAnnotator.js b/src/doc/DocAnnotator.js
@@ -492,9 +492,6 @@ class DocAnnotator extends Annotator {
         const location = this.getLocationFromEvent(this.lastHighlightEvent, highlightType);
         const controller = this.modeControllers[highlightType];
 
-        this.highlighter.removeAllHighlights();
-        this.resetHighlightSelection(this.lastHighlightEvent);
-
         if (!location || !controller) {
             return null;
         }