Skip to content

Commit

Permalink
fix(discoverability): set mode to REGION on escape key press (#1263)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@ChenCodes @mergify
ChenCodes and mergify[bot] authored Sep 24, 2020
1 parent 9a81ad3 commit e72a7c7
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 16 deletions.
6 changes: 6 additions & 0 deletions src/lib/AnnotationControlsFSM.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export enum AnnotationInput {
CANCEL = 'cancel',
CLICK = 'click',
CREATE = 'create',
RESET = 'reset',
STARTED = 'started',
SUCCESS = 'success',
UPDATE = 'update',
Expand Down Expand Up @@ -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) {
Expand Down
87 changes: 74 additions & 13 deletions src/lib/__tests__/AnnotationControlsFSM-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});
});
Expand All @@ -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);
});
});
Expand Down Expand Up @@ -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);
Expand All @@ -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);
});
});
Expand All @@ -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);
});
});
Expand Down
8 changes: 7 additions & 1 deletion src/lib/viewers/BaseViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down
20 changes: 18 additions & 2 deletions src/lib/viewers/__tests__/BaseViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});

Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit e72a7c7

Please sign in to comment.