Chore: Removing DrawingModeController dependency on DocDrawingThread

src/controllers/AnnotationModeController.js

@@ -277,6 +277,10 @@ class AnnotationModeController extends EventEmitter {
      */
     getThreadByID(threadID) {
         let thread = null;
+        if (!threadID) {
+            return thread;
+        }
+
         Object.keys(this.threads).some((pageNum) => {
             const pageThreads = this.threads[pageNum];
             if (!pageThreads) {

src/controllers/DrawingModeController.js

@@ -1,17 +1,10 @@
 import AnnotationModeController from './AnnotationModeController';
 import shell from './drawingShell.html';
-import DocDrawingThread from '../doc/DocDrawingThread';
-import {
-    replaceHeader,
-    enableElement,
-    disableElement,
-    eventToLocationHandler,
-    clearCanvas,
-    hasValidBoundaryCoordinates
-} from '../util';
+import { replaceHeader, enableElement, disableElement, clearCanvas, hasValidBoundaryCoordinates } from '../util';
 import {
     TYPES,
     STATES,
+    THREAD_EVENT,
     SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL,
     SELECTOR_ANNOTATION_BUTTON_DRAW_POST,
     SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO,
@@ -107,40 +100,18 @@ class DrawingModeController extends AnnotationModeController {
     /** @inheritdoc */
     setupHandlers() {
         /* eslint-disable require-jsdoc */
-        const locationFunction = (event) => this.annotator.getLocationFromEvent(event, TYPES.point);
+        this.locationFunction = (event) => this.annotator.getLocationFromEvent(event, TYPES.point);
         /* eslint-enable require-jsdoc */
 
-        // Setup
-        const threadParams = this.annotator.getThreadParams([], {}, TYPES.draw);
-        this.currentThread = new DocDrawingThread(threadParams);
-        this.currentThread.addListener('threadevent', (data) => {
-            const thread = this.currentThread || this.selectedThread;
-            this.handleThreadEvents(thread, data);
-        });
-
-        // Get handlers
-        this.pushElementHandler(
-            this.annotatedElement,
-            ['mousemove', 'touchmove'],
-            eventToLocationHandler(locationFunction, this.currentThread.handleMove)
-        );
-
-        this.pushElementHandler(
-            this.annotatedElement,
-            ['mousedown', 'touchstart'],
-            eventToLocationHandler(locationFunction, this.currentThread.handleStart)
-        );
-
-        this.pushElementHandler(
-            this.annotatedElement,
-            ['mouseup', 'touchcancel', 'touchend'],
-            eventToLocationHandler(locationFunction, this.currentThread.handleStop)
-        );
+        this.drawingStartHandler = this.drawingStartHandler.bind(this);
+        this.pushElementHandler(this.annotatedElement, ['mousedown', 'touchstart'], this.drawingStartHandler);
 
         this.pushElementHandler(this.cancelButtonEl, 'click', () => {
             if (this.currentThread) {
                 this.currentThread.cancelUnsavedAnnotation();
             }
+
+            this.currentThread = undefined;
             this.toggleMode();
         });
 
@@ -149,11 +120,61 @@ class DrawingModeController extends AnnotationModeController {
                 this.saveThread(this.currentThread);
             }
 
+            this.currentThread = undefined;
             this.toggleMode();
         });
 
-        this.pushElementHandler(this.undoButtonEl, 'click', this.currentThread.undo);
-        this.pushElementHandler(this.redoButtonEl, 'click', this.currentThread.redo);
+        this.pushElementHandler(this.undoButtonEl, 'click', () => {
+            if (this.currentThread) {
+                this.currentThread.undo();
+            }
+        });
+
+        this.pushElementHandler(this.redoButtonEl, 'click', () => {
+            if (this.currentThread) {
+                this.currentThread.redo();
+            }
+        });
+    }
+
+    /**
+     * Start a drawing stroke
+     *
+     * @param {Event} event - DOM event
+     * @return {void}
+     */
+    drawingStartHandler(event) {
+        event.stopPropagation();
+        event.preventDefault();
+
+        // Get annotation location from click event, ignore click if location is invalid
+        const location = this.annotator.getLocationFromEvent(event, TYPES.point);
+        if (!location) {
+            return;
+        }
+
+        if (this.currentThread) {
+            this.currentThread.handleStart(location);
+            return;
+        }
+
+        // Add initial drawing boundary based on starting location
+        location.minX = location.x;
+        location.maxX = location.x;
+        location.minY = location.y;
+        location.maxY = location.y;
+
+        // Create new thread with no annotations, show indicator, and show dialog
+        const thread = this.annotator.createAnnotationThread([], location, TYPES.draw);
+        if (!thread) {
+            return;
+        }
+
+        this.currentThread = thread;
+        this.emit(THREAD_EVENT.pending, thread.getThreadEventData());
+        thread.bindDrawingListeners(this.locationFunction);
+        thread.addListener('threadevent', (data) => this.handleThreadEvents(thread, data));
+        thread.handleStart(location);
     }
 
     /**
@@ -171,6 +192,9 @@ class DrawingModeController extends AnnotationModeController {
         const { eventData } = data;
         switch (data.event) {
             case 'softcommit':
+                thread.removeListener('threadevent', this.handleThreadEvents);
+                thread.unbindDrawingListeners();
+
                 this.currentThread = undefined;
                 this.saveThread(thread);
                 this.unbindListeners();
@@ -187,6 +211,10 @@ class DrawingModeController extends AnnotationModeController {
                     return;
                 }
 
+                this.currentThread = undefined;
+                thread.removeListener('threadevent', this.handleThreadEvents);
+                thread.unbindDrawingListeners();
+
                 if (thread.state === STATES.pending) {
                     // Soft delete, in-progress thread doesn't require a redraw or a delete on the server
                     // Clear in-progress thread and restart drawing

src/controllers/PointModeController.js

@@ -16,12 +16,6 @@ class PointModeController extends AnnotationModeController {
     /** @property {HTMLElement} - The button to exit point annotation mode */
     exitButtonEl;
 
-    /** @property {HTMLElement} - The button to cancel the pending thread */
-    cancelButtonEl;
-
-    /** @property {HTMLElement} - The button to commit the pending thread */
-    postButtonEl;
-
     /** @inheritdoc */
     init(data) {
         super.init(data);
@@ -123,23 +117,7 @@ class PointModeController extends AnnotationModeController {
         // Get handlers
         this.pushElementHandler(this.annotatedElement, ['mousedown', 'touchstart'], this.pointClickHandler);
 
-        this.pushElementHandler(this.exitButtonEl, 'click', () => {
-            if (this.currentThread) {
-                this.currentThread.cancelUnsavedAnnotation();
-            }
-
-            this.toggleMode();
-        });
-
-        this.pushElementHandler(this.cancelButtonEl, 'click', () => {
-            this.currentThread.cancelUnsavedAnnotation();
-            this.emit(CONTROLLER_EVENT.toggleMode);
-        });
-
-        this.pushElementHandler(this.postButtonEl, 'click', () => {
-            this.currentThread.saveAnnotation(TYPES.point);
-            this.emit(CONTROLLER_EVENT.toggleMode);
-        });
+        this.pushElementHandler(this.exitButtonEl, 'click', this.toggleMode);
     }
 
     /** @inheritdoc */

src/controllers/__tests__/DrawingModeController-test.js

@@ -12,7 +12,8 @@ import {
     SELECTOR_ANNOTATION_BUTTON_DRAW_REDO,
     SELECTOR_DRAW_MODE_HEADER,
     CLASS_ANNOTATION_MODE,
-    CLASS_ACTIVE
+    CLASS_ACTIVE,
+    THREAD_EVENT
 } from '../../constants';
 
 let controller;
@@ -39,6 +40,9 @@ describe('controllers/DrawingModeController', () => {
             deleteThread: () => {},
             clearBoundary: () => {},
             drawBoundary: () => {},
+            bindDrawingListeners: () => {},
+            unbindDrawingListeners: () => {},
+            getThreadEventData: () => {},
             show: () => {}
         };
         stubs.threadMock = sandbox.mock(stubs.thread);
@@ -186,22 +190,63 @@ describe('controllers/DrawingModeController', () => {
 
     describe('setupHandlers()', () => {
         it('should successfully contain draw mode handlers if undo and redo buttons exist', () => {
-            controller.annotator = {
-                getThreadParams: sandbox.stub(),
-                getLocationFromEvent: sandbox.stub()
-            };
             controller.annotatedElement = {};
-            stubs.getParams = controller.annotator.getThreadParams.returns({});
-            stubs.getLocation = controller.annotator.getLocationFromEvent;
-
             controller.postButtonEl = 'not undefined';
             controller.undoButtonEl = 'also not undefined';
             controller.redoButtonEl = 'additionally not undefined';
             controller.cancelButtonEl = 'definitely not undefined';
 
             controller.setupHandlers();
-            expect(stubs.getParams).to.be.called;
-            expect(controller.handlers.length).to.equal(7);
+            expect(controller.handlers.length).to.equal(5);
+        });
+    });
+
+    describe('drawingStartHandler()', () => {
+        const event = {
+            stopPropagation: () => {},
+            preventDefault: () => {}
+        };
+        const eventMock = sandbox.mock(event);
+        const location = {};
+
+        beforeEach(() => {
+            controller.annotator = {
+                getLocationFromEvent: () => {},
+                createAnnotationThread: () => {}
+            };
+            stubs.annotatorMock = sandbox.mock(controller.annotator);
+            controller.currentThread = undefined;
+            controller.locationFunction = sandbox.stub();
+
+            eventMock.expects('stopPropagation');
+            eventMock.expects('preventDefault');
+        });
+
+        it('should do nothing if drawing start is invalid', () => {
+            stubs.annotatorMock.expects('getLocationFromEvent');
+            stubs.annotatorMock.expects('createAnnotationThread').never();
+            controller.drawingStartHandler(event);
+        });
+
+        it('should continue drawing if in the middle of creating a new drawing', () => {
+            controller.currentThread = stubs.thread;
+            stubs.threadMock.expects('handleStart').withArgs(location);
+            stubs.annotatorMock.expects('getLocationFromEvent').returns(location);
+            stubs.annotatorMock.expects('createAnnotationThread').never();
+            controller.drawingStartHandler(event);
+        });
+
+        it('should begin a new drawing thread if none exist already', () => {
+            stubs.annotatorMock.expects('getLocationFromEvent').returns(location);
+            stubs.annotatorMock.expects('createAnnotationThread').returns(stubs.thread);
+            stubs.threadMock.expects('bindDrawingListeners').withArgs(controller.locationFunction);
+            stubs.threadMock.expects('addListener').withArgs('threadevent', sinon.match.func);
+            stubs.threadMock.expects('handleStart').withArgs(location);
+            stubs.threadMock.expects('getThreadEventData').returns({});
+
+            controller.drawingStartHandler(event);
+            expect(controller.currentThread).to.not.be.undefined;
+            expect(controller.emit).to.be.calledWith(THREAD_EVENT.pending, {});
         });
     });
 
@@ -234,12 +279,15 @@ describe('controllers/DrawingModeController', () => {
 
         it('should save thread on softcommit', () => {
             stubs.threadMock.expects('handleStart').never();
+            stubs.threadMock.expects('removeListener').withArgs('threadevent', sinon.match.func);
+            stubs.threadMock.expects('unbindDrawingListeners');
             controller.handleThreadEvents(stubs.thread, {
                 event: 'softcommit'
             });
             expect(controller.unbindListeners).to.be.called;
             expect(controller.bindListeners).to.be.called;
             expect(controller.saveThread).to.be.called;
+            expect(controller.currentThread).to.be.undefined;
         });
 
         it('should start a new thread on pagechanged', () => {
@@ -252,7 +300,9 @@ describe('controllers/DrawingModeController', () => {
                     page: 1
                 },
                 info: 'I am a thread',
-                saveAnnotation: sandbox.stub()
+                saveAnnotation: sandbox.stub(),
+                removeListener: sandbox.stub(),
+                unbindDrawingListeners: sandbox.stub()
             };
             const thread2 = {
                 minX: 10,
@@ -280,27 +330,20 @@ describe('controllers/DrawingModeController', () => {
             expect(controller.unbindListeners).to.be.called;
             expect(controller.bindListeners).to.be.called;
             expect(thread2.handleStart).to.be.calledWith(data.eventData.location);
-        });
-
-        it('should update undo and redo buttons on availableactions', () => {
-            controller.handleThreadEvents(stubs.thread, {
-                event: 'availableactions',
-                eventData: {
-                    undo: 1,
-                    redo: 2
-                }
-            });
-            expect(controller.updateUndoRedoButtonEls).to.be.calledWith(1, 2);
+            expect(controller.currentThread).to.not.be.undefined;
         });
 
         it('should soft delete a pending thread and restart mode listeners', () => {
             stubs.thread.state = 'pending';
             stubs.threadMock.expects('destroy');
+            stubs.threadMock.expects('removeListener').withArgs('threadevent', sinon.match.func);
+            stubs.threadMock.expects('unbindDrawingListeners');
             controller.handleThreadEvents(stubs.thread, {
                 event: 'dialogdelete'
             });
             expect(controller.unbindListeners).to.be.called;
             expect(controller.bindListeners).to.be.called;
+            expect(controller.currentThread).to.be.undefined;
         });
 
         it('should delete a non-pending thread', () => {
@@ -311,10 +354,13 @@ describe('controllers/DrawingModeController', () => {
             const unregisterThreadStub = sandbox.stub(controller, 'unregisterThread');
 
             stubs.threadMock.expects('deleteThread');
+            stubs.threadMock.expects('removeListener').withArgs('threadevent', sinon.match.func);
+            stubs.threadMock.expects('unbindDrawingListeners');
             controller.handleThreadEvents(stubs.thread, {
                 event: 'dialogdelete'
             });
             expect(unregisterThreadStub).to.be.called;
+            expect(controller.currentThread).to.be.undefined;
         });
 
         it('should not delete a thread if the dialog no longer exists', () => {
@@ -323,6 +369,8 @@ describe('controllers/DrawingModeController', () => {
             controller.threads[1] = new rbush();
             controller.registerThread(stubs.thread);
             const unregisterThreadStub = sandbox.stub(controller, 'unregisterThread');
+            stubs.threadMock.expects('removeListener').never();
+            stubs.threadMock.expects('unbindDrawingListeners').never();
 
             controller.handleThreadEvents(stubs.thread, {
                 event: 'dialogdelete'

src/controllers/__tests__/PointModeController-test.js

@@ -129,12 +129,10 @@ describe('controllers/PointModeController', () => {
 
     describe('setupHandlers()', () => {
         it('should successfully contain mode handlers', () => {
-            controller.postButtonEl = 'not undefined';
-            controller.cancelButtonEl = 'definitely not undefined';
             controller.exitButtonEl = 'also definitely not undefined';
 
             controller.setupHandlers();
-            expect(controller.handlers.length).to.equal(4);
+            expect(controller.handlers.length).to.equal(2);
         });
     });
 

src/doc/DocDrawingThread.js

@@ -78,8 +78,8 @@ class DocDrawingThread extends DrawingThread {
                 page: location.page,
                 dimensions: location.dimensions
             };
-            this.checkAndHandleScaleUpdate();
         }
+        this.checkAndHandleScaleUpdate();
 
         this.drawingFlag = DRAW_STATES.drawing;
         if (!this.pendingPath) {