From 94d52f5de2280767e455412fb5a535d97a8c9c86 Mon Sep 17 00:00:00 2001 From: Jared Stoffan Date: Wed, 17 Jun 2020 13:44:46 -0700 Subject: [PATCH] feat(annotations): Add support for image annotations (#1221) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- src/lib/viewers/BaseViewer.js | 34 +++++++++-- src/lib/viewers/__tests__/BaseViewer-test.js | 61 ++++++++++++++----- src/lib/viewers/doc/DocBaseViewer.js | 25 ++------ .../doc/__tests__/DocBaseViewer-test.js | 2 +- src/lib/viewers/image/ImageViewer.js | 13 ++-- .../image/__tests__/ImageViewer-test.js | 10 +++ 6 files changed, 98 insertions(+), 47 deletions(-) diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index e5e57ac15..e75c1e919 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -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); @@ -243,6 +244,10 @@ class BaseViewer extends EventEmitter { }); } + if (this.annotationControls) { + this.annotationControls.destroy(); + } + fullscreen.removeAllListeners(); document.defaultView.removeEventListener('resize', this.debouncedResizeHandler); this.removeAllListeners(); @@ -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); } /** @@ -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; } diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 9a285f686..521f14623 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -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); @@ -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; @@ -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); @@ -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 @@ -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; }); diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index f4b66b5ff..ca536618c 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -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'; @@ -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); @@ -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); } @@ -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(); } @@ -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. * diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 46a53e23f..a3606f43d 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -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, }); }); diff --git a/src/lib/viewers/image/ImageViewer.js b/src/lib/viewers/image/ImageViewer.js index 263a3310f..5d4b4ee52 100644 --- a/src/lib/viewers/image/ImageViewer.js +++ b/src/lib/viewers/image/ImageViewer.js @@ -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'; @@ -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 }); + } } /** @@ -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; } //-------------------------------------------------------------------------- diff --git a/src/lib/viewers/image/__tests__/ImageViewer-test.js b/src/lib/viewers/image/__tests__/ImageViewer-test.js index f26979386..cb277de63 100644 --- a/src/lib/viewers/image/__tests__/ImageViewer-test.js +++ b/src/lib/viewers/image/__tests__/ImageViewer-test.js @@ -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'; @@ -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; }); });