Skip to content

Commit

Permalink
New: Update BaseViewer to use box-annotations npm package (#459)
Browse files Browse the repository at this point in the history 
* Chore: Stop bundling annotations in box-content-preview
* Update: Adding try/catch around BoxAnnotations constructor
* Chore: Only including box-annotations in webpack processing
* Update: Bump box-annotations to v0.1.0
  • Loading branch information
@pramodsum
pramodsum authored Oct 30, 2017
1 parent 3ea11cc commit 4f71ab2
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 51 deletions.
4 changes: 2 additions & 2 deletions build/webpack.common.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ module.exports = (language) => {
}),
exclude: [
path.resolve('src/third-party'),
path.resolve('node_modules')
path.resolve('^(?!node_modules/box-annotations).*')
]
},
{
test: /\.(svg|html)$/,
loader: 'raw-loader',
exclude: [
path.resolve('src/third-party'),
path.resolve('node_modules')
path.resolve('^(?!node_modules/box-annotations).*')
]
},
{
Expand Down
3 changes: 1 addition & 2 deletions build/webpack.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ function updateConfig(conf, language, index) {
const config = Object.assign(conf, {
entry: {
preview: [`${lib}/Preview.js`],
csv: [`${lib}/viewers/text/BoxCSV.js`],
annotations: [`${lib}/annotations/BoxAnnotations.js`]
csv: [`${lib}/viewers/text/BoxCSV.js`]
},
output: {
path: path.resolve('dist', version, language),
Expand Down
3 changes: 1 addition & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"babel-preset-es2015": "^6.24.0",
"babel-preset-es2016": "^6.22.0",
"babel-preset-react": "^6.23.0",
"box-annotations": "^0.1.0",
"chai": "^3.5.0",
"chai-as-promised": "5.3.0",
"chai-dom": "^1.5.0",
Expand Down Expand Up @@ -79,9 +80,7 @@
"postcss-loader": "^2.0.6",
"prettier": "^1.5.3",
"prettier-eslint-cli": "^4.1.1",
"rangy": "^1.3.0",
"raw-loader": "^0.5.1",
"rbush": "^2.0.1",
"react": "^16.0.0",
"react-dom": "^16.0.0",
"react-virtualized": "^9.9.0",
Expand Down
30 changes: 12 additions & 18 deletions src/lib/viewers/BaseViewer.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'box-annotations/lib/Annotator.scss';
import BoxAnnotations from 'box-annotations/lib/BoxAnnotations';
import autobind from 'autobind-decorator';
import EventEmitter from 'events';
import debounce from 'lodash.debounce';
Expand Down Expand Up @@ -30,8 +32,6 @@ import {
} from '../constants';
import { ICON_FILE_DEFAULT } from '../icons/icons';

const ANNOTATIONS_JS = ['annotations.js'];
const ANNOTATIONS_CSS = ['annotations.css'];
const ANNOTATION_TYPE_DRAW = 'draw';
const ANNOTATION_TYPE_POINT = 'point';
const LOAD_TIMEOUT_MS = 180000; // 3m
Expand Down Expand Up @@ -140,7 +140,7 @@ class BaseViewer extends EventEmitter {
// the assets are available, the showAnnotations flag is true, and the
// expiring embed is not a shared link
if (this.areAnnotationsEnabled() && !this.options.sharedLink) {
this.annotationsPromise = this.loadAssets(ANNOTATIONS_JS, ANNOTATIONS_CSS);
this.loadAnnotator();
}
}

Expand Down Expand Up @@ -365,12 +365,8 @@ class BaseViewer extends EventEmitter {
this.scale = event.scale;
}

if (this.annotationsPromise) {
this.annotationsPromise.then(this.loadAnnotator).catch((error) => {
/* eslint-disable no-console */
console.error('Annotation assets failed to load', error.toString());
/* eslint-enable no-console */
});
if (this.annotatorConf) {
this.initAnnotations();
}
});
}
Expand Down Expand Up @@ -661,16 +657,14 @@ class BaseViewer extends EventEmitter {
return;
}

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

// No annotatorConf will be returned if the user does not have the correct permissions
if (!this.annotatorConf) {
return;
try {
const boxAnnotations = new BoxAnnotations();
this.annotatorConf = boxAnnotations.determineAnnotator(this.options, this.viewerConfig);
} catch (err) {
/* eslint-disable no-console */
console.error('Annotation assets failed to load');
/* eslint-enable no-console */
}

this.initAnnotations();
}

/**
Expand Down
46 changes: 19 additions & 27 deletions src/lib/viewers/__tests__/BaseViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ describe('lib/viewers/BaseViewer', () => {
describe('setup()', () => {
it('should set options, a container, bind event listeners, and set timeout', () => {
sandbox.stub(base, 'addCommonListeners');
sandbox.stub(base, 'loadAssets').returns(Promise.resolve());
sandbox.stub(base, 'areAnnotationsEnabled').returns(true);
sandbox.stub(base, 'finishLoadingSetup');
sandbox.stub(base, 'loadAnnotator');
base.options.showAnnotations = true;

base.setup();
Expand All @@ -65,12 +66,12 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.containerEl).to.have.class('bp');
expect(base.addCommonListeners).to.be.called;
expect(base.loadTimeout).to.be.a.number;
expect(base.loadAssets).to.be.called;
expect(base.loadAnnotator).to.be.called;
});

it('should add a mobile class to the container if on mobile', () => {
base.isMobile = true;
sandbox.stub(base, 'loadAssets').returns(Promise.resolve());
sandbox.stub(base, 'loadAnnotator');
sandbox.stub(base, 'finishLoadingSetup');

base.setup();
Expand All @@ -81,24 +82,26 @@ describe('lib/viewers/BaseViewer', () => {

it('should not load annotations assets if global preview showAnnotations option is false', () => {
sandbox.stub(base, 'addCommonListeners');
sandbox.stub(base, 'loadAssets');
sandbox.stub(base, 'areAnnotationsEnabled').returns(false);
sandbox.stub(base, 'loadAnnotator');
sandbox.stub(base, 'finishLoadingSetup');
base.options.showAnnotations = false;

base.setup();

expect(base.loadAssets).to.not.be.called;
expect(base.loadAnnotator).to.not.be.called;
});

it('should not load annotations assets if expiring embed is a shared link', () => {
sandbox.stub(base, 'addCommonListeners');
sandbox.stub(base, 'loadAssets');
sandbox.stub(base, 'areAnnotationsEnabled').returns(true);
sandbox.stub(base, 'loadAnnotator');
sandbox.stub(base, 'finishLoadingSetup');
base.options.sharedLink = 'url';

base.setup();

expect(base.loadAssets).to.not.be.called;
expect(base.loadAnnotator).to.not.be.called;
});
});

Expand Down Expand Up @@ -284,7 +287,7 @@ describe('lib/viewers/BaseViewer', () => {
stubs.fullscreenAddListener = sandbox.stub(fullscreen, 'addListener');
stubs.baseAddListener = sandbox.spy(base, 'addListener');
stubs.documentAddEventListener = sandbox.stub(document.defaultView, 'addEventListener');
stubs.loadAnnotator = sandbox.stub(base, 'loadAnnotator');
sandbox.stub(base, 'initAnnotations');


});
Expand Down Expand Up @@ -312,34 +315,27 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.containerEl.addEventListener).to.be.calledWith('contextmenu', sinon.match.func);
});

it('should load the annotator when load is emitted with scale', (done) => {
it('should load the annotator when load is emitted with scale', () => {
base.containerEl = containerEl;
base.annotationsPromise = {
then: (arg) => {
expect(base.scale).to.equal(1.5);
expect(arg).to.equal(base.loadAnnotator);
done();
}
};
base.annotatorConf = {};

base.addCommonListeners();
expect(base.scale).to.equal(1);

base.emit('load', {
scale: 1.5
});
expect(base.scale).to.equal(1.5);
expect(base.initAnnotations).to.be.called;
});

it('should load the annotator when load is emitted without an event', (done) => {
it('should not load the annotator when there is no annotator config', () => {
base.containerEl = containerEl;
base.annotationsPromise = {
then: (arg) => {
expect(arg).to.equal(base.loadAnnotator);
done();
}
};
base.annotatorConf = undefined;

base.addCommonListeners();
base.emit('load');
expect(base.initAnnotations).to.not.be.called;
});
});

Expand Down Expand Up @@ -768,7 +764,6 @@ describe('lib/viewers/BaseViewer', () => {
TYPE: ['point', 'highlight']
};
stubs.areAnnotationsEnabled = sandbox.stub(base, 'areAnnotationsEnabled').returns(true);
sandbox.stub(base, 'initAnnotations');

base.options = {
viewer: {
Expand Down Expand Up @@ -806,7 +801,6 @@ describe('lib/viewers/BaseViewer', () => {

window.BoxAnnotations = BoxAnnotations;
base.loadAnnotator();
expect(base.initAnnotations).to.be.called;
});

it('should not load an annotator if no loader was found', () => {
Expand All @@ -815,7 +809,6 @@ describe('lib/viewers/BaseViewer', () => {
}
window.BoxAnnotations = BoxAnnotations;
base.loadAnnotator();
expect(base.initAnnotations).to.not.be.called;
});

it('should not load an annotator if the viewer is not annotatable', () => {
Expand All @@ -827,7 +820,6 @@ describe('lib/viewers/BaseViewer', () => {
window.BoxAnnotations = BoxAnnotations;
stubs.areAnnotationsEnabled.returns(false);
base.loadAnnotator();
expect(base.initAnnotations).to.not.be.called;
});
});

Expand Down
7 changes: 7 additions & 0 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,13 @@ [email protected]:
dependencies:
hoek "4.x.x"

box-annotations@^0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/box-annotations/-/box-annotations-0.1.0.tgz#2951ec199eb7674c2b8a00825cc36a0b5b13711f"
dependencies:
rangy "^1.3.0"
rbush "^2.0.1"

brace-expansion@^1.1.7:
version "1.1.8"
resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-1.1.8.tgz#c07b211c7c952ec1f8efd51a77ef0d1d3990a292"
Expand Down

0 comments on commit 4f71ab2

Please sign in to comment.