diff --git a/src/controllers/AnnotationModeController.js b/src/controllers/AnnotationModeController.js index 17acbc047..ff13da1cf 100644 --- a/src/controllers/AnnotationModeController.js +++ b/src/controllers/AnnotationModeController.js @@ -599,6 +599,17 @@ class AnnotationModeController extends EventEmitter { Object.keys(this.annotations).forEach((pageNum) => this.renderPage(pageNum)); } + /** + * Unregisters and destroys the specified thread + * + * @param {AnnotationThread} thread thread to destroy + * @return {void} + */ + destroyThread(thread: AnnotationThread) { + this.unregisterThread(thread); + thread.destroy(); + } + /** * Renders annotations from memory for a specified page. * @@ -614,8 +625,7 @@ class AnnotationModeController extends EventEmitter { pageThreads.forEach((thread, index) => { // Destroy any pending threads that may exist on re-render if (thread.state === STATES.pending || thread.id === this.pendingThreadID) { - this.unregisterThread(thread); - thread.destroy(); + this.destroyThread(thread); return; } @@ -645,10 +655,9 @@ class AnnotationModeController extends EventEmitter { const pageThreads = this.annotations[pageNum].all() || []; pageThreads.forEach((thread) => { if (thread.state === STATES.pending || thread.id === this.pendingThreadID) { - this.unregisterThread(thread); + this.destroyThread(thread); hadPendingThreads = true; this.pendingThreadID = null; - thread.destroy(); } }); }); diff --git a/src/controllers/DrawingModeController.js b/src/controllers/DrawingModeController.js index 4f88e858e..e56a000b6 100644 --- a/src/controllers/DrawingModeController.js +++ b/src/controllers/DrawingModeController.js @@ -93,7 +93,7 @@ class DrawingModeController extends AnnotationModeController { */ cancelDrawing(): void { if (this.currentThread) { - this.currentThread.destroy(); + this.destroyThread(this.currentThread); } this.exit(); @@ -108,7 +108,8 @@ class DrawingModeController extends AnnotationModeController { if ( this.currentThread && this.currentThread.state === STATES.pending && - this.currentThread.pathContainer.length > 0 + this.currentThread.pathContainer && + this.currentThread.pathContainer.undoStack.length > 0 ) { this.currentThread.save(TYPES.draw); } @@ -305,8 +306,7 @@ class DrawingModeController extends AnnotationModeController { this.unbindListeners(); this.bindListeners(); } else { - this.unregisterThread(thread); - thread.destroy(); + this.destroyThread(thread); this.renderPage(thread.location.page); } diff --git a/src/controllers/PointModeController.js b/src/controllers/PointModeController.js index e7aaf8449..7e16ff651 100644 --- a/src/controllers/PointModeController.js +++ b/src/controllers/PointModeController.js @@ -65,8 +65,7 @@ class PointModeController extends AnnotationModeController { const thread = this.getThreadByID(this.pendingThreadID); if (thread) { - this.unregisterThread(thread); - thread.destroy(); + this.destroyThread(thread); } this.pendingThreadID = null; diff --git a/src/controllers/__tests__/DrawingModeController-test.js b/src/controllers/__tests__/DrawingModeController-test.js index 60b30542c..eefae8a53 100644 --- a/src/controllers/__tests__/DrawingModeController-test.js +++ b/src/controllers/__tests__/DrawingModeController-test.js @@ -87,6 +87,55 @@ describe('controllers/DrawingModeController', () => { }); }); + describe('cancelDrawing()', () => { + it('should exit drawing mode', () => { + controller.exit = jest.fn(); + controller.destroyThread = jest.fn(); + controller.currentThread = thread; + + controller.cancelDrawing(); + expect(controller.exit).toBeCalled(); + expect(controller.destroyThread).toBeCalled(); + }); + }); + + describe('postDrawing()', () => { + beforeEach(() => { + controller.exit = jest.fn(); + thread.state = STATES.pending; + }); + + it('should exit drawing mode', () => { + controller.postDrawing(); + expect(controller.exit).toBeCalled(); + expect(thread.save).not.toBeCalled(); + }); + + it('should not save non-pending threads', () => { + thread.state = STATES.inactive; + controller.currentThread = thread; + controller.postDrawing(); + expect(controller.exit).toBeCalled(); + expect(thread.save).not.toBeCalled(); + }); + + it('should not save pending threads without paths', () => { + thread.pathContainer = { undoStack: [] }; + controller.currentThread = thread; + controller.postDrawing(); + expect(controller.exit).toBeCalled(); + expect(thread.save).not.toBeCalled(); + }); + + it('should save the current pending thread', () => { + thread.pathContainer = { undoStack: [{}] }; + controller.currentThread = thread; + controller.postDrawing(); + expect(controller.exit).toBeCalled(); + expect(thread.save).toBeCalled(); + }); + }); + describe('setupHandlers()', () => { it('should successfully contain draw mode handlers if undo and redo buttons exist', () => { controller.annotatedElement = {}; diff --git a/src/controllers/__tests__/PointModeController-test.js b/src/controllers/__tests__/PointModeController-test.js index 3fc8a6e0f..e656677f2 100644 --- a/src/controllers/__tests__/PointModeController-test.js +++ b/src/controllers/__tests__/PointModeController-test.js @@ -91,7 +91,7 @@ describe('controllers/PointModeController', () => { }); describe('resetMode()', () => { - beforeEach(() => { + it('should reset annotation mode', () => { // Set up annotation mode controller.annotatedElement = document.createElement('div'); controller.annotatedElement.classList.add(CLASS_ANNOTATION_MODE); @@ -99,14 +99,15 @@ describe('controllers/PointModeController', () => { controller.buttonEl = document.createElement('button'); controller.buttonEl.classList.add(CLASS_ACTIVE); - }); + controller.getThreadByID = jest.fn().mockReturnValue(thread); + controller.destroyThread = jest.fn(); - it('should reset annotation mode', () => { controller.buttonEl = document.createElement('button'); controller.buttonEl.classList.add(CLASS_ACTIVE); controller.resetMode(); expect(controller.buttonEl.classList).not.toContain(CLASS_ACTIVE); expect(controller.hadPendingThreads).toBeFalsy(); + expect(controller.destroyThread).toBeCalledWith(thread); }); });