Skip to content

Commit

Permalink
Revert annotations bundling (#94)
Browse files Browse the repository at this point in the history
- Revert "Fix: Ensuring annotations are loaded when viewer has permissions (#90)"
- Revert "Chore: Bundling Annotators separately in annotations.js (#75)"
- This fixes preview performance metrics based on the 'load' event
  • Loading branch information
tonyjin authored Apr 29, 2017
1 parent 3e1e045 commit 176ad13
Show file tree
Hide file tree
Showing 22 changed files with 767 additions and 890 deletions.
3 changes: 1 addition & 2 deletions build/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,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
6 changes: 6 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
hideLoadingIndicator,
showDownloadButton,
showLoadingDownloadButton,
showAnnotateButton,
showPrintButton,
showNavigation
} from './ui';
Expand All @@ -45,6 +46,7 @@ import {
CLASS_NAVIGATION_VISIBILITY,
FILE_EXT_ERROR_MAP,
PERMISSION_DOWNLOAD,
PERMISSION_ANNOTATE,
PERMISSION_PREVIEW,
PREVIEW_SCRIPT_NAME,
X_REP_HINT_BASE,
Expand Down Expand Up @@ -932,6 +934,10 @@ class Preview extends EventEmitter {
}
}

if (checkPermission(this.file, PERMISSION_ANNOTATE) && !Browser.isMobile() && checkFeature(this.viewer, 'isAnnotatable', 'point')) {
showAnnotateButton(this.viewer.getPointModeClickHandler());
}

const { error } = data;
if (error) {
// Bump up preview count
Expand Down
38 changes: 38 additions & 0 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,7 @@ describe('lib/Preview', () => {
stubs.canDownload = sandbox.stub(Browser, 'canDownload');
stubs.showDownloadButton = sandbox.stub(ui, 'showDownloadButton');
stubs.showPrintButton = sandbox.stub(ui, 'showPrintButton');
stubs.showAnnotateButton = sandbox.stub(ui, 'showAnnotateButton');
stubs.hideLoadingIndicator = sandbox.stub(ui, 'hideLoadingIndicator');
stubs.emit = sandbox.stub(preview, 'emit');
stubs.logPreviewEvent = sandbox.stub(preview, 'logPreviewEvent');
Expand Down Expand Up @@ -1368,6 +1369,43 @@ describe('lib/Preview', () => {
expect(stubs.showPrintButton).to.be.called;
});

it('should show the annotation button if you have annotation permissions', () => {
stubs.checkPermission.onCall(1).returns(false).onCall(2).returns(true);


preview.finishLoading();
expect(stubs.showAnnotateButton).to.not.be.called;

stubs.checkPermission.onCall(3).returns(true);

preview.finishLoading();
expect(stubs.showAnnotateButton).to.be.called;
});

it('should show the annotation button if you are not on a mobile browser', () => {
stubs.isMobile.returns(true);

preview.finishLoading();
expect(stubs.showAnnotateButton).to.not.be.called;

stubs.isMobile.returns(false);

preview.finishLoading();
expect(stubs.showAnnotateButton).to.be.called;
});

it('should show the annotation button if the viewer has annotation functionality', () => {
stubs.checkFeature.returns(false);

preview.finishLoading();
expect(stubs.showAnnotateButton).to.not.be.called;

stubs.checkFeature.returns(true);

preview.finishLoading();
expect(stubs.showAnnotateButton).to.be.called;
});

it('should increment the preview count', () => {
preview.count.success = 0;

Expand Down
13 changes: 13 additions & 0 deletions src/lib/__tests__/ui-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,19 @@ describe('lib/ui', () => {
});
});

describe('showAnnotateButton()', () => {
it('should set up and show annotate button', () => {
const buttonEl = containerEl.querySelector(constants.SELECTOR_BOX_PREVIEW_BTN_ANNOTATE);
buttonEl.classList.add(constants.CLASS_HIDDEN);
sandbox.mock(buttonEl).expects('addEventListener').withArgs('click', handler);

ui.showAnnotateButton(handler);

expect(buttonEl.title).to.equal('Point annotation mode');
expect(buttonEl.classList.contains(constants.CLASS_HIDDEN)).to.be.false;
});
});

describe('showPrintButton()', () => {
it('should set up and show print button', () => {
const buttonEl = containerEl.querySelector(constants.SELECTOR_BOX_PREVIEW_BTN_PRINT);
Expand Down
39 changes: 4 additions & 35 deletions src/lib/annotations/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ class Annotator extends EventEmitter {
*/
constructor(data) {
super();

this.canAnnotate = data.canAnnotate;
this.container = data.container;
this.options = data.options;
this.annotatedElement = data.annotatedElement;
this.annotationService = data.annotationService;
this.fileVersionID = data.fileVersionID;
this.locale = data.locale;
this.validationErrorDisplayed = false;

this.notification = new Notification(this.annotatedElement);
}

/**
Expand All @@ -65,29 +65,8 @@ class Annotator extends EventEmitter {
* @return {void}
*/
init() {
this.annotatedElement = this.getAnnotatedEl(this.container);
this.annotationService = this.initAnnotationService(this.options);
this.notification = new Notification(this.annotatedElement);

this.setScale(1);
this.setupAnnotations();
this.showAnnotations();
}

/**
* Initializes the Annotation Service with appropriate options
*
* @param {Object} options - Options passed from the viewer to the annotator
* @return {AnnotationService} AnnotationService instance
*/
initAnnotationService(options) {
const { apiHost, fileId, token } = options;
return new AnnotationService({
apiHost,
fileId,
token,
canAnnotate: this.canAnnotate
});
}

/**
Expand Down Expand Up @@ -240,16 +219,6 @@ class Annotator extends EventEmitter {
createAnnotationThread(annotations, location, type) {}
/* eslint-enable no-unused-vars */

/**
* Must be implemented to determine the annotated element in the viewer.
*
* @param {HTMLElement} containerEl - Container element for the viewer
* @return {HTMLElement} Annotated element in the viewer
*/
/* eslint-disable no-unused-vars */
getAnnotatedEl(containerEl) {}
/* eslint-enable no-unused-vars */

//--------------------------------------------------------------------------
// Protected
//--------------------------------------------------------------------------
Expand Down
54 changes: 0 additions & 54 deletions src/lib/annotations/BoxAnnotations.js

This file was deleted.

Loading

0 comments on commit 176ad13

Please sign in to comment.