Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Allow BoxAnnotations to be passed in as a Preview option #539

Merged
merged 4 commits into from
Dec 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,9 @@ class Preview extends EventEmitter {
// Custom Box3D application definition
this.options.box3dApplication = options.box3dApplication;

// Custom BoxAnnotations definition
this.options.boxAnnotations = options.boxAnnotations;

// Save the reference to any additional custom options for viewers
this.options.viewers = options.viewers || {};

Expand Down
27 changes: 22 additions & 5 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,18 +694,27 @@ class BaseViewer extends EventEmitter {
return;
}

this.annotationsLoadPromise = this.loadAssets([ANNOTATIONS_JS], [ANNOTATIONS_CSS]);
// Auto-resolves promise if BoxAnnotations is passed in as a Preview option
this.annotationsLoadPromise =
this.options.boxAnnotations instanceof BoxAnnotations
? Promise.resolve()
: this.loadAssets([ANNOTATIONS_JS], [ANNOTATIONS_CSS]);
}

/**
* Fetches the Box Annotations library
* Fetches the Box Annotations library. Creates an instance of BoxAnnotations
* if one isn't passed in to the preview options
*
* @protected
* @return {void}
*/
annotationsLoadHandler() {
// Set viewer-specific annotation options
const viewerOptions = {};
viewerOptions[this.options.viewer.NAME] = this.viewerConfig;

/* global BoxAnnotations */
const boxAnnotations = new BoxAnnotations();
const boxAnnotations = this.options.boxAnnotations || new BoxAnnotations(viewerOptions);
this.annotatorConf = boxAnnotations.determineAnnotator(this.options, this.viewerConfig);

if (this.annotatorConf) {
Expand Down Expand Up @@ -755,13 +764,21 @@ class BaseViewer extends EventEmitter {
*/
areAnnotationsEnabled() {
// Respect viewer-specific annotation option if it is set
// #TODO(@spramod|@jholdstock): remove this after we have annotation instancing
Copy link
Contributor Author

@pramodsum pramodsum Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.viewerConfig = this.getViewerAnnotationsConfig();
if (this.options.boxAnnotations instanceof BoxAnnotations) {
const { boxAnnotations, viewer } = this.options;
const annotatorConfig = boxAnnotations.options[viewer.NAME];
this.viewerConfig = {
enabled: annotatorConfig.enabled || !!annotatorConfig.enabledTypes
};
} else {
this.viewerConfig = this.getViewerAnnotationsConfig();
}

if (this.viewerConfig && this.viewerConfig.enabled !== undefined) {
return this.viewerConfig.enabled;
}

// Ignore viewer config if BoxAnnotations was pass into Preview as an option
// Otherwise, use global preview annotation option
return !!this.options.showAnnotations;
}
Expand Down
35 changes: 34 additions & 1 deletion src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* 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 @@ -775,10 +776,19 @@ describe('lib/viewers/BaseViewer', () => {

});

it('should resolve the promise if a BoxAnnotations instance was passed into Preview', (done) => {
base.areAnnotationsEnabled.returns(true);
base.options.boxAnnotations = new 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']);
expect(base.loadAssets).to.be.calledWith(['annotations.js'], ['annotations.css']);
});
});

Expand All @@ -792,6 +802,7 @@ describe('lib/viewers/BaseViewer', () => {
};

beforeEach(() => {
base.options.viewer = { NAME: 'viewerName' };
window.BoxAnnotations = function BoxAnnotations() {
this.determineAnnotator = sandbox.stub().returns(conf);
}
Expand All @@ -804,6 +815,14 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.annotatorConf).to.equal(conf);
});

it('should not instantiate an instance of BoxAnnotations if one is already passed in', () => {
base.options.boxAnnotations = {
determineAnnotator: sandbox.stub()
};
base.annotationsLoadHandler();
expect(base.options.boxAnnotations.determineAnnotator).to.be.called;
});

it('should init annotations if a conf is present', () => {
base.annotationsLoadHandler();
expect(base.initAnnotations).to.be.called;
Expand Down Expand Up @@ -875,6 +894,20 @@ describe('lib/viewers/BaseViewer', () => {
base.options.showAnnotations = false;
expect(base.areAnnotationsEnabled()).to.equal(false);
});

it('should user BoxAnnotations options if an instance of BoxAnnotations is passed into Preview', () => {
stubs.getViewerOption.withArgs('annotations').returns(null);
base.options.showAnnotations = false;
base.options.boxAnnotations = undefined;
expect(base.areAnnotationsEnabled()).to.equal(false);

base.options.viewer = { NAME: 'viewerName' };
base.options.boxAnnotations = sinon.createStubInstance(window.BoxAnnotations);
base.options.boxAnnotations.options = {
'viewerName': { enabled: true }
}
expect(base.areAnnotationsEnabled()).to.equal(true);
});
});

describe('getViewerAnnotationsConfig()', () => {
Expand Down