From 62f782240566dc32ee8b2400e26b69254df963a7 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Thu, 18 Jun 2020 12:00:13 -0700 Subject: [PATCH 1/3] feat(annotations): Press Esc to exit annotations mode --- src/lib/AnnotationControls.ts | 36 +++++++++- src/lib/__tests__/AnnotationControls-test.js | 67 +++++++++++++++++++ src/lib/viewers/BaseViewer.js | 11 +++ src/lib/viewers/__tests__/BaseViewer-test.js | 12 ++++ src/lib/viewers/doc/DocBaseViewer.js | 5 +- .../doc/__tests__/DocBaseViewer-test.js | 1 + 6 files changed, 130 insertions(+), 2 deletions(-) diff --git a/src/lib/AnnotationControls.ts b/src/lib/AnnotationControls.ts index c87125d31..aa2980437 100644 --- a/src/lib/AnnotationControls.ts +++ b/src/lib/AnnotationControls.ts @@ -17,6 +17,7 @@ export enum AnnotationMode { export type ClickHandler = ({ activeControl, event }: { activeControl: AnnotationMode; event: MouseEvent }) => void; export type Options = { onRegionClick?: ClickHandler; + onReset?: () => void; }; declare const __: (key: string) => string; @@ -34,6 +35,10 @@ export default class AnnotationControls { private currentActiveControl: AnnotationMode = AnnotationMode.NONE; + private hasInit = false; + + private onReset: () => void = noop; + /** * [constructor] */ @@ -55,8 +60,12 @@ export default class AnnotationControls { * [destructor] */ public destroy(): void { + if (!this.hasInit) { + return; + } fullscreen.removeListener('enter', this.handleFullscreenEnter); fullscreen.removeListener('exit', this.handleFullscreenExit); + document.removeEventListener('keydown', this.handleKeyDown); } /** @@ -110,6 +119,8 @@ export default class AnnotationControls { this.currentActiveControl = AnnotationMode.NONE; updateButton(); + + this.onReset(); }; /** @@ -145,10 +156,28 @@ export default class AnnotationControls { onClick({ activeControl: this.currentActiveControl, event }); }; + /** + * Escape key handler, reset all control buttons, + * and stop propagation to prevent preview modal from exiting + */ + private handleKeyDown = (event: KeyboardEvent): void => { + if (event.key !== 'Escape' || this.currentActiveControl === AnnotationMode.NONE) { + return; + } + + this.resetControls(); + + event.preventDefault(); + event.stopPropagation(); + }; + /** * Initialize the annotation controls with options. */ - public init({ onRegionClick = noop }: Options = {}): void { + public init({ onRegionClick = noop, onReset = noop }: Options = {}): void { + if (this.hasInit) { + return; + } const groupElement = this.controls.addGroup(CLASS_ANNOTATIONS_GROUP); const regionButton = this.controls.add( __('region_comment'), @@ -160,5 +189,10 @@ export default class AnnotationControls { ); regionButton.setAttribute('data-testid', 'bp-AnnotationsControls-regionBtn'); + + this.onReset = onReset; + document.addEventListener('keydown', this.handleKeyDown); + + this.hasInit = true; } } diff --git a/src/lib/__tests__/AnnotationControls-test.js b/src/lib/__tests__/AnnotationControls-test.js index 47c9b3b50..4b74f3de5 100644 --- a/src/lib/__tests__/AnnotationControls-test.js +++ b/src/lib/__tests__/AnnotationControls-test.js @@ -66,9 +66,13 @@ describe('lib/AnnotationControls', () => { describe('destroy()', () => { it('should remove all listeners', () => { + sandbox.spy(document, 'removeEventListener'); + annotationControls.hasInit = true; + annotationControls.destroy(); expect(stubs.fullscreenRemoveListener).to.be.calledTwice; + expect(document.removeEventListener).to.be.calledWith('keydown', annotationControls.handleKeyDown); }); }); @@ -95,6 +99,66 @@ describe('lib/AnnotationControls', () => { sinon.match.any, ); }); + + it('should add keydown event listener', () => { + sandbox.spy(document, 'addEventListener'); + + annotationControls.init(); + + expect(document.addEventListener).to.be.calledWith('keydown', annotationControls.handleKeyDown); + }); + + it('should set onRest and hasInit', () => { + const onResetMock = sandbox.stub(); + + annotationControls.init({ onReset: onResetMock }); + + expect(annotationControls.onReset).to.equal(onResetMock); + expect(annotationControls.hasInit).to.equal(true); + }); + + it('should early return if hasInit is true', () => { + annotationControls.hasInit = true; + + sandbox.spy(document, 'addEventListener'); + + annotationControls.init(); + + expect(annotationControls.controls.add).not.to.be.called; + expect(document.addEventListener).not.to.be.called; + }); + }); + + describe('handleKeyDown', () => { + let eventMock; + + beforeEach(() => { + annotationControls.resetControls = sandbox.stub(); + annotationControls.currentActiveControl = 'region'; + eventMock = { + key: 'Escape', + preventDefault: sandbox.stub(), + stopPropagation: sandbox.stub(), + }; + }); + + it('should not call resetControls if key is not Escape or mode is none', () => { + annotationControls.handleKeyDown({ key: 'Enter' }); + + expect(annotationControls.resetControls).not.to.be.called; + + annotationControls.currentActiveControl = 'none'; + annotationControls.handleKeyDown({ key: 'Escape' }); + + expect(annotationControls.resetControls).not.to.be.called; + }); + + it('should call resetControls and prevent default event', () => { + annotationControls.handleKeyDown(eventMock); + + expect(eventMock.preventDefault).to.be.called; + expect(eventMock.stopPropagation).to.be.called; + }); }); describe('handleClick()', () => { @@ -139,9 +203,11 @@ describe('lib/AnnotationControls', () => { describe('resetControls()', () => { beforeEach(() => { stubs.updateRegionButton = sandbox.stub(); + stubs.onReset = sandbox.stub(); annotationControls.controlsMap = { [AnnotationMode.REGION]: stubs.updateRegionButton, }; + annotationControls.onReset = stubs.onReset; }); it('should not change if no current active control', () => { @@ -158,6 +224,7 @@ describe('lib/AnnotationControls', () => { expect(annotationControls.currentActiveControl).to.equal(AnnotationMode.NONE); expect(stubs.updateRegionButton).to.be.called; + expect(stubs.onReset).to.be.called; }); }); diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index e75c1e919..e7a680478 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -151,6 +151,7 @@ class BaseViewer extends EventEmitter { this.mobileZoomEndHandler = this.mobileZoomEndHandler.bind(this); this.handleAnnotatorEvents = this.handleAnnotatorEvents.bind(this); this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this); + this.handleAnnotationControlsReset = this.handleAnnotationControlsReset.bind(this); this.handleFullscreenEnter = this.handleFullscreenEnter.bind(this); this.handleFullscreenExit = this.handleFullscreenExit.bind(this); this.handleRegionClick = this.handleRegionClick.bind(this); @@ -1040,6 +1041,16 @@ class BaseViewer extends EventEmitter { return !!permissions && !!permissions.can_view_annotations; } + /** + * Handler for annotation toolbar button reset + * + * @private + * @return {void} + */ + handleAnnotationControlsReset() { + this.annotator.toggleAnnotationMode(AnnotationMode.NONE); + } + /** * Handler for annotation toolbar region comment button click event. * diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 521f14623..ed21c0bed 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -1748,4 +1748,16 @@ describe('lib/viewers/BaseViewer', () => { expect(base.annotator.emit).to.be.calledWith('annotations_active_set', '123'); }); }); + + describe('handleAnnotationControlsReset()', () => { + it('should call toggleAnnotationMode', () => { + base.annotator = { + toggleAnnotationMode: sandbox.stub(), + }; + + base.handleAnnotationControlsReset(); + + expect(base.annotator.toggleAnnotationMode).to.be.calledWith('none'); + }); + }); }); diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index ca536618c..d3fdcacc4 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -1098,7 +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.handleRegionClick }); + this.annotationControls.init({ + onRegionClick: this.handleRegionClick, + onReset: this.handleAnnotationControlsReset, + }); } } diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index a3606f43d..d83363d1c 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -2304,6 +2304,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { ); expect(docBase.annotationControls.init).to.be.calledWith({ onRegionClick: docBase.handleRegionClick, + onReset: docBase.handleAnnotationControlsReset, }); }); From bfe5321a6ab1e9f7739beb55ebbcd343a75a58c6 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Thu, 18 Jun 2020 14:11:45 -0700 Subject: [PATCH 2/3] feat(annotations): Add reset to ImageViewer --- src/lib/viewers/image/ImageViewer.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/viewers/image/ImageViewer.js b/src/lib/viewers/image/ImageViewer.js index 5d4b4ee52..4f637a6ca 100644 --- a/src/lib/viewers/image/ImageViewer.js +++ b/src/lib/viewers/image/ImageViewer.js @@ -288,7 +288,10 @@ class ImageViewer extends ImageBaseViewer { if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { this.annotationControls = new AnnotationControls(this.controls); - this.annotationControls.init({ onRegionClick: this.handleRegionClick }); + this.annotationControls.init({ + onRegionClick: this.handleRegionClick, + onReset: this.handleAnnotationControlsReset, + }); } } From c7d580048fabc392ef6250a4d3124e979850b72b Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Thu, 18 Jun 2020 16:12:40 -0700 Subject: [PATCH 3/3] feat(annotations): Address comments --- src/lib/AnnotationControls.ts | 2 ++ src/lib/__tests__/AnnotationControls-test.js | 10 ++++++++++ src/lib/viewers/image/__tests__/ImageViewer-test.js | 13 +++++++++++++ 3 files changed, 25 insertions(+) diff --git a/src/lib/AnnotationControls.ts b/src/lib/AnnotationControls.ts index aa2980437..252abfb96 100644 --- a/src/lib/AnnotationControls.ts +++ b/src/lib/AnnotationControls.ts @@ -66,6 +66,8 @@ export default class AnnotationControls { fullscreen.removeListener('enter', this.handleFullscreenEnter); fullscreen.removeListener('exit', this.handleFullscreenExit); document.removeEventListener('keydown', this.handleKeyDown); + + this.hasInit = false; } /** diff --git a/src/lib/__tests__/AnnotationControls-test.js b/src/lib/__tests__/AnnotationControls-test.js index 4b74f3de5..100370151 100644 --- a/src/lib/__tests__/AnnotationControls-test.js +++ b/src/lib/__tests__/AnnotationControls-test.js @@ -73,6 +73,16 @@ describe('lib/AnnotationControls', () => { expect(stubs.fullscreenRemoveListener).to.be.calledTwice; expect(document.removeEventListener).to.be.calledWith('keydown', annotationControls.handleKeyDown); + expect(annotationControls.hasInit).to.equal(false); + }); + + it('should early return if hasInit is false', () => { + sandbox.spy(document, 'removeEventListener'); + + annotationControls.destroy(); + + expect(stubs.fullscreenRemoveListener).not.to.be.called; + expect(document.removeEventListener).not.to.be.called; }); }); diff --git a/src/lib/viewers/image/__tests__/ImageViewer-test.js b/src/lib/viewers/image/__tests__/ImageViewer-test.js index cb277de63..e534ab3a7 100644 --- a/src/lib/viewers/image/__tests__/ImageViewer-test.js +++ b/src/lib/viewers/image/__tests__/ImageViewer-test.js @@ -362,6 +362,19 @@ describe('lib/viewers/image/ImageViewer', () => { image.loadUI(); expect(image.annotationControls instanceof AnnotationControls).to.be.true; }); + + it('should call annotations controls init with callbacks', () => { + sandbox.stub(image, 'areNewAnnotationsEnabled').returns(true); + sandbox.stub(image, 'hasAnnotationCreatePermission').returns(true); + sandbox.stub(AnnotationControls.prototype, 'init').callsFake(); + + image.loadUI(); + + expect(AnnotationControls.prototype.init).to.be.calledWith({ + onRegionClick: image.handleRegionClick, + onReset: image.handleAnnotationControlsReset, + }); + }); }); describe('isRotated()', () => {