Skip to content

Commit

Permalink
Fix: Ensure that image.handleMouseDown() is unbound in annotation mode (
Browse files Browse the repository at this point in the history
  • Loading branch information
pramodsum committed Aug 17, 2017
1 parent 783adbc commit fd477c7
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 5 deletions.
6 changes: 5 additions & 1 deletion src/lib/annotations/Annotator.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions src/lib/annotations/__tests__/Annotator-test.js
Expand Up @@ -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';
Expand Down Expand Up @@ -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 = {
Expand Down
5 changes: 4 additions & 1 deletion src/lib/viewers/BaseViewer.js
Expand Up @@ -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':
Expand Down
37 changes: 36 additions & 1 deletion src/lib/viewers/__tests__/BaseViewer-test.js
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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
});
Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/image/ImageBaseViewer.js
Expand Up @@ -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();
}

/**
Expand All @@ -347,6 +346,7 @@ class ImageBaseViewer extends BaseViewer {
*/
enableViewerControls() {
super.enableViewerControls();
this.bindDOMListeners();

if (!this.isMobile) {
this.updateCursor();
Expand Down
38 changes: 38 additions & 0 deletions src/lib/viewers/image/__tests__/ImageBaseViewer-test.js
Expand Up @@ -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;
});
});
});

0 comments on commit fd477c7

Please sign in to comment.