Skip to content

Commit

Permalink
fix(annotations): move discoverability changes into DocBaseViewer (#1271
Browse files Browse the repository at this point in the history 
)

* fix(annotations): move discoverability changes into DocBaseViewer

* chore: add unit tests

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@mergify
Conrad Chan and mergify[bot] authored Oct 19, 2020
1 parent ce69954 commit cb26355
Show file tree
Hide file tree
Showing 6 changed files with 327 additions and 201 deletions.
58 changes: 7 additions & 51 deletions src/lib/viewers/BaseViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,7 @@ class BaseViewer extends EventEmitter {
this.mobileZoomChangeHandler = this.mobileZoomChangeHandler.bind(this);
this.mobileZoomEndHandler = this.mobileZoomEndHandler.bind(this);
this.handleAnnotatorEvents = this.handleAnnotatorEvents.bind(this);
this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this);
this.handleAnnotationControlsClick = this.handleAnnotationControlsClick.bind(this);
this.handleAnnotationControlsEscape = this.handleAnnotationControlsEscape.bind(this);
this.handleAnnotationCreatorChangeEvent = this.handleAnnotationCreatorChangeEvent.bind(this);
this.handleFullscreenEnter = this.handleFullscreenEnter.bind(this);
this.handleFullscreenExit = this.handleFullscreenExit.bind(this);
this.createAnnotator = this.createAnnotator.bind(this);
Expand Down Expand Up @@ -593,10 +590,6 @@ class BaseViewer extends EventEmitter {

if (this.annotator && this.areNewAnnotationsEnabled()) {
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 @@ -989,7 +982,7 @@ class BaseViewer extends EventEmitter {
const annotatorOptions = this.createAnnotatorOptions({
annotator: this.annotatorConf,
features: options && options.features,
initialMode: this.options.enableAnnotationsDiscoverability ? AnnotationMode.REGION : AnnotationMode.NONE,
initialMode: this.getInitialAnnotationMode(),
intl: (options && options.intl) || intlUtil.createAnnotatorIntl(),
modeButtons: ANNOTATION_BUTTONS,
});
Expand All @@ -1003,6 +996,10 @@ class BaseViewer extends EventEmitter {
}
}

getInitialAnnotationMode() {
return AnnotationMode.NONE;
}

/**
* Initializes annotations.
*
Expand Down Expand Up @@ -1033,12 +1030,6 @@ class BaseViewer extends EventEmitter {

// Add a custom listener for events emmited by the annotator
this.annotator.addListener('annotatorevent', this.handleAnnotatorEvents);

if (this.areNewAnnotationsEnabled() && this.annotationControls) {
this.annotator.addListener('annotations_create', this.handleAnnotationCreateEvent);
this.annotator.addListener('creator_staged_change', this.handleAnnotationCreatorChangeEvent);
this.annotator.addListener('creator_status_change', this.handleAnnotationCreatorChangeEvent);
}
}

/**
Expand Down Expand Up @@ -1088,12 +1079,8 @@ class BaseViewer extends EventEmitter {
* @return {void}
*/
handleAnnotationControlsEscape() {
if (this.options.enableAnnotationsDiscoverability) {
this.annotator.toggleAnnotationMode(AnnotationMode.REGION);
this.processAnnotationModeChange(this.annotationControlsFSM.transition(AnnotationInput.RESET));
} else {
this.annotator.toggleAnnotationMode(AnnotationMode.NONE);
}
this.processAnnotationModeChange(this.annotationControlsFSM.transition(AnnotationInput.RESET));
this.annotator.toggleAnnotationMode(AnnotationMode.NONE);
}

/**
Expand Down Expand Up @@ -1122,23 +1109,6 @@ class BaseViewer extends EventEmitter {
}
};

/**
* Handler for annotation controls button click event.
*
* @private
* @param {AnnotationMode} mode one of annotation modes
* @return {void}
*/
handleAnnotationControlsClick({ mode }) {
const nextMode = this.annotationControlsFSM.transition(AnnotationInput.CLICK, mode);
this.annotator.toggleAnnotationMode(
this.options.enableAnnotationsDiscoverability && nextMode === AnnotationMode.NONE
? AnnotationMode.REGION
: nextMode,
);
this.processAnnotationModeChange(nextMode);
}

/**
* Handles the 'scrolltoannotation' event and calls the annotator scroll method
* @param {string | Object} event - Annotation Event
Expand Down Expand Up @@ -1303,20 +1273,6 @@ class BaseViewer extends EventEmitter {
this.emit('annotatorevent', data);
}

handleAnnotationCreateEvent({ annotation: { id } = {}, meta: { status } = {} }) {
// Only on success do we exit create annotation mode. If error occurs,
// we remain in create mode
if (status === 'success') {
this.annotator.emit('annotations_active_set', id);

this.processAnnotationModeChange(this.annotationControlsFSM.transition(AnnotationInput.SUCCESS));
}
}

handleAnnotationCreatorChangeEvent({ status, type }) {
this.processAnnotationModeChange(this.annotationControlsFSM.transition(status, type));
}

/**
* Creates combined options to give to the annotator
*
Expand Down
152 changes: 7 additions & 145 deletions src/lib/viewers/__tests__/BaseViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ 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';
Expand Down Expand Up @@ -574,28 +573,6 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.annotator.emit).toBeCalledWith(ANNOTATOR_EVENT.setVisibility, true);
expect(base.enableAnnotationControls).toBeCalled();
});

test(`should show annotations and toggle annotations mode to REGION if enableAnnotationsDiscoverability is true`, () => {
jest.spyOn(base, 'areNewAnnotationsEnabled').mockReturnValue(true);
jest.spyOn(base, 'enableAnnotationControls').mockImplementation();

base.annotator = {
emit: jest.fn(),
toggleAnnotationMode: jest.fn(),
};
base.annotationControls = {
destroy: jest.fn(),
};
base.options.enableAnnotationsDiscoverability = true;
base.processAnnotationModeChange = jest.fn();

base.handleFullscreenExit();

expect(base.annotator.emit).toBeCalledWith(ANNOTATOR_EVENT.setVisibility, true);
expect(base.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.REGION);
expect(base.processAnnotationModeChange).toBeCalledWith(AnnotationMode.NONE);
expect(base.enableAnnotationControls).toBeCalled();
});
});

describe('resize()', () => {
Expand Down Expand Up @@ -1227,22 +1204,6 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.createAnnotatorOptions).toBeCalledWith(expect.objectContaining(annotationsOptions));
});

test('should create annotator with initial mode region if discoverability is enabled', () => {
jest.spyOn(base, 'areAnnotationsEnabled').mockReturnValue(true);
jest.spyOn(base, 'createAnnotatorOptions').mockImplementation();

base.options.boxAnnotations = {
determineAnnotator: jest.fn().mockReturnValue(conf),
};
base.options.enableAnnotationsDiscoverability = true;

base.createAnnotator();

expect(base.createAnnotatorOptions).toBeCalledWith(
expect.objectContaining({ initialMode: AnnotationMode.REGION }),
);
});

test('should emit annotator_create event', () => {
jest.spyOn(base, 'areAnnotationsEnabled').mockReturnValue(true);

Expand Down Expand Up @@ -1291,20 +1252,6 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.addListener).toBeCalledWith('scale', expect.any(Function));
expect(base.addListener).toBeCalledWith('scrolltoannotation', base.handleScrollToAnnotation);
expect(base.annotator.addListener).toBeCalledWith('annotatorevent', expect.any(Function));
expect(base.annotator.addListener).toBeCalledWith('annotations_create', base.handleAnnotationCreateEvent);
expect(base.annotator.addListener).toBeCalledWith(
'annotations_initialized',
base.handleAnnotationsInitialized,
);
expect(base.annotator.addListener).toBeCalledWith(
'creator_staged_change',
base.handleAnnotationCreatorChangeEvent,
);
expect(base.annotator.addListener).toBeCalledWith(
'creator_status_change',
base.handleAnnotationCreatorChangeEvent,
);
expect(base.emit).toBeCalledWith('annotator', base.annotator);
});

test('should call the correct handler to toggle annotation modes', () => {
Expand Down Expand Up @@ -1805,107 +1752,16 @@ describe('lib/viewers/BaseViewer', () => {
});
});

describe('handleAnnotationCreateEvent()', () => {
beforeEach(() => {
base.annotator = {
emit: jest.fn(),
};
base.annotationControls = {
destroy: jest.fn(),
setMode: jest.fn(),
};
base.processAnnotationModeChange = jest.fn();
});

const createEvent = status => ({
annotation: { id: '123' },
meta: {
status,
},
});

['error', 'pending'].forEach(status => {
test(`should not do anything if status is ${status}`, () => {
const event = createEvent(status);
base.handleAnnotationCreateEvent(event);

expect(base.annotator.emit).not.toBeCalled();
});
});

test('should reset controls if status is success', () => {
const event = createEvent('success');
base.handleAnnotationCreateEvent(event);

expect(base.annotator.emit).toBeCalledWith('annotations_active_set', '123');
expect(base.processAnnotationModeChange).toBeCalledWith(AnnotationMode.NONE);
});
});

describe('handleAnnotationCreatorChangeEvent()', () => {
test('should set mode', () => {
base.annotationControls = {
destroy: jest.fn(),
setMode: jest.fn(),
};

base.processAnnotationModeChange = jest.fn();
base.handleAnnotationCreatorChangeEvent({ status: AnnotationInput.CREATE, type: AnnotationMode.HIGHLIGHT });

expect(base.processAnnotationModeChange).toBeCalledWith(AnnotationMode.HIGHLIGHT);
});
});

describe('handleAnnotationControlsEscape()', () => {
test('should call toggleAnnotationMode with AnnotationMode.NONE if enableAnnotationsDiscoverability is false', () => {
test('should call toggleAnnotationMode with AnnotationMode.NONE', () => {
base.annotator = {
toggleAnnotationMode: jest.fn(),
};
base.options.enableAnnotationsDiscoverability = false;

base.handleAnnotationControlsEscape();

expect(base.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.NONE);
});

test('should reset annotationControlsFSM state and call toggleAnnotationMode with AnnotationMode.REGION if enableAnnotationsDiscoverability is true', () => {
base.annotator = {
toggleAnnotationMode: jest.fn(),
};
base.options.enableAnnotationsDiscoverability = true;
base.processAnnotationModeChange = jest.fn();

base.handleAnnotationControlsEscape();

expect(base.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.REGION);
expect(base.processAnnotationModeChange).toBeCalledWith(AnnotationMode.NONE);
});
});

describe('handleAnnotationControlsClick', () => {
beforeEach(() => {
base.annotator = {
toggleAnnotationMode: jest.fn(),
};
base.processAnnotationModeChange = jest.fn();
});

test('should call toggleAnnotationMode and processAnnotationModeChange', () => {
base.handleAnnotationControlsClick({ mode: AnnotationMode.REGION });

expect(base.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.REGION);
expect(base.processAnnotationModeChange).toBeCalledWith(AnnotationMode.REGION);
});

test('should call toggleAnnotationMode with appropriate mode if discoverability is enabled', () => {
base.options.enableAnnotationsDiscoverability = false;
base.handleAnnotationControlsClick({ mode: AnnotationMode.NONE });
expect(base.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.NONE);

base.options.enableAnnotationsDiscoverability = true;
base.handleAnnotationControlsClick({ mode: AnnotationMode.NONE });
expect(base.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.REGION);
});
});

describe('processAnnotationModeChange()', () => {
Expand Down Expand Up @@ -1954,4 +1810,10 @@ describe('lib/viewers/BaseViewer', () => {
});
});
});

describe('getInitialAnnotationMode()', () => {
test('should return none as initial mode', () => {
expect(base.getInitialAnnotationMode()).toBe(AnnotationMode.NONE);
});
});
});
Loading

0 comments on commit cb26355

Please sign in to comment.