Skip to content

Commit

Permalink
feat(annotations): Press Esc to exit annotations mode (#1226)
Browse files Browse the repository at this point in the history
* feat(annotations): Press Esc to exit annotations mode

* feat(annotations): Add reset to ImageViewer

* feat(annotations): Address comments
  • Loading branch information
Mingze authored Jun 18, 2020
1 parent 94d52f5 commit 96786d5
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 3 deletions.
38 changes: 37 additions & 1 deletion src/lib/AnnotationControls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export enum AnnotationMode {
export type ClickHandler = ({ activeControl, event }: { activeControl: AnnotationMode; event: MouseEvent }) => void;
export type Options = {
onRegionClick?: ClickHandler;
onReset?: () => void;
};

declare const __: (key: string) => string;
Expand All @@ -34,6 +35,10 @@ export default class AnnotationControls {

private currentActiveControl: AnnotationMode = AnnotationMode.NONE;

private hasInit = false;

private onReset: () => void = noop;

/**
* [constructor]
*/
Expand All @@ -55,8 +60,14 @@ export default class AnnotationControls {
* [destructor]
*/
public destroy(): void {
if (!this.hasInit) {
return;
}
fullscreen.removeListener('enter', this.handleFullscreenEnter);
fullscreen.removeListener('exit', this.handleFullscreenExit);
document.removeEventListener('keydown', this.handleKeyDown);

this.hasInit = false;
}

/**
Expand Down Expand Up @@ -110,6 +121,8 @@ export default class AnnotationControls {

this.currentActiveControl = AnnotationMode.NONE;
updateButton();

this.onReset();
};

/**
Expand Down Expand Up @@ -145,10 +158,28 @@ export default class AnnotationControls {
onClick({ activeControl: this.currentActiveControl, event });
};

/**
* Escape key handler, reset all control buttons,
* and stop propagation to prevent preview modal from exiting
*/
private handleKeyDown = (event: KeyboardEvent): void => {
if (event.key !== 'Escape' || this.currentActiveControl === AnnotationMode.NONE) {
return;
}

this.resetControls();

event.preventDefault();
event.stopPropagation();
};

/**
* Initialize the annotation controls with options.
*/
public init({ onRegionClick = noop }: Options = {}): void {
public init({ onRegionClick = noop, onReset = noop }: Options = {}): void {
if (this.hasInit) {
return;
}
const groupElement = this.controls.addGroup(CLASS_ANNOTATIONS_GROUP);
const regionButton = this.controls.add(
__('region_comment'),
Expand All @@ -160,5 +191,10 @@ export default class AnnotationControls {
);

regionButton.setAttribute('data-testid', 'bp-AnnotationsControls-regionBtn');

this.onReset = onReset;
document.addEventListener('keydown', this.handleKeyDown);

this.hasInit = true;
}
}
77 changes: 77 additions & 0 deletions src/lib/__tests__/AnnotationControls-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,23 @@ describe('lib/AnnotationControls', () => {

describe('destroy()', () => {
it('should remove all listeners', () => {
sandbox.spy(document, 'removeEventListener');
annotationControls.hasInit = true;

annotationControls.destroy();

expect(stubs.fullscreenRemoveListener).to.be.calledTwice;
expect(document.removeEventListener).to.be.calledWith('keydown', annotationControls.handleKeyDown);
expect(annotationControls.hasInit).to.equal(false);
});

it('should early return if hasInit is false', () => {
sandbox.spy(document, 'removeEventListener');

annotationControls.destroy();

expect(stubs.fullscreenRemoveListener).not.to.be.called;
expect(document.removeEventListener).not.to.be.called;
});
});

Expand All @@ -95,6 +109,66 @@ describe('lib/AnnotationControls', () => {
sinon.match.any,
);
});

it('should add keydown event listener', () => {
sandbox.spy(document, 'addEventListener');

annotationControls.init();

expect(document.addEventListener).to.be.calledWith('keydown', annotationControls.handleKeyDown);
});

it('should set onRest and hasInit', () => {
const onResetMock = sandbox.stub();

annotationControls.init({ onReset: onResetMock });

expect(annotationControls.onReset).to.equal(onResetMock);
expect(annotationControls.hasInit).to.equal(true);
});

it('should early return if hasInit is true', () => {
annotationControls.hasInit = true;

sandbox.spy(document, 'addEventListener');

annotationControls.init();

expect(annotationControls.controls.add).not.to.be.called;
expect(document.addEventListener).not.to.be.called;
});
});

describe('handleKeyDown', () => {
let eventMock;

beforeEach(() => {
annotationControls.resetControls = sandbox.stub();
annotationControls.currentActiveControl = 'region';
eventMock = {
key: 'Escape',
preventDefault: sandbox.stub(),
stopPropagation: sandbox.stub(),
};
});

it('should not call resetControls if key is not Escape or mode is none', () => {
annotationControls.handleKeyDown({ key: 'Enter' });

expect(annotationControls.resetControls).not.to.be.called;

annotationControls.currentActiveControl = 'none';
annotationControls.handleKeyDown({ key: 'Escape' });

expect(annotationControls.resetControls).not.to.be.called;
});

it('should call resetControls and prevent default event', () => {
annotationControls.handleKeyDown(eventMock);

expect(eventMock.preventDefault).to.be.called;
expect(eventMock.stopPropagation).to.be.called;
});
});

describe('handleClick()', () => {
Expand Down Expand Up @@ -139,9 +213,11 @@ describe('lib/AnnotationControls', () => {
describe('resetControls()', () => {
beforeEach(() => {
stubs.updateRegionButton = sandbox.stub();
stubs.onReset = sandbox.stub();
annotationControls.controlsMap = {
[AnnotationMode.REGION]: stubs.updateRegionButton,
};
annotationControls.onReset = stubs.onReset;
});

it('should not change if no current active control', () => {
Expand All @@ -158,6 +234,7 @@ describe('lib/AnnotationControls', () => {

expect(annotationControls.currentActiveControl).to.equal(AnnotationMode.NONE);
expect(stubs.updateRegionButton).to.be.called;
expect(stubs.onReset).to.be.called;
});
});

Expand Down
11 changes: 11 additions & 0 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ class BaseViewer extends EventEmitter {
this.mobileZoomEndHandler = this.mobileZoomEndHandler.bind(this);
this.handleAnnotatorEvents = this.handleAnnotatorEvents.bind(this);
this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this);
this.handleAnnotationControlsReset = this.handleAnnotationControlsReset.bind(this);
this.handleFullscreenEnter = this.handleFullscreenEnter.bind(this);
this.handleFullscreenExit = this.handleFullscreenExit.bind(this);
this.handleRegionClick = this.handleRegionClick.bind(this);
Expand Down Expand Up @@ -1040,6 +1041,16 @@ class BaseViewer extends EventEmitter {
return !!permissions && !!permissions.can_view_annotations;
}

/**
* Handler for annotation toolbar button reset
*
* @private
* @return {void}
*/
handleAnnotationControlsReset() {
this.annotator.toggleAnnotationMode(AnnotationMode.NONE);
}

/**
* Handler for annotation toolbar region comment button click event.
*
Expand Down
12 changes: 12 additions & 0 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1748,4 +1748,16 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.annotator.emit).to.be.calledWith('annotations_active_set', '123');
});
});

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

base.handleAnnotationControlsReset();

expect(base.annotator.toggleAnnotationMode).to.be.calledWith('none');
});
});
});
5 changes: 4 additions & 1 deletion src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,10 @@ class DocBaseViewer extends BaseViewer {
this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT);

if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) {
this.annotationControls.init({ onRegionClick: this.handleRegionClick });
this.annotationControls.init({
onRegionClick: this.handleRegionClick,
onReset: this.handleAnnotationControlsReset,
});
}
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2304,6 +2304,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
);
expect(docBase.annotationControls.init).to.be.calledWith({
onRegionClick: docBase.handleRegionClick,
onReset: docBase.handleAnnotationControlsReset,
});
});

Expand Down
5 changes: 4 additions & 1 deletion src/lib/viewers/image/ImageViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,10 @@ class ImageViewer extends ImageBaseViewer {

if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) {
this.annotationControls = new AnnotationControls(this.controls);
this.annotationControls.init({ onRegionClick: this.handleRegionClick });
this.annotationControls.init({
onRegionClick: this.handleRegionClick,
onReset: this.handleAnnotationControlsReset,
});
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/lib/viewers/image/__tests__/ImageViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,19 @@ describe('lib/viewers/image/ImageViewer', () => {
image.loadUI();
expect(image.annotationControls instanceof AnnotationControls).to.be.true;
});

it('should call annotations controls init with callbacks', () => {
sandbox.stub(image, 'areNewAnnotationsEnabled').returns(true);
sandbox.stub(image, 'hasAnnotationCreatePermission').returns(true);
sandbox.stub(AnnotationControls.prototype, 'init').callsFake();

image.loadUI();

expect(AnnotationControls.prototype.init).to.be.calledWith({
onRegionClick: image.handleRegionClick,
onReset: image.handleAnnotationControlsReset,
});
});
});

describe('isRotated()', () => {
Expand Down

0 comments on commit 96786d5

Please sign in to comment.