Fix: Ensuring annotations are loaded when viewer has permissions (#90)

- This is for the case when the viewer has annotation permissions but ?showAnnotations=false
box · Apr 25, 2017 · b90cdde · b90cdde
1 parent efa8b2a
commit b90cdde
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 14 deletions.
diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js
@@ -74,11 +74,10 @@ class BaseViewer extends EventEmitter {
         // the assets are available, the showAnnotations flag is true, and the
         // expiring embed is not a shared link
         // TODO(@spramod): Determine the expected behavior on shared links
-        const { showAnnotations, sharedLink } = this.options;
-        if (showAnnotations && !sharedLink) {
+        if (this.isViewerAnnotatable() && !this.options.sharedLink) {
             this.loadAssets(ANNOTATIONS_JS)
                 .then(() => {
-                    this.annotationsLoaded = true;
+                    this.boxAnnotationsLoaded = true;
                 })
                 .catch(this.handleAssetError);
         }
@@ -275,7 +274,7 @@ class BaseViewer extends EventEmitter {
         });
 
         this.addListener('load', () => {
-            if (this.annotationsLoaded && this.options.showAnnotations) {
+            if (this.boxAnnotationsLoaded && this.isViewerAnnotatable()) {
                 this.loadAnnotator();
             }
         });
@@ -583,6 +582,16 @@ class BaseViewer extends EventEmitter {
             }
         }
 
+        // Respect viewer-specific annotation option if it is set
+        return this.isViewerAnnotatable();
+    }
+
+    /**
+     * Returns whether or not viewer has the option to annotate
+     *
+     * @return {boolean} Whether or not viewer is annotatable
+     */
+    isViewerAnnotatable() {
         // Respect viewer-specific annotation option if it is set
         const viewerAnnotations = this.getViewerOption('annotations');
         if (typeof viewerAnnotations === 'boolean') {

diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js
@@ -680,29 +680,37 @@ describe('lib/viewers/BaseViewer', () => {
 
     describe('isAnnotatable()', () => {
         beforeEach(() => {
-            stubs.getViewerOption = sandbox.stub(base, 'getViewerOption');
-            stubs.getViewerOption.withArgs('annotations').returns(true);
             base.annotationTypes = ['point', 'highlight'];
+            sandbox.stub(base, 'isViewerAnnotatable').returns(true);
+        });
+
+        it('should return true if the type is supported by the viewer', () => {
+            expect(base.isAnnotatable('point')).to.equal(true);
         });
 
         it('should return false if the type is not supported by the viewer', () => {
             expect(base.isAnnotatable('drawing')).to.equal(false);
         });
+    });
+
+    describe('isViewerAnnotatable()', () => {
+        beforeEach(() => {
+            stubs.getViewerOption = sandbox.stub(base, 'getViewerOption').withArgs('annotations').returns(false);
+        });
 
         it('should return true if viewer option is set to true', () => {
-            expect(base.isAnnotatable('point')).to.equal(true);
-            stubs.getViewerOption.withArgs('annotations').returns(false);
-            expect(base.isAnnotatable('point')).to.equal(false);
+            expect(base.isViewerAnnotatable()).to.equal(false);
+            stubs.getViewerOption.returns(true);
+            expect(base.isViewerAnnotatable()).to.equal(true);
         });
 
         it('should use the global show annotationsBoolean if the viewer param is not specified', () => {
-            base.annotationTypes = null;
             stubs.getViewerOption.withArgs('annotations').returns(null);
             base.options.showAnnotations = true;
-            expect(base.isAnnotatable()).to.equal(true);
+            expect(base.isViewerAnnotatable()).to.equal(true);
 
             base.options.showAnnotations = false;
-            expect(base.isAnnotatable()).to.equal(false);
+            expect(base.isViewerAnnotatable()).to.equal(false);
         });
     });
 

diff --git a/src/lib/viewers/image/__tests__/ImageViewer-test.js b/src/lib/viewers/image/__tests__/ImageViewer-test.js
@@ -385,7 +385,7 @@ describe('lib/viewers/image/ImageViewer', () => {
 
     describe('loadUI()', () => {
         beforeEach(() => {
-            image.annotationsLoaded = false;
+            image.boxAnnotationsLoaded = false;
         });
 
         it('should load UI & controls for zoom', () => {
@@ -395,7 +395,7 @@ describe('lib/viewers/image/ImageViewer', () => {
 
             expect(image.controls).to.not.be.undefined;
             expect(image.controls.buttonRefs.length).to.equal(5);
-            expect(image.annotationsLoaded).to.be.false;
+            expect(image.boxAnnotationsLoaded).to.be.false;
         });
     });