From b382ed59b6938099f9f5ec3724ddee59d6a7d698 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Thu, 16 Apr 2020 16:21:04 -0700 Subject: [PATCH 1/3] feat(annotation): handle create event --- src/lib/AnnotationControls.ts | 8 ++++++++ src/lib/viewers/BaseViewer.js | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/lib/AnnotationControls.ts b/src/lib/AnnotationControls.ts index cc56cbb27..974999bc2 100644 --- a/src/lib/AnnotationControls.ts +++ b/src/lib/AnnotationControls.ts @@ -94,6 +94,14 @@ export default class AnnotationControls { */ private handleFullscreenExit = (): void => this.handleFullscreenChange(false); + /** + * Returns the current active annotation mode + * @return {string} Current active annotation mode + */ + public getActiveMode = (): AnnotationMode => { + return this.currentActiveControl; + }; + /** * Deactivate current control button */ diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 151e781a2..2ae587a22 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -149,6 +149,7 @@ class BaseViewer extends EventEmitter { this.mobileZoomChangeHandler = this.mobileZoomChangeHandler.bind(this); this.mobileZoomEndHandler = this.mobileZoomEndHandler.bind(this); this.handleAnnotatorEvents = this.handleAnnotatorEvents.bind(this); + this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this); this.handleFullscreenEnter = this.handleFullscreenEnter.bind(this); this.handleFullscreenExit = this.handleFullscreenExit.bind(this); this.createAnnotator = this.createAnnotator.bind(this); @@ -976,7 +977,7 @@ class BaseViewer extends EventEmitter { this.annotator.addListener('annotatorevent', this.handleAnnotatorEvents); if (this.options.showAnnotationsControls && this.annotationControls) { - this.annotator.addListener('annotationcreate', this.annotationControls.resetControls); + this.annotator.addListener('annotations_create', this.handleAnnotationCreateEvent); } } @@ -1100,6 +1101,18 @@ class BaseViewer extends EventEmitter { this.emit('annotatorevent', data); } + handleAnnotationCreateEvent({ annotation, meta: { status } = {} }) { + if (status !== 'pending') { + const activeMode = this.annotationControls.getActiveMode(); + this.annotator.toggleAnnotationMode(activeMode); + this.annotationControls.resetControls(); + } + + if (status === 'success') { + this.annotator.emit('annotations_active_set', annotation.id); + } + } + /** * Creates combined options to give to the annotator * From 7f35c6141b192a02d32deeae6d73ab20b58610bb Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 17 Apr 2020 14:10:28 -0700 Subject: [PATCH 2/3] chore: address PR comments, add unit tests --- src/lib/AnnotationControls.ts | 4 -- src/lib/__tests__/AnnotationControls-test.js | 7 +++ src/lib/viewers/BaseViewer.js | 11 ++--- src/lib/viewers/__tests__/BaseViewer-test.js | 50 ++++++++++++++++++-- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/lib/AnnotationControls.ts b/src/lib/AnnotationControls.ts index 974999bc2..3c7981e8c 100644 --- a/src/lib/AnnotationControls.ts +++ b/src/lib/AnnotationControls.ts @@ -94,10 +94,6 @@ export default class AnnotationControls { */ private handleFullscreenExit = (): void => this.handleFullscreenChange(false); - /** - * Returns the current active annotation mode - * @return {string} Current active annotation mode - */ public getActiveMode = (): AnnotationMode => { return this.currentActiveControl; }; diff --git a/src/lib/__tests__/AnnotationControls-test.js b/src/lib/__tests__/AnnotationControls-test.js index 3b596f4c8..6bf592ae6 100644 --- a/src/lib/__tests__/AnnotationControls-test.js +++ b/src/lib/__tests__/AnnotationControls-test.js @@ -156,4 +156,11 @@ describe('lib/AnnotationControls', () => { expect(stubs.updateRegionButton).to.be.called; }); }); + + describe('getActiveMode()', () => { + it('should return the current active mode', () => { + annotationControls.currentActiveControl = 'region'; + expect(annotationControls.getActiveMode()).to.equal('region'); + }); + }); }); diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 2ae587a22..a4c235ac1 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -1101,15 +1101,14 @@ class BaseViewer extends EventEmitter { this.emit('annotatorevent', data); } - handleAnnotationCreateEvent({ annotation, meta: { status } = {} }) { - if (status !== 'pending') { + handleAnnotationCreateEvent({ annotation: { id } = {}, meta: { status } = {} }) { + // Only on success do we exit create annotation mode. If error occurs, + // we remain in create mode + if (status === 'success') { const activeMode = this.annotationControls.getActiveMode(); this.annotator.toggleAnnotationMode(activeMode); this.annotationControls.resetControls(); - } - - if (status === 'success') { - this.annotator.emit('annotations_active_set', annotation.id); + this.annotator.emit('annotations_active_set', id); } } diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 423b48903..a5f31e6ff 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -1139,10 +1139,7 @@ describe('lib/viewers/BaseViewer', () => { expect(base.addListener).to.be.calledWith('toggleannotationmode', sinon.match.func); expect(base.addListener).to.be.calledWith('scale', sinon.match.func); expect(base.addListener).to.be.calledWith('scrolltoannotation', sinon.match.func); - expect(base.annotator.addListener).to.be.calledWith( - 'annotationcreate', - base.annotationControls.resetControls, - ); + expect(base.annotator.addListener).to.be.calledWith('annotations_create', base.handleAnnotationCreateEvent); expect(base.annotator.addListener).to.be.calledWith('annotatorevent', sinon.match.func); expect(base.emit).to.be.calledWith('annotator', base.annotator); }); @@ -1495,4 +1492,49 @@ describe('lib/viewers/BaseViewer', () => { expect(stubs.emit).not.to.be.called; }); }); + + describe('handleAnnotationCreateEvent()', () => { + beforeEach(() => { + base.annotationControls = { + getActiveMode: sandbox.stub(), + resetControls: sandbox.stub(), + }; + + base.annotator = { + toggleAnnotationMode: sandbox.stub(), + emit: sandbox.stub(), + }; + }); + + const createEvent = status => ({ + annotation: { id: '123' }, + meta: { + status, + }, + }); + + ['error', 'pending'].forEach(status => { + test(`should not do anything if status is ${status}`, () => { + const event = createEvent(status); + base.handleAnnotationCreateEvent(event); + + expect(base.annotationControls.getActiveMode).not.to.be.called; + expect(base.annotator.toggleAnnotationMode).not.to.be.called; + expect(base.annotationControls.resetControls).not.to.be.called; + expect(base.annotator.emit).not.to.be.called; + }); + }); + + test('should reset controls if status is success', () => { + base.annotationControls.getActiveMode.returns('region'); + + const event = createEvent('success'); + base.handleAnnotationCreateEvent(event); + + expect(base.annotationControls.getActiveMode).to.be.called; + expect(base.annotator.toggleAnnotationMode).to.be.calledWith('region'); + expect(base.annotationControls.resetControls).to.be.called; + expect(base.annotator.emit).to.be.calledWith('annotations_active_set', '123'); + }); + }); }); From 1500a352fca44e5d5495f1ac36aab072b426789c Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 17 Apr 2020 14:26:53 -0700 Subject: [PATCH 3/3] chore: fix unit test --- src/lib/viewers/__tests__/BaseViewer-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index a5f31e6ff..2e2f833d7 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -1514,7 +1514,7 @@ describe('lib/viewers/BaseViewer', () => { }); ['error', 'pending'].forEach(status => { - test(`should not do anything if status is ${status}`, () => { + it(`should not do anything if status is ${status}`, () => { const event = createEvent(status); base.handleAnnotationCreateEvent(event); @@ -1525,7 +1525,7 @@ describe('lib/viewers/BaseViewer', () => { }); }); - test('should reset controls if status is success', () => { + it('should reset controls if status is success', () => { base.annotationControls.getActiveMode.returns('region'); const event = createEvent('success');