chore: address PR comments, add unit tests

box · Apr 17, 2020 · 7f35c61 · 7f35c61
1 parent b382ed5
commit 7f35c61
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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');
+        });
+    });
 });