From e72a7c784b205a8ee07479e1d29a2f5f8403dfee Mon Sep 17 00:00:00 2001 From: Jack Chen Date: Thu, 24 Sep 2020 09:57:26 -0700 Subject: [PATCH] fix(discoverability): set mode to REGION on escape key press (#1263) * fix(discoverability): set mode to REGION on escape key press * fix(discoverability): add reset function to AnnotationControlsFSM * fix(discoverability): call reset method and processAnnotationModeChange * fix(discoverability): refactor reset method as part of transition method * fix(discoverability): update tests * fix(discoverability): test NONE -> NONE path Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- src/lib/AnnotationControlsFSM.ts | 6 ++ .../__tests__/AnnotationControlsFSM-test.js | 87 ++++++++++++++++--- src/lib/viewers/BaseViewer.js | 8 +- src/lib/viewers/__tests__/BaseViewer-test.js | 20 ++++- 4 files changed, 105 insertions(+), 16 deletions(-) diff --git a/src/lib/AnnotationControlsFSM.ts b/src/lib/AnnotationControlsFSM.ts index bacf17608..e5c043dea 100644 --- a/src/lib/AnnotationControlsFSM.ts +++ b/src/lib/AnnotationControlsFSM.ts @@ -12,6 +12,7 @@ export enum AnnotationInput { CANCEL = 'cancel', CLICK = 'click', CREATE = 'create', + RESET = 'reset', STARTED = 'started', SUCCESS = 'success', UPDATE = 'update', @@ -52,6 +53,11 @@ export default class AnnotationControlsFSM { return stateModeMap[this.currentState]; } + if (input === AnnotationInput.RESET) { + this.currentState = AnnotationState.NONE; + return stateModeMap[this.currentState]; + } + switch (this.currentState) { case AnnotationState.NONE: if (input === AnnotationInput.CREATE || input === AnnotationInput.STARTED) { diff --git a/src/lib/__tests__/AnnotationControlsFSM-test.js b/src/lib/__tests__/AnnotationControlsFSM-test.js index 4a08a0cf8..8beb6891d 100644 --- a/src/lib/__tests__/AnnotationControlsFSM-test.js +++ b/src/lib/__tests__/AnnotationControlsFSM-test.js @@ -59,13 +59,21 @@ describe('lib/AnnotationControlsFSM', () => { expect(annotationControlsFSM.getState()).to.equal(AnnotationState.NONE); }); }); + + // Should reset state + it('should reset state if input is AnnotationInput.RESET', () => { + const annotationControlsFSM = new AnnotationControlsFSM(); + + expect(annotationControlsFSM.transition(AnnotationInput.RESET)).to.equal(AnnotationMode.NONE); + expect(annotationControlsFSM.getState()).to.equal(AnnotationState.NONE); + }); }); describe('AnnotationState.HIGHLIGHT/REGION', () => { // Stay in the same state [AnnotationState.HIGHLIGHT, AnnotationState.REGION].forEach(state => { Object.values(AnnotationInput) - .filter(input => input !== AnnotationInput.CLICK) + .filter(input => input !== AnnotationInput.CLICK && input !== AnnotationInput.RESET) .forEach(input => { it(`should stay in state ${state} if input is ${input}`, () => { const annotationControlsFSM = new AnnotationControlsFSM(state); @@ -80,18 +88,30 @@ describe('lib/AnnotationControlsFSM', () => { describe('AnnotationState.HIGHLIGHT', () => { [ { + input: AnnotationInput.CLICK, mode: AnnotationMode.HIGHLIGHT, output: AnnotationMode.NONE, }, { + input: AnnotationInput.CLICK, mode: AnnotationMode.REGION, output: AnnotationMode.REGION, }, - ].forEach(({ mode, output }) => { - it(`should output ${output} if input is click and mode is ${mode}`, () => { + { + input: AnnotationInput.RESET, + mode: AnnotationMode.HIGHLIGHT, + output: AnnotationMode.NONE, + }, + { + input: AnnotationInput.RESET, + mode: AnnotationMode.REGION, + output: AnnotationMode.NONE, + }, + ].forEach(({ input, mode, output }) => { + it(`should output ${output} if input is ${input} and mode is ${mode}`, () => { const annotationControlsFSM = new AnnotationControlsFSM(AnnotationState.HIGHLIGHT); - expect(annotationControlsFSM.transition(AnnotationInput.CLICK, mode)).to.equal(output); + expect(annotationControlsFSM.transition(input, mode)).to.equal(output); expect(annotationControlsFSM.getState()).to.equal(output); }); }); @@ -101,18 +121,30 @@ describe('lib/AnnotationControlsFSM', () => { describe('AnnotationState.REGION', () => { [ { + input: AnnotationInput.CLICK, mode: AnnotationMode.REGION, output: AnnotationMode.NONE, }, { + input: AnnotationInput.CLICK, mode: AnnotationMode.HIGHLIGHT, output: AnnotationMode.HIGHLIGHT, }, - ].forEach(({ mode, output }) => { - it(`should output ${output} if input is click and mode is ${mode}`, () => { + { + input: AnnotationInput.RESET, + mode: AnnotationMode.REGION, + output: AnnotationMode.NONE, + }, + { + input: AnnotationInput.RESET, + mode: AnnotationMode.HIGHLIGHT, + output: AnnotationMode.NONE, + }, + ].forEach(({ input, mode, output }) => { + it(`should output ${output} if input is ${input} and mode is ${mode}`, () => { const annotationControlsFSM = new AnnotationControlsFSM(AnnotationState.REGION); - expect(annotationControlsFSM.transition(AnnotationInput.CLICK, mode)).to.equal(output); + expect(annotationControlsFSM.transition(input, mode)).to.equal(output); expect(annotationControlsFSM.getState()).to.equal(output); }); }); @@ -142,6 +174,11 @@ describe('lib/AnnotationControlsFSM', () => { nextState: AnnotationState.NONE, output: AnnotationMode.NONE, }, + { + input: AnnotationInput.RESET, + nextState: AnnotationState.NONE, + output: AnnotationMode.NONE, + }, ].forEach(({ input, nextState, output }) => { it(`should go to state ${nextState} and output ${output} if input is ${input}`, () => { const annotationControlsFSM = new AnnotationControlsFSM(state); @@ -166,18 +203,30 @@ describe('lib/AnnotationControlsFSM', () => { describe('AnnotationState.HIGHLIGHT_TEMP', () => { [ { + input: AnnotationInput.CLICK, mode: AnnotationMode.HIGHLIGHT, output: AnnotationMode.NONE, }, { + input: AnnotationInput.CLICK, mode: AnnotationMode.REGION, output: AnnotationMode.REGION, }, - ].forEach(({ mode, output }) => { - it(`should output ${output} if input is click and mode is ${mode}`, () => { + { + input: AnnotationInput.RESET, + mode: AnnotationMode.HIGHLIGHT, + output: AnnotationMode.NONE, + }, + { + input: AnnotationInput.RESET, + mode: AnnotationMode.REGION, + output: AnnotationMode.NONE, + }, + ].forEach(({ input, mode, output }) => { + it(`should output ${output} if input is ${input} and mode is ${mode}`, () => { const annotationControlsFSM = new AnnotationControlsFSM(AnnotationState.HIGHLIGHT_TEMP); - expect(annotationControlsFSM.transition(AnnotationInput.CLICK, mode)).to.equal(output); + expect(annotationControlsFSM.transition(input, mode)).to.equal(output); expect(annotationControlsFSM.getState()).to.equal(output); }); }); @@ -187,18 +236,30 @@ describe('lib/AnnotationControlsFSM', () => { describe('AnnotationState.REGION_TEMP', () => { [ { + input: AnnotationInput.CLICK, mode: AnnotationMode.REGION, output: AnnotationMode.NONE, }, { + input: AnnotationInput.CLICK, mode: AnnotationMode.HIGHLIGHT, output: AnnotationMode.HIGHLIGHT, }, - ].forEach(({ mode, output }) => { - it(`should output ${output} if input is click and mode is ${mode}`, () => { + { + input: AnnotationInput.RESET, + mode: AnnotationMode.REGION, + output: AnnotationMode.NONE, + }, + { + input: AnnotationInput.RESET, + mode: AnnotationMode.HIGHLIGHT, + output: AnnotationMode.NONE, + }, + ].forEach(({ input, mode, output }) => { + it(`should output ${output} if input is ${input} and mode is ${mode}`, () => { const annotationControlsFSM = new AnnotationControlsFSM(AnnotationState.REGION_TEMP); - expect(annotationControlsFSM.transition(AnnotationInput.CLICK, mode)).to.equal(output); + expect(annotationControlsFSM.transition(input, mode)).to.equal(output); expect(annotationControlsFSM.getState()).to.equal(output); }); }); diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 598f49043..f41a2b685 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -595,6 +595,7 @@ class BaseViewer extends EventEmitter { this.annotator.emit(ANNOTATOR_EVENT.setVisibility, true); if (this.options.enableAnnotationsDiscoverability) { this.annotator.toggleAnnotationMode(AnnotationMode.REGION); + this.processAnnotationModeChange(this.annotationControlsFSM.transition(AnnotationInput.RESET)); } this.enableAnnotationControls(); } @@ -1087,7 +1088,12 @@ class BaseViewer extends EventEmitter { * @return {void} */ handleAnnotationControlsEscape() { - this.annotator.toggleAnnotationMode(AnnotationMode.NONE); + if (this.options.enableAnnotationsDiscoverability) { + this.annotator.toggleAnnotationMode(AnnotationMode.REGION); + this.processAnnotationModeChange(this.annotationControlsFSM.transition(AnnotationInput.RESET)); + } else { + this.annotator.toggleAnnotationMode(AnnotationMode.NONE); + } } /** diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 221048200..6371aa16a 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -600,12 +600,14 @@ describe('lib/viewers/BaseViewer', () => { destroy: sandbox.mock(), }; base.options.enableAnnotationsDiscoverability = true; + base.processAnnotationModeChange = sandbox.mock(); base.handleFullscreenExit(); expect(base.annotator.emit).to.be.calledWith(ANNOTATOR_EVENT.setVisibility, true); - expect(base.enableAnnotationControls).to.be.called; expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.REGION); + expect(base.processAnnotationModeChange).to.be.calledWith(AnnotationMode.NONE); + expect(base.enableAnnotationControls).to.be.called; }); }); @@ -1875,15 +1877,29 @@ describe('lib/viewers/BaseViewer', () => { }); describe('handleAnnotationControlsEscape()', () => { - it('should call toggleAnnotationMode', () => { + it('should call toggleAnnotationMode with AnnotationMode.NONE if enableAnnotationsDiscoverability is false', () => { base.annotator = { toggleAnnotationMode: sandbox.stub(), }; + base.options.enableAnnotationsDiscoverability = false; base.handleAnnotationControlsEscape(); expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.NONE); }); + + it('should reset annotationControlsFSM state and call toggleAnnotationMode with AnnotationMode.REGION if enableAnnotationsDiscoverability is true', () => { + base.annotator = { + toggleAnnotationMode: sandbox.stub(), + }; + base.options.enableAnnotationsDiscoverability = true; + base.processAnnotationModeChange = sandbox.stub(); + + base.handleAnnotationControlsEscape(); + + expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.REGION); + expect(base.processAnnotationModeChange).to.be.calledWith(AnnotationMode.NONE); + }); }); describe('handleAnnotationControlsClick', () => {