From fd477c7ac4335f9cc643ada5c4f2a7fa40d72445 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Thu, 17 Aug 2017 14:25:20 -0700 Subject: [PATCH] Fix: Ensure that image.handleMouseDown() is unbound in annotation mode (#319) --- src/lib/annotations/Annotator.js | 6 +- .../annotations/__tests__/Annotator-test.js | 68 +++++++++++++++++++ src/lib/viewers/BaseViewer.js | 5 +- src/lib/viewers/__tests__/BaseViewer-test.js | 37 +++++++++- src/lib/viewers/image/ImageBaseViewer.js | 4 +- .../image/__tests__/ImageBaseViewer-test.js | 38 +++++++++++ 6 files changed, 153 insertions(+), 5 deletions(-) diff --git a/src/lib/annotations/Annotator.js b/src/lib/annotations/Annotator.js index a67f4ca1f..de570d4a8 100644 --- a/src/lib/annotations/Annotator.js +++ b/src/lib/annotations/Annotator.js @@ -375,7 +375,10 @@ class Annotator extends EventEmitter { * @return {void} */ disableAnnotationMode(mode, buttonEl) { - this.emit(MODE_EXIT); + if (this.isInAnnotationMode(mode)) { + this.emit(MODE_EXIT); + } + this.annotatedElement.classList.remove(CLASS_ANNOTATION_MODE); if (buttonEl) { buttonEl.classList.remove(CLASS_ACTIVE); @@ -732,6 +735,7 @@ class Annotator extends EventEmitter { pointClickHandler(event) { event.stopPropagation(); event.preventDefault(); + this.emit(MODE_EXIT); // Determine if a point annotation dialog is already open and close the // current open dialog diff --git a/src/lib/annotations/__tests__/Annotator-test.js b/src/lib/annotations/__tests__/Annotator-test.js index 1b9ca3fc7..d9208fd83 100644 --- a/src/lib/annotations/__tests__/Annotator-test.js +++ b/src/lib/annotations/__tests__/Annotator-test.js @@ -7,6 +7,8 @@ import { STATES, TYPES, CLASS_ANNOTATION_DRAW_MODE, + CLASS_ANNOTATION_MODE, + CLASS_ACTIVE, CLASS_HIDDEN, SELECTOR_BOX_PREVIEW_BTN_ANNOTATE_POINT } from '../annotationConstants'; @@ -337,6 +339,72 @@ describe('lib/annotations/Annotator', () => { }); }); + describe('disableAnnotationMode()', () => { + beforeEach(() => { + stubs.isInMode = sandbox.stub(annotator, 'isInAnnotationMode').returns(false); + stubs.emit = sandbox.stub(annotator, 'emit'); + stubs.unbindMode = sandbox.stub(annotator, 'unbindModeListeners'); + stubs.bindDOM = sandbox.stub(annotator, 'bindDOMListeners'); + stubs.hide = sandbox.stub(annotatorUtil, 'hideElement'); + stubs.show = sandbox.stub(annotatorUtil, 'showElement'); + }); + + it('should exit annotation mode if currently in the specified mode', () => { + stubs.isInMode.returns(true); + annotator.disableAnnotationMode(TYPES.point); + expect(stubs.emit).to.be.calledWith(MODE_EXIT); + expect(stubs.unbindMode).to.be.calledWith(TYPES.point); + expect(stubs.bindDOM).to.be.called; + expect(annotator.annotatedElement).to.not.have.class(CLASS_ANNOTATION_MODE); + }); + + it('should deactivate point annotation mode button', () => { + const btn = document.querySelector('.bp-btn-annotate'); + annotator.disableAnnotationMode(TYPES.point, btn); + expect(btn).to.not.have.class(CLASS_ACTIVE); + }); + + it('should deactivate draw annotation mode button', () => { + const btn = document.querySelector('.bp-btn-annotate'); + annotator.disableAnnotationMode(TYPES.draw, btn); + expect(btn).to.not.have.class(CLASS_ACTIVE); + expect(stubs.show).to.be.called; + expect(stubs.hide).to.be.calledTwice; + }); + }); + + describe('enableAnnotationMode()', () => { + beforeEach(() => { + stubs.emit = sandbox.stub(annotator, 'emit'); + stubs.unbindDOM = sandbox.stub(annotator, 'unbindDOMListeners'); + stubs.bindMode = sandbox.stub(annotator, 'bindModeListeners'); + stubs.hide = sandbox.stub(annotatorUtil, 'hideElement'); + stubs.show = sandbox.stub(annotatorUtil, 'showElement'); + }); + + it('should enter annotation mode', () => { + annotator.enableAnnotationMode(TYPES.point); + expect(stubs.emit).to.be.calledWith(MODE_ENTER, TYPES.point); + expect(stubs.unbindDOM).to.be.called; + expect(stubs.bindMode).to.be.calledWith(TYPES.point); + expect(annotator.annotatedElement).to.have.class(CLASS_ANNOTATION_MODE); + }); + + it('should deactivate point annotation mode button', () => { + const btn = document.querySelector('.bp-btn-annotate'); + annotator.enableAnnotationMode(TYPES.point, btn); + expect(btn).to.have.class(CLASS_ACTIVE); + }); + + it('should deactivate draw annotation mode button', () => { + const btn = document.querySelector('.bp-btn-annotate'); + annotator.enableAnnotationMode(TYPES.draw, btn); + expect(btn).to.have.class(CLASS_ACTIVE); + expect(stubs.hide).to.be.called; + expect(stubs.show).to.be.calledTwice; + }); + }); + describe('fetchAnnotations()', () => { beforeEach(() => { annotator.annotationService = { diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 0e1489ec2..2d1f3320d 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -727,7 +727,10 @@ class BaseViewer extends EventEmitter { } break; case ANNOTATION_MODE_EXIT: - this.enableViewerControls(); + if (!this.annotator.isInAnnotationMode(ANNOTATION_TYPE_POINT)) { + this.enableViewerControls(); + } + this.emit('notificationhide'); break; case 'annotationerror': diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 2be625032..a34fdb17b 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -702,6 +702,26 @@ describe('lib/viewers/BaseViewer', () => { }); }); + describe('disableViewerControls()', () => { + it('should disable viewer controls', () => { + base.controls = { + disable: sandbox.stub() + }; + base.disableViewerControls(); + expect(base.controls.disable).to.be.called; + }); + }); + + describe('enableViewerControls()', () => { + it('should enable viewer controls', () => { + base.controls = { + enable: sandbox.stub() + }; + base.enableViewerControls(); + expect(base.controls.enable).to.be.called; + }); + }); + describe('loadAnnotator()', () => { beforeEach(() => { base.options.viewer = { @@ -811,6 +831,10 @@ describe('lib/viewers/BaseViewer', () => { beforeEach(() => { sandbox.stub(base, 'emit'); + base.annotator = { + isInAnnotationMode: sandbox.stub() + }; + stubs.isInAnnotationMode = base.annotator.isInAnnotationMode; }); it('should disable controls and show point mode notification on annotationmodeenter', () => { @@ -833,8 +857,19 @@ describe('lib/viewers/BaseViewer', () => { expect(base.emit).to.be.calledWith('notificationshow', sinon.match.string); }); - it('should enable controls and hide notification on annotationmodeexit', () => { + it('should hide notification on annotationmodeexit', () => { + sandbox.stub(base, 'enableViewerControls'); + stubs.isInAnnotationMode.returns(true); + base.handleAnnotatorNotifications({ + event: ANNOTATION_MODE_EXIT + }); + expect(base.enableViewerControls).to.not.be.called; + expect(base.emit).to.be.calledWith('notificationhide'); + }); + + it('should enable controls if not in point annotation mode on annotationmodeexit', () => { sandbox.stub(base, 'enableViewerControls'); + stubs.isInAnnotationMode.returns(false); base.handleAnnotatorNotifications({ event: ANNOTATION_MODE_EXIT }); diff --git a/src/lib/viewers/image/ImageBaseViewer.js b/src/lib/viewers/image/ImageBaseViewer.js index 13632bb54..fa78a64b8 100644 --- a/src/lib/viewers/image/ImageBaseViewer.js +++ b/src/lib/viewers/image/ImageBaseViewer.js @@ -335,8 +335,7 @@ class ImageBaseViewer extends BaseViewer { */ disableViewerControls() { super.disableViewerControls(); - this.imageEl.classList.remove(CSS_CLASS_ZOOMABLE); - this.imageEl.classList.remove(CSS_CLASS_PANNABLE); + this.unbindDOMListeners(); } /** @@ -347,6 +346,7 @@ class ImageBaseViewer extends BaseViewer { */ enableViewerControls() { super.enableViewerControls(); + this.bindDOMListeners(); if (!this.isMobile) { this.updateCursor(); diff --git a/src/lib/viewers/image/__tests__/ImageBaseViewer-test.js b/src/lib/viewers/image/__tests__/ImageBaseViewer-test.js index 65968a11e..a1199ccfb 100644 --- a/src/lib/viewers/image/__tests__/ImageBaseViewer-test.js +++ b/src/lib/viewers/image/__tests__/ImageBaseViewer-test.js @@ -522,4 +522,42 @@ describe('lib/viewers/image/ImageBaseViewer', () => { expect(stubs.loadUI).to.have.been.called; }); }); + + describe('disableViewerControls()', () => { + it('should disable viewer controls', () => { + imageBase.controls = { + disable: sandbox.stub() + }; + sandbox.stub(imageBase, 'unbindDOMListeners'); + imageBase.disableViewerControls(); + expect(imageBase.controls.disable).to.be.called; + expect(imageBase.unbindDOMListeners).to.be.called; + }); + }); + + describe('enableViewerControls()', () => { + it('should enable viewer controls', () => { + imageBase.controls = { + enable: sandbox.stub() + }; + imageBase.isMobile = true; + sandbox.stub(imageBase, 'bindDOMListeners'); + sandbox.stub(imageBase, 'updateCursor'); + imageBase.enableViewerControls(); + expect(imageBase.controls.enable).to.be.called; + expect(imageBase.bindDOMListeners).to.be.called; + expect(imageBase.updateCursor).to.not.be.called; + }); + + it('should update cursor if not on mobile', () => { + imageBase.controls = { + enable: sandbox.stub() + }; + imageBase.isMobile = false; + sandbox.stub(imageBase, 'bindDOMListeners'); + sandbox.stub(imageBase, 'updateCursor'); + imageBase.enableViewerControls(); + expect(imageBase.updateCursor).to.be.called; + }); + }); });