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

Selectively disable annotations #322

Merged
44 changes: 38 additions & 6 deletions src/lib/annotations/BoxAnnotations.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,49 @@ class BoxAnnotations {
return Array.isArray(this.annotators) ? this.annotators : [];
}

/**
* Get all annotators for a given viewer.
*
* @param {string} viewerName - Name of the viewer to get annotators for
* @return {Object} Annotator for the viewer
*/
getAnnotatorsForViewer(viewerName, disabledAnnotators = []) {
const annotators = this.getAnnotators();

return annotators.find(
(annotator) => !disabledAnnotators.includes(annotator.NAME) && annotator.VIEWER.includes(viewerName)
);
}

/**
* Chooses a annotator based on viewer.
*
* @param {Object} viewer - Current preview viewer
* @param {Object} viewerName - Current preview viewer name
* @param {Array} [disabledAnnotators] - List of disabled annotators
Copy link
Contributor

Choose a reason for hiding this comment

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

reorder these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This already has been reorder to what we've already discussed

Copy link
Contributor

@pramodsum pramodsum Aug 21, 2017

Choose a reason for hiding this comment

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

👍 I think i misread it because both were arrays here

* @return {Object} The annotator to use
* @param {Array} [viewerConfig] - Annotation configuration for a specific viewer
* @return {Object|null} A copy of the annotator to use, if available
*/
determineAnnotator(viewer, disabledAnnotators = []) {
return this.annotators.find(
(annotator) => !disabledAnnotators.includes(annotator.NAME) && annotator.VIEWER.includes(viewer)
);
determineAnnotator(viewerName, disabledAnnotators = [], viewerConfig = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this instead
determineAnnotator(viewerName, viewerConfig = {}, disabledAnnotators = []) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1, and done

const originalAnnotator = this.getAnnotatorsForViewer(viewerName, disabledAnnotators);

Copy link
Contributor

Choose a reason for hiding this comment

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

const annotator = this.getAnnotatorsForViewer(viewerName, disabledAnnotators);
const modifiedAnnotator = null;
if (!annotator) {
    return modifiedAnnotator;
}

modifiedAnnotator = ....
if (viewerConfig.enabled) {
    if (Array.isArray(....)) {
        ...
    }
}
return modifiedAnnotator;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much better, ty. Done

if (originalAnnotator) {
const annotator = Object.assign({}, originalAnnotator);
// If explicitly disabled via config, do nothing
if (viewerConfig.enabled === false) {
return null;
}

// Filter out disabled annotation types
if (Array.isArray(viewerConfig.disabledTypes)) {
annotator.TYPE = annotator.TYPE.filter((type) => {
return !viewerConfig.disabledTypes.includes(type);
});
}

return annotator;
}

return null;
}
}

Expand Down
45 changes: 35 additions & 10 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -622,28 +622,52 @@ class BaseViewer extends EventEmitter {
// Annotations
//--------------------------------------------------------------------------

/**
* Get the configuration for viewer annotations and transform if legacy format.
*
* @return {Object} An object containing configuration properties.
*/
getViewerAnnotationsConfig() {
const config = this.getViewerOption('annotations') || {};

// Backwards compatability for old boolean flag usage
if (typeof config === 'boolean') {
return {
enabled: config
};
}

return config;
}

/**
* Loads the appropriate annotator and loads the file's annotations
*
* @protected
* @return {void}
*/
loadAnnotator() {
// Do nothing if annotations are disabled for the viewer
if (!this.areAnnotationsEnabled()) {
return;
}

/* global BoxAnnotations */
const boxAnnotations = new BoxAnnotations();
// #TODO(@spramod|@jholdstock): remove this after we have annotation instancing
const viewerName = this.options.viewer.NAME;
const annotationsConfig = this.getViewerAnnotationsConfig();

this.annotatorConf = boxAnnotations.determineAnnotator(this.options.viewer.NAME);
this.annotatorConf = boxAnnotations.determineAnnotator(viewerName, undefined, annotationsConfig);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above but if we modify the function declaration, then we can call it like this
boxAnnotations.determineAnnotator(viewerName, annotationsConfig);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DONE! :D

if (!this.annotatorConf) {
return;
}

if (this.areAnnotationsEnabled()) {
const { file } = this.options;
this.canAnnotate = checkPermission(file, PERMISSION_ANNOTATE);
const { file } = this.options;
this.canAnnotate = checkPermission(file, PERMISSION_ANNOTATE);

if (this.canAnnotate) {
this.initAnnotations();
}
if (this.canAnnotate) {
this.initAnnotations();
}
}

Expand Down Expand Up @@ -696,9 +720,10 @@ class BaseViewer extends EventEmitter {
*/
areAnnotationsEnabled() {
// Respect viewer-specific annotation option if it is set
const viewerAnnotations = this.getViewerOption('annotations');
if (typeof viewerAnnotations === 'boolean') {
return viewerAnnotations;
const viewerAnnotations = this.getViewerAnnotationsConfig();

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

// Otherwise, use global preview annotation option
Expand Down