Skip to content

Commit

Permalink
feat(annotations): Set region as default mode if discover FF is on (#…
Browse files Browse the repository at this point in the history
…1255)

* feat(annotations): Set region as default mode if discover FF is on

* feat(annotator): Address comments
  • Loading branch information
Mingze committed Sep 15, 2020
1 parent 46aad4f commit 9311be8
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,8 @@ class Preview extends EventEmitter {

this.options.showAnnotationsHighlightText = !!options.showAnnotationsHighlightText;

this.options.enableAnnotationsDiscoverability = !!options.enableAnnotationsDiscoverability;

// Enable or disable hotkeys
this.options.useHotkeys = options.useHotkeys !== false;

Expand Down
7 changes: 6 additions & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,7 @@ class BaseViewer extends EventEmitter {
const annotatorOptions = this.createAnnotatorOptions({
annotator: this.annotatorConf,
features: options && options.features,
initialMode: this.options.enableAnnotationsDiscoverability ? AnnotationMode.REGION : AnnotationMode.NONE,
intl: (options && options.intl) || intlUtil.createAnnotatorIntl(),
modeButtons: ANNOTATION_BUTTONS,
});
Expand Down Expand Up @@ -1082,7 +1083,11 @@ class BaseViewer extends EventEmitter {
*/
handleAnnotationControlsClick({ mode }) {
const nextMode = this.annotationControlsFSM.transition(AnnotationInput.CLICK, mode);
this.annotator.toggleAnnotationMode(nextMode);
this.annotator.toggleAnnotationMode(
this.options.enableAnnotationsDiscoverability && nextMode === AnnotationMode.NONE
? AnnotationMode.REGION
: nextMode,
);
this.annotationControls.setMode(nextMode);
}

Expand Down
48 changes: 39 additions & 9 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import intl from '../../i18n';
import * as util from '../../util';
import * as icons from '../../icons/icons';
import * as constants from '../../constants';
import { AnnotationInput } from '../../AnnotationControlsFSM';
import { AnnotationMode } from '../../AnnotationControls';
import { EXCLUDED_EXTENSIONS } from '../../extensions';
import { VIEWER_EVENT, LOAD_METRIC, ERROR_CODE } from '../../events';
import { AnnotationMode } from '../../AnnotationControls';
import Timer from '../../Timer';
import Api from '../../api';

Expand Down Expand Up @@ -1113,6 +1114,7 @@ describe('lib/viewers/BaseViewer', () => {
describe('createAnnotator()', () => {
const annotatorMock = {};
const annotationsOptions = {
initialMode: AnnotationMode.NONE,
intl: {
language: 'en-US',
locale: 'en-US',
Expand Down Expand Up @@ -1203,6 +1205,20 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.createAnnotatorOptions).to.be.calledWith(sinon.match(annotationsOptions));
});

it('should create annotator with initial mode region if discoverability is enabled', () => {
sandbox.stub(base, 'areAnnotationsEnabled').returns(true);
sandbox.stub(base, 'createAnnotatorOptions');

base.options.boxAnnotations = {
determineAnnotator: sandbox.stub().returns(conf),
};
base.options.enableAnnotationsDiscoverability = true;

base.createAnnotator();

expect(base.createAnnotatorOptions).to.be.calledWith(sinon.match({ initialMode: AnnotationMode.REGION }));
});

it('should emit annotator_create event', () => {
sandbox.stub(base, 'areAnnotationsEnabled').returns(true);

Expand Down Expand Up @@ -1808,7 +1824,7 @@ describe('lib/viewers/BaseViewer', () => {
base.handleAnnotationCreateEvent(event);

expect(base.annotator.emit).to.be.calledWith('annotations_active_set', '123');
expect(base.annotationControls.setMode).to.be.calledWith('none');
expect(base.annotationControls.setMode).to.be.calledWith(AnnotationMode.NONE);
});
});

Expand All @@ -1818,9 +1834,9 @@ describe('lib/viewers/BaseViewer', () => {
destroy: sandbox.stub(),
setMode: sandbox.stub(),
};
base.handleAnnotationCreatorChangeEvent({ status: 'create', type: 'highlight' });
base.handleAnnotationCreatorChangeEvent({ status: AnnotationInput.CREATE, type: AnnotationMode.HIGHLIGHT });

expect(base.annotationControls.setMode).to.be.calledWith('highlight');
expect(base.annotationControls.setMode).to.be.calledWith(AnnotationMode.HIGHLIGHT);
});
});

Expand All @@ -1832,7 +1848,7 @@ describe('lib/viewers/BaseViewer', () => {

base.handleAnnotationControlsEscape();

expect(base.annotator.toggleAnnotationMode).to.be.calledWith('none');
expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.NONE);
});
});

Expand All @@ -1844,15 +1860,29 @@ describe('lib/viewers/BaseViewer', () => {
};
});

it('should call toggleAnnotationMode', () => {
it('should call toggleAnnotationMode and setMode', () => {
base.annotator = {
toggleAnnotationMode: sandbox.stub(),
};

base.handleAnnotationControlsClick({ mode: 'region' });
base.handleAnnotationControlsClick({ mode: AnnotationMode.REGION });

expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.REGION);
expect(base.annotationControls.setMode).to.be.calledWith(AnnotationMode.REGION);
});

it('should call toggleAnnotationMode with appropriate mode if discoverability is enabled', () => {
base.annotator = {
toggleAnnotationMode: sandbox.stub(),
};

base.options.enableAnnotationsDiscoverability = false;
base.handleAnnotationControlsClick({ mode: AnnotationMode.NONE });
expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.NONE);

expect(base.annotator.toggleAnnotationMode).to.be.calledWith('region');
expect(base.annotationControls.setMode).to.be.calledWith('region');
base.options.enableAnnotationsDiscoverability = true;
base.handleAnnotationControlsClick({ mode: AnnotationMode.NONE });
expect(base.annotator.toggleAnnotationMode).to.be.calledWith(AnnotationMode.REGION);
});
});
});

0 comments on commit 9311be8

Please sign in to comment.