Skip to content

Commit

Permalink
feat(annotations): Add support for image annotations (#1221)
Browse files Browse the repository at this point in the history
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
jstoffan and mergify[bot] authored Jun 17, 2020
1 parent b84d809 commit 94d52f5
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 47 deletions.
34 changes: 28 additions & 6 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ class BaseViewer extends EventEmitter {
this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this);
this.handleFullscreenEnter = this.handleFullscreenEnter.bind(this);
this.handleFullscreenExit = this.handleFullscreenExit.bind(this);
this.handleRegionClick = this.handleRegionClick.bind(this);
this.createAnnotator = this.createAnnotator.bind(this);
this.viewerLoadHandler = this.viewerLoadHandler.bind(this);
this.initAnnotations = this.initAnnotations.bind(this);
Expand Down Expand Up @@ -243,6 +244,10 @@ class BaseViewer extends EventEmitter {
});
}

if (this.annotationControls) {
this.annotationControls.destroy();
}

fullscreen.removeAllListeners();
document.defaultView.removeEventListener('resize', this.debouncedResizeHandler);
this.removeAllListeners();
Expand Down Expand Up @@ -1016,16 +1021,33 @@ class BaseViewer extends EventEmitter {
}

/**
* Returns whether or not user has permissions to create annotations on the current file
* Returns whether or not user has permissions to create new annotations on the current file
*
* @param {Object} permissions Permissions on the current file
* @return {boolean} Whether or not user has create permission
*/
hasAnnotationCreatePermission(permissions = this.options.file.permissions) {
if (!permissions) {
return false;
}
return permissions.can_annotate || permissions.can_create_annotations;
return !!permissions && !!permissions.can_create_annotations;
}

/**
* Returns whether or not user has permissions to view new annotations on the current file
*
* @param {Object} permissions Permissions on the current file
* @return {boolean} Whether or not user has view permission
*/
hasAnnotationViewPermission(permissions = this.options.file.permissions) {
return !!permissions && !!permissions.can_view_annotations;
}

/**
* Handler for annotation toolbar region comment button click event.
*
* @private
* @return {void}
*/
handleRegionClick() {
this.annotator.toggleAnnotationMode(AnnotationMode.REGION);
}

/**
Expand Down Expand Up @@ -1114,7 +1136,7 @@ class BaseViewer extends EventEmitter {
const { showAnnotationsControls, file } = this.options;
const { permissions, extension } = file || {};

if (!this.hasAnnotationPermissions(permissions)) {
if (!this.hasAnnotationCreatePermission(permissions) && !this.hasAnnotationViewPermission(permissions)) {
return false;
}

Expand Down
61 changes: 45 additions & 16 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ describe('lib/viewers/BaseViewer', () => {
toggleAnnotationMode: sandbox.mock(),
};
base.annotationControls = {
destroy: sandbox.mock(),
resetControls: sandbox.mock(),
};
sandbox.stub(base, 'areNewAnnotationsEnabled').returns(true);
Expand Down Expand Up @@ -618,10 +619,18 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.emit).to.be.calledWith('destroy');
});

it('should clean up the annotation controls', () => {
base.annotationControls = {
destroy: sandbox.stub(),
};
base.destroy();
expect(base.annotationControls.destroy).to.be.called;
});

it('should clean up annotator', () => {
base.annotator = {
removeAllListeners: sandbox.mock(),
destroy: sandbox.mock(),
removeAllListeners: sandbox.stub(),
destroy: sandbox.stub(),
};
base.destroy();
expect(base.annotator.removeAllListeners).to.be.called;
Expand Down Expand Up @@ -1175,6 +1184,7 @@ describe('lib/viewers/BaseViewer', () => {
CONSTRUCTOR: sandbox.stub().returns(base.annotator),
};
base.annotationControls = {
destroy: sandbox.stub(),
resetControls: sandbox.stub(),
};
sandbox.stub(base, 'areNewAnnotationsEnabled').returns(true);
Expand Down Expand Up @@ -1211,26 +1221,48 @@ describe('lib/viewers/BaseViewer', () => {
let permissions = {};
beforeEach(() => {
permissions = {
can_annotate: false,
can_create_annotations: false,
};
});

it('should return false if both create permissions are false', () => {
expect(base.hasAnnotationCreatePermission(permissions)).to.be.false;
it('should return false if it does not receive permissions', () => {
expect(base.hasAnnotationCreatePermission(null)).to.be.false;
expect(base.hasAnnotationCreatePermission(undefined)).to.be.false;
});

it('should return true if it has old create permission', () => {
permissions.can_annotate = true;
expect(base.hasAnnotationCreatePermission(permissions)).to.be.true;
it('should return false if it receives new create permissions that are false', () => {
expect(base.hasAnnotationCreatePermission(permissions)).to.be.false;
});

it('should return true if it has new create permission', () => {
it('should return true if it receives new create permissions that are true', () => {
permissions.can_create_annotations = true;
expect(base.hasAnnotationCreatePermission(permissions)).to.be.true;
});
});

describe('hasAnnotationViewPermission()', () => {
let permissions = {};
beforeEach(() => {
permissions = {
can_view_annotations: false,
};
});

it('should return false if it does not receive permissions', () => {
expect(base.hasAnnotationViewPermission(null)).to.be.false;
expect(base.hasAnnotationViewPermission(undefined)).to.be.false;
});

it('should return false if it receives new view permissions that are false', () => {
expect(base.hasAnnotationViewPermission(permissions)).to.be.false;
});

it('should return true if it receives new view permissions that are true', () => {
permissions.can_view_annotations = true;
expect(base.hasAnnotationViewPermission(permissions)).to.be.true;
});
});

describe('hasAnnotationPermissions()', () => {
const permissions = {
can_annotate: false, // Old
Expand Down Expand Up @@ -1421,16 +1453,13 @@ describe('lib/viewers/BaseViewer', () => {

describe('areNewAnnotationsEnabled()', () => {
beforeEach(() => {
stubs.hasPermissions = sandbox.stub(base, 'hasAnnotationPermissions').returns(true);
base.options.file = {
permissions: {
can_annotate: true,
},
};
stubs.hasCreatePermissions = sandbox.stub(base, 'hasAnnotationCreatePermission').returns(true);
stubs.hasViewPermissions = sandbox.stub(base, 'hasAnnotationViewPermission').returns(true);
});

it('should return false if the user cannot create/view annotations', () => {
stubs.hasPermissions.returns(false);
stubs.hasCreatePermissions.returns(false);
stubs.hasViewPermissions.returns(false);
expect(base.areNewAnnotationsEnabled()).to.be.false;
});

Expand Down
25 changes: 5 additions & 20 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import throttle from 'lodash/throttle';
import AnnotationControls, { AnnotationMode } from '../../AnnotationControls';
import AnnotationControls from '../../AnnotationControls';
import BaseViewer from '../BaseViewer';
import Browser from '../../Browser';
import Controls from '../../Controls';
Expand Down Expand Up @@ -97,7 +97,6 @@ class DocBaseViewer extends BaseViewer {
this.pinchToZoomEndHandler = this.pinchToZoomEndHandler.bind(this);
this.pinchToZoomStartHandler = this.pinchToZoomStartHandler.bind(this);
this.print = this.print.bind(this);
this.regionClickHandler = this.regionClickHandler.bind(this);
this.setPage = this.setPage.bind(this);
this.throttledScrollHandler = this.getScrollHandler().bind(this);
this.toggleThumbnails = this.toggleThumbnails.bind(this);
Expand Down Expand Up @@ -164,10 +163,6 @@ class DocBaseViewer extends BaseViewer {
URL.revokeObjectURL(this.printURL);
}

if (this.annotationControls) {
this.annotationControls.destroy();
}

if (this.pageControls) {
this.pageControls.removeListener('pagechange', this.setPage);
}
Expand Down Expand Up @@ -1014,11 +1009,13 @@ class DocBaseViewer extends BaseViewer {
loadUI() {
this.controls = new Controls(this.containerEl);
this.pageControls = new PageControls(this.controls, this.docEl);
this.pageControls.addListener('pagechange', this.setPage);
this.zoomControls = new ZoomControls(this.controls);

if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) {
this.annotationControls = new AnnotationControls(this.controls);
}
this.pageControls.addListener('pagechange', this.setPage);

this.bindControlListeners();
}

Expand Down Expand Up @@ -1101,22 +1098,10 @@ class DocBaseViewer extends BaseViewer {
this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT);

if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) {
this.annotationControls.init({
onRegionClick: this.regionClickHandler,
});
this.annotationControls.init({ onRegionClick: this.handleRegionClick });
}
}

/**
* Handler for annotation toolbar region comment button click event.
*
* @private
* @return {void}
*/
regionClickHandler() {
this.annotator.toggleAnnotationMode(AnnotationMode.REGION);
}

/**
* Handler for 'pagesinit' event.
*
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2303,7 +2303,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
ICON_FULLSCREEN_OUT,
);
expect(docBase.annotationControls.init).to.be.calledWith({
onRegionClick: docBase.regionClickHandler,
onRegionClick: docBase.handleRegionClick,
});
});

Expand Down
13 changes: 9 additions & 4 deletions src/lib/viewers/image/ImageViewer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import AnnotationControls from '../../AnnotationControls';
import ImageBaseViewer from './ImageBaseViewer';
import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT, ICON_ROTATE_LEFT } from '../../icons/icons';
import { CLASS_INVISIBLE } from '../../constants';

import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT, ICON_ROTATE_LEFT } from '../../icons/icons';
import './Image.scss';

const CSS_CLASS_IMAGE = 'bp-image';
Expand Down Expand Up @@ -285,6 +285,11 @@ class ImageViewer extends ImageBaseViewer {
ICON_FULLSCREEN_IN,
);
this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT);

if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) {
this.annotationControls = new AnnotationControls(this.controls);
this.annotationControls.init({ onRegionClick: this.handleRegionClick });
}
}

/**
Expand Down Expand Up @@ -339,8 +344,8 @@ class ImageViewer extends ImageBaseViewer {
this.imageEl.style.top = `${topPadding}px`;

// Fix the scroll position of the image to be centered
this.wrapperEl.scrollLeft = (this.wrapperEl.scrollWidth - viewport.width) / 2;
this.wrapperEl.scrollTop = (this.wrapperEl.scrollHeight - viewport.height) / 2;
this.wrapperEl.scrollLeft = (this.imageEl.clientWidth - viewport.width) / 2;
this.wrapperEl.scrollTop = (this.imageEl.clientHeight - viewport.height) / 2;
}

//--------------------------------------------------------------------------
Expand Down
10 changes: 10 additions & 0 deletions src/lib/viewers/image/__tests__/ImageViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable no-unused-expressions */
import AnnotationControls from '../../../AnnotationControls';
import ImageViewer from '../ImageViewer';
import BaseViewer from '../../BaseViewer';
import Browser from '../../../Browser';
Expand Down Expand Up @@ -351,6 +352,15 @@ describe('lib/viewers/image/ImageViewer', () => {
expect(image.controls).to.not.be.undefined;
expect(image.controls.buttonRefs.length).to.equal(5);
expect(image.zoomControls.currentScale).to.equal(50);
expect(image.annotationControls).to.be.undefined; // Not enabled by default
});

it('should add annotations controls', () => {
sandbox.stub(image, 'areNewAnnotationsEnabled').returns(true);
sandbox.stub(image, 'hasAnnotationCreatePermission').returns(true);

image.loadUI();
expect(image.annotationControls instanceof AnnotationControls).to.be.true;
});
});

Expand Down

0 comments on commit 94d52f5

Please sign in to comment.