Skip to content

Commit

Permalink
Fix: Validate passed in boxAnnotations is instance of the proper type (
Browse files Browse the repository at this point in the history
…#542)

* Fix: Validate passed in boxAnnotations is instance of the proper type
* Chore: Remove extra areAnnotationsEnabled() in loadAnnotator()
  • Loading branch information
pramodsum authored Dec 11, 2017
1 parent e3281cf commit 3a1ae72
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 19 deletions.
9 changes: 2 additions & 7 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -689,14 +689,9 @@ class BaseViewer extends EventEmitter {
* @return {void}
*/
loadAnnotator() {
// Do nothing if annotations are disabled for the viewer
if (!this.areAnnotationsEnabled()) {
return;
}

// Auto-resolves promise if BoxAnnotations is passed in as a Preview option
this.annotationsLoadPromise =
this.options.boxAnnotations instanceof BoxAnnotations
window.BoxAnnotations && this.options.boxAnnotations instanceof window.BoxAnnotations
? Promise.resolve()
: this.loadAssets([ANNOTATIONS_JS], [ANNOTATIONS_CSS]);
}
Expand Down Expand Up @@ -764,7 +759,7 @@ class BaseViewer extends EventEmitter {
*/
areAnnotationsEnabled() {
// Respect viewer-specific annotation option if it is set
if (this.options.boxAnnotations instanceof BoxAnnotations) {
if (window.BoxAnnotations && this.options.boxAnnotations instanceof window.BoxAnnotations) {
const { boxAnnotations, viewer } = this.options;
const annotatorConfig = boxAnnotations.options[viewer.NAME];
this.viewerConfig = {
Expand Down
27 changes: 15 additions & 12 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable no-unused-expressions */
import EventEmitter from 'events';
import BoxAnnotations from 'box-annotations';
import BaseViewer from '../BaseViewer';
import Browser from '../../Browser';
import RepStatus from '../../RepStatus';
Expand Down Expand Up @@ -764,29 +763,30 @@ describe('lib/viewers/BaseViewer', () => {
});

describe('loadAnnotator()', () => {
const conf = {
annotationsEnabled: true,
types: {
point: true,
highlight: false
}
};

beforeEach(() => {
sandbox.stub(base, 'areAnnotationsEnabled');
sandbox.stub(base, 'loadAssets');
});

it('should do nothing if annotations are not enabled', () => {
base.areAnnotationsEnabled.returns(false);
base.loadAnnotator();
expect(base.loadAssets).to.not.be.called;

window.BoxAnnotations = function BoxAnnotations() {
this.determineAnnotator = sandbox.stub().returns(conf);
}
});

it('should resolve the promise if a BoxAnnotations instance was passed into Preview', (done) => {
base.areAnnotationsEnabled.returns(true);
base.options.boxAnnotations = new BoxAnnotations({});
base.options.boxAnnotations = new window.BoxAnnotations({});

base.loadAnnotator();
expect(base.loadAssets).to.not.be.calledWith(['annotations.js']);
base.annotationsLoadPromise.then(() => done());
});

it('should load the annotations assets', () => {
base.areAnnotationsEnabled.returns(true);
base.loadAnnotator();
expect(base.loadAssets).to.be.calledWith(['annotations.js'], ['annotations.css']);
});
Expand Down Expand Up @@ -907,6 +907,9 @@ describe('lib/viewers/BaseViewer', () => {
'viewerName': { enabled: true }
}
expect(base.areAnnotationsEnabled()).to.equal(true);

window.BoxAnnotations = undefined;
expect(base.areAnnotationsEnabled()).to.equal(false);
});
});

Expand Down

0 comments on commit 3a1ae72

Please sign in to comment.