From a95c78d82a7b7fabdd5db8b5c75f6b5bbe678e2a Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Mon, 22 Jun 2020 13:54:50 -0700 Subject: [PATCH] fix(annotations): Address comments --- src/lib/AnnotationControls.ts | 16 ++++++---------- src/lib/__tests__/AnnotationControls-test.js | 10 +--------- src/lib/viewers/doc/DocBaseViewer.js | 2 +- .../viewers/doc/__tests__/DocBaseViewer-test.js | 2 +- src/lib/viewers/image/ImageViewer.js | 2 +- .../viewers/image/__tests__/ImageViewer-test.js | 2 +- 6 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/lib/AnnotationControls.ts b/src/lib/AnnotationControls.ts index aa3ce69db4..2a8f89380f 100644 --- a/src/lib/AnnotationControls.ts +++ b/src/lib/AnnotationControls.ts @@ -14,10 +14,10 @@ export enum AnnotationMode { NONE = 'none', REGION = 'region', } -export type ClickHandler = ({ activeControl, event }: { activeControl: AnnotationMode; event: MouseEvent }) => void; +export type ClickHandler = ({ event }: { event: MouseEvent }) => void; export type Options = { - onRegionClick?: ClickHandler; onEscape?: () => void; + onRegionClick?: ClickHandler; }; declare const __: (key: string) => string; @@ -105,10 +105,6 @@ export default class AnnotationControls { */ private handleFullscreenExit = (): void => this.handleFullscreenChange(false); - public getActiveMode = (): AnnotationMode => { - return this.currentMode; - }; - /** * Deactivate current control button */ @@ -144,16 +140,16 @@ export default class AnnotationControls { * Region comment button click handler */ private handleClick = (onClick: ClickHandler, mode: AnnotationMode) => (event: MouseEvent): void => { - const prevActiveControl = this.currentMode; + const prevMode = this.currentMode; this.resetControls(); - if (prevActiveControl !== mode) { + if (prevMode !== mode) { this.currentMode = mode as AnnotationMode; this.controlsMap[mode](); } - onClick({ activeControl: this.currentMode, event }); + onClick({ event }); }; /** @@ -175,7 +171,7 @@ export default class AnnotationControls { /** * Initialize the annotation controls with options. */ - public init({ onRegionClick = noop, onEscape = noop }: Options = {}): void { + public init({ onEscape = noop, onRegionClick = noop }: Options = {}): void { if (this.hasInit) { return; } diff --git a/src/lib/__tests__/AnnotationControls-test.js b/src/lib/__tests__/AnnotationControls-test.js index 66051d5c65..39db6b1dd4 100644 --- a/src/lib/__tests__/AnnotationControls-test.js +++ b/src/lib/__tests__/AnnotationControls-test.js @@ -15,7 +15,7 @@ let stubs = {}; const sandbox = sinon.sandbox.create(); -describe('lib/AnnotationControls', () => { +describe.only('lib/AnnotationControls', () => { before(() => { fixture.setBase('src/lib'); }); @@ -192,7 +192,6 @@ describe('lib/AnnotationControls', () => { annotationControls.handleClick(stubs.onRegionClick, AnnotationMode.REGION)(stubs.event); expect(stubs.onRegionClick).to.be.calledWith({ - activeControl: AnnotationMode.REGION, event: stubs.event, }); }); @@ -235,11 +234,4 @@ describe('lib/AnnotationControls', () => { expect(stubs.updateRegionButton).to.be.called; }); }); - - describe('getActiveMode()', () => { - it('should return the current active mode', () => { - annotationControls.currentMode = 'region'; - expect(annotationControls.getActiveMode()).to.equal('region'); - }); - }); }); diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 11ee20cb32..29cb579427 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -1099,8 +1099,8 @@ class DocBaseViewer extends BaseViewer { if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { this.annotationControls.init({ - onRegionClick: this.handleRegionClick, onEscape: this.handleAnnotationControlsEscape, + onRegionClick: this.handleRegionClick, }); } } diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index d4323fa9dc..9187095a68 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -2303,8 +2303,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { ICON_FULLSCREEN_OUT, ); expect(docBase.annotationControls.init).to.be.calledWith({ - onRegionClick: docBase.handleRegionClick, onEscape: docBase.handleAnnotationControlsEscape, + onRegionClick: docBase.handleRegionClick, }); }); diff --git a/src/lib/viewers/image/ImageViewer.js b/src/lib/viewers/image/ImageViewer.js index ce292ac45b..0cce9c2529 100644 --- a/src/lib/viewers/image/ImageViewer.js +++ b/src/lib/viewers/image/ImageViewer.js @@ -289,8 +289,8 @@ class ImageViewer extends ImageBaseViewer { if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { this.annotationControls = new AnnotationControls(this.controls); this.annotationControls.init({ - onRegionClick: this.handleRegionClick, onEscape: this.handleAnnotationControlsEscape, + onRegionClick: this.handleRegionClick, }); } } diff --git a/src/lib/viewers/image/__tests__/ImageViewer-test.js b/src/lib/viewers/image/__tests__/ImageViewer-test.js index 86a4f64410..8b559db47b 100644 --- a/src/lib/viewers/image/__tests__/ImageViewer-test.js +++ b/src/lib/viewers/image/__tests__/ImageViewer-test.js @@ -371,8 +371,8 @@ describe('lib/viewers/image/ImageViewer', () => { image.loadUI(); expect(AnnotationControls.prototype.init).to.be.calledWith({ - onRegionClick: image.handleRegionClick, onEscape: image.handleAnnotationControlsEscape, + onRegionClick: image.handleRegionClick, }); }); });