Skip to content

Commit

Permalink
feat(annotations): Add Highlight Text button in toolbar (#1236)
Browse files Browse the repository at this point in the history
* feat(annotations): Add Highlight Text button in toolbar

* feat(annotations): Address PR feedbacks

* feat(annotations): Add tests

* feat(annotations): Change tooltip text
  • Loading branch information
Mingze authored Jul 27, 2020
1 parent 96942d6 commit c0869fe
Show file tree
Hide file tree
Showing 10 changed files with 178 additions and 57 deletions.
2 changes: 2 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ enter_fullscreen=Enter fullscreen
exit_fullscreen=Exit fullscreen
# Button tooltip for annotation region comment
region_comment=Comment on Region
# Button tooltip for annotation highlight text
highlight_text=Highlight and Comment
# Button tooltip for going to the previous page in a preview
previous_page=Previous page
# Input tooltip for navigating to a specific page in a preview
Expand Down
2 changes: 1 addition & 1 deletion src/lib/AnnotationControls.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
padding-left: 4px;
border-left: 1px solid $twos;

.bp-AnnotationControls-regionBtn {
.bp-AnnotationControls-button {
svg {
width: 34px;
height: 32px;
Expand Down
109 changes: 77 additions & 32 deletions src/lib/AnnotationControls.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,66 @@
import noop from 'lodash/noop';

import { ICON_REGION_COMMENT } from './icons/icons';
import { ICON_HIGHLIGHT_TEXT, ICON_REGION_COMMENT } from './icons/icons';
import Controls from './Controls';

export const CLASS_ANNOTATIONS_BUTTON = 'bp-AnnotationControls-button';
export const CLASS_ANNOTATIONS_GROUP = 'bp-AnnotationControls-group';
export const CLASS_HIGHLIGHT_BUTTON = 'bp-AnnotationControls-highlightBtn';
export const CLASS_REGION_BUTTON = 'bp-AnnotationControls-regionBtn';

export const CLASS_BUTTON_ACTIVE = 'is-active';
export const CLASS_GROUP_HIDE = 'is-hidden';

export enum AnnotationMode {
HIGHLIGHT = 'highlight',
NONE = 'none',
REGION = 'region',
}
export type ClickHandler = ({ event }: { event: MouseEvent }) => void;
export type Options = {
fileId: string;
onEscape?: () => void;
onHighlightClick?: ClickHandler;
onRegionClick?: ClickHandler;
showHighlightText: boolean;
};

type ButtonProps = {
classname: string;
icon: string;
resinTarget: string;
testid: string;
text: string;
};

declare const __: (key: string) => string;

interface ControlsMap {
[key: string]: () => void;
}
const buttonClassMap: { [key: string]: string } = {
[AnnotationMode.HIGHLIGHT]: CLASS_HIGHLIGHT_BUTTON,
[AnnotationMode.REGION]: CLASS_REGION_BUTTON,
};
const buttonPropsMap: { [key: string]: ButtonProps } = {
[AnnotationMode.HIGHLIGHT]: {
classname: `${CLASS_ANNOTATIONS_BUTTON} ${CLASS_HIGHLIGHT_BUTTON}`,
icon: ICON_HIGHLIGHT_TEXT,
resinTarget: 'highlightText',
testid: 'bp-AnnotationsControls-highlightBtn',
text: __('highlight_text'),
},
[AnnotationMode.REGION]: {
classname: `${CLASS_ANNOTATIONS_BUTTON} ${CLASS_REGION_BUTTON}`,
icon: ICON_REGION_COMMENT,
resinTarget: 'highlightRegion',
testid: 'bp-AnnotationsControls-regionBtn',
text: __('region_comment'),
},
};

export default class AnnotationControls {
private controls: Controls;

private controlsElement: HTMLElement;

private controlsMap: ControlsMap;

private currentMode: AnnotationMode = AnnotationMode.NONE;

private hasInit = false;
Expand All @@ -49,9 +77,6 @@ export default class AnnotationControls {

this.controls = controls;
this.controlsElement = controls.controlsEl;
this.controlsMap = {
[AnnotationMode.REGION]: this.updateRegionButton,
};
}

/**
Expand All @@ -75,10 +100,10 @@ export default class AnnotationControls {
return;
}

const updateButton = this.controlsMap[this.currentMode];
const prevMode = this.currentMode;

this.currentMode = AnnotationMode.NONE;
updateButton();
this.updateButton(prevMode);
};

/**
Expand All @@ -99,19 +124,19 @@ export default class AnnotationControls {
}

/**
* Update region button UI
* Update button UI
*/
private updateRegionButton = (): void => {
const regionButtonElement = this.controlsElement.querySelector(`.${CLASS_REGION_BUTTON}`);
private updateButton = (mode: AnnotationMode): void => {
const buttonElement = this.controlsElement.querySelector(`.${buttonClassMap[mode]}`);

if (!regionButtonElement) {
if (!buttonElement) {
return;
}

if (this.currentMode === AnnotationMode.REGION) {
regionButtonElement.classList.add(CLASS_BUTTON_ACTIVE);
if (this.currentMode === mode) {
buttonElement.classList.add(CLASS_BUTTON_ACTIVE);
} else {
regionButtonElement.classList.remove(CLASS_BUTTON_ACTIVE);
buttonElement.classList.remove(CLASS_BUTTON_ACTIVE);
}
};

Expand All @@ -125,7 +150,7 @@ export default class AnnotationControls {

if (prevMode !== mode) {
this.currentMode = mode as AnnotationMode;
this.controlsMap[mode]();
this.updateButton(mode);
}

onClick({ event });
Expand All @@ -147,27 +172,47 @@ export default class AnnotationControls {
event.stopPropagation();
};

private addButton = (mode: AnnotationMode, handler: ClickHandler, parent: HTMLElement, fileId: string): void => {
const buttonProps = buttonPropsMap[mode];

if (!buttonProps) {
return;
}

const buttonElement = this.controls.add(
buttonProps.text,
this.handleClick(handler, mode),
buttonProps.classname,
buttonProps.icon,
'button',
parent,
);

buttonElement.setAttribute('data-resin-target', buttonProps.resinTarget);
buttonElement.setAttribute('data-resin-fileId', fileId);
buttonElement.setAttribute('data-testid', buttonProps.testid);
};

/**
* Initialize the annotation controls with options.
*/
public init({ fileId, onEscape = noop, onRegionClick = noop }: Options): void {
public init({
fileId,
onEscape = noop,
onRegionClick = noop,
onHighlightClick = noop,
showHighlightText = false,
}: Options): void {
if (this.hasInit) {
return;
}
const groupElement = this.controls.addGroup(CLASS_ANNOTATIONS_GROUP);
const regionButton = this.controls.add(
__('region_comment'),
this.handleClick(onRegionClick, AnnotationMode.REGION),
CLASS_REGION_BUTTON,
ICON_REGION_COMMENT,
'button',
groupElement,
);

groupElement.setAttribute('data-resin-feature', 'annotations');
regionButton.setAttribute('data-resin-target', 'highlightRegion');
regionButton.setAttribute('data-resin-fileId', fileId);
regionButton.setAttribute('data-testid', 'bp-AnnotationsControls-regionBtn');

this.addButton(AnnotationMode.REGION, onRegionClick, groupElement, fileId);
if (showHighlightText) {
this.addButton(AnnotationMode.HIGHLIGHT, onHighlightClick, groupElement, fileId);
}

this.onEscape = onEscape;
document.addEventListener('keydown', this.handleKeyDown);
Expand Down
2 changes: 2 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,8 @@ class Preview extends EventEmitter {
// Whether annotations v4 buttons should be shown in toolbar
this.options.showAnnotationsControls = !!options.showAnnotationsControls;

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

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

Expand Down
84 changes: 60 additions & 24 deletions src/lib/__tests__/AnnotationControls-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { ICON_REGION_COMMENT } from '../icons/icons';
import AnnotationControls, {
AnnotationMode,
CLASS_ANNOTATIONS_BUTTON,
CLASS_ANNOTATIONS_GROUP,
CLASS_BUTTON_ACTIVE,
CLASS_GROUP_HIDE,
Expand All @@ -23,6 +24,7 @@ describe('lib/AnnotationControls', () => {
fixture.load('__tests__/AnnotationControls-test.html');
stubs.classListAdd = sandbox.stub();
stubs.classListRemove = sandbox.stub();
stubs.onHighlightClick = sandbox.stub();
stubs.onRegionClick = sandbox.stub();
stubs.querySelector = sandbox.stub().returns({
classList: {
Expand All @@ -48,7 +50,6 @@ describe('lib/AnnotationControls', () => {
it('should create the correct DOM structure', () => {
expect(annotationControls.controls).not.to.be.undefined;
expect(annotationControls.controlsElement).not.to.be.undefined;
expect(annotationControls.controlsMap).not.to.be.undefined;
expect(annotationControls.currentMode).to.equal(AnnotationMode.NONE);
});

Expand Down Expand Up @@ -79,25 +80,28 @@ describe('lib/AnnotationControls', () => {

describe('init()', () => {
beforeEach(() => {
stubs.regionButton = {
setAttribute: sandbox.stub(),
};
stubs.regionHandler = sandbox.stub();

sandbox.stub(annotationControls, 'handleClick').returns(stubs.regionHandler);
sandbox.stub(annotationControls.controls, 'add').returns(stubs.regionButton);
sandbox.stub(annotationControls, 'addButton');
});

it('should add the controls', () => {
it('should only add region button', () => {
annotationControls.init({ fileId: '0', onRegionClick: stubs.onRegionClick });

expect(annotationControls.controls.add).to.be.calledWith(
__('region_comment'),
stubs.regionHandler,
CLASS_REGION_BUTTON,
ICON_REGION_COMMENT,
'button',
expect(annotationControls.addButton).to.be.calledOnceWith(
AnnotationMode.REGION,
stubs.onRegionClick,
sinon.match.any,
'0',
);
});

it('should add highlight button', () => {
annotationControls.init({ fileId: '0', onHighlightClick: stubs.onHighlightClick, showHighlightText: true });

expect(annotationControls.addButton).to.be.calledWith(
AnnotationMode.HIGHLIGHT,
stubs.onHighlightClick,
sinon.match.any,
'0',
);
});

Expand All @@ -109,7 +113,7 @@ describe('lib/AnnotationControls', () => {
expect(document.addEventListener).to.be.calledWith('keydown', annotationControls.handleKeyDown);
});

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

annotationControls.init({ fileId: '0', onEscape: onEscapeMock });
Expand All @@ -125,7 +129,7 @@ describe('lib/AnnotationControls', () => {

annotationControls.init({ fileId: '0' });

expect(annotationControls.controls.add).not.to.be.called;
expect(annotationControls.addButton).not.to.be.called;
expect(document.addEventListener).not.to.be.called;
});
});
Expand Down Expand Up @@ -190,27 +194,25 @@ describe('lib/AnnotationControls', () => {

describe('resetControls()', () => {
beforeEach(() => {
stubs.updateRegionButton = sandbox.stub();
sandbox.stub(annotationControls, 'updateButton');

stubs.onEscape = sandbox.stub();
annotationControls.controlsMap = {
[AnnotationMode.REGION]: stubs.updateRegionButton,
};
});

it('should not change if no current active control', () => {
annotationControls.resetControls();

expect(annotationControls.currentMode).to.equal(AnnotationMode.NONE);
expect(stubs.updateRegionButton).not.to.be.called;
expect(annotationControls.updateButton).not.to.be.called;
});

it('should call updateRegionButton if current control is region', () => {
it('should call updateButton if current control is region', () => {
annotationControls.currentMode = AnnotationMode.REGION;

annotationControls.resetControls();

expect(annotationControls.currentMode).to.equal(AnnotationMode.NONE);
expect(stubs.updateRegionButton).to.be.called;
expect(annotationControls.updateButton).to.be.calledWith(AnnotationMode.REGION);
});
});

Expand All @@ -225,4 +227,38 @@ describe('lib/AnnotationControls', () => {
expect(stubs.classListRemove).to.be.calledWith(CLASS_GROUP_HIDE);
});
});

describe('addButton()', () => {
beforeEach(() => {
stubs.buttonElement = {
setAttribute: sandbox.stub(),
};
stubs.clickHandler = sandbox.stub();

sandbox.stub(annotationControls, 'handleClick').returns(stubs.clickHandler);
sandbox.stub(annotationControls.controls, 'add').returns(stubs.buttonElement);
});

it('should do nothing for unknown button', () => {
annotationControls.addButton('draw', sandbox.stub(), 'group', '0');

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

it('should add controls and add resin tags', () => {
annotationControls.addButton(AnnotationMode.REGION, sandbox.stub(), 'group', '0');

expect(annotationControls.controls.add).to.be.calledWith(
__('region_comment'),
stubs.clickHandler,
`${CLASS_ANNOTATIONS_BUTTON} ${CLASS_REGION_BUTTON}`,
ICON_REGION_COMMENT,
'button',
'group',
);

expect(stubs.buttonElement.setAttribute).to.be.calledWith('data-resin-target', 'highlightRegion');
expect(stubs.buttonElement.setAttribute).to.be.calledWith('data-resin-fileId', '0');
});
});
});
1 change: 1 addition & 0 deletions src/lib/icons/highlight_text.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions src/lib/icons/icons.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import FULLSCREEN_OUT from './full_screen_out_24px.svg';
import ROTATE_LEFT from './rotate_left_24px.svg';
import ZOOM_IN from './zoom_in.svg';
import ZOOM_OUT from './zoom_out.svg';
import HIGHLIGHT_TEXT from './highlight_text.svg';
import REGION_COMMENT from './region_comment.svg';
import ARROW_LEFT from './arrow_left_24px.svg';
import ARROW_RIGHT from './arrow_right_24px.svg';
Expand Down Expand Up @@ -56,6 +57,7 @@ export const ICON_FULLSCREEN_OUT = FULLSCREEN_OUT;
export const ICON_ROTATE_LEFT = ROTATE_LEFT;
export const ICON_ZOOM_IN = ZOOM_IN;
export const ICON_ZOOM_OUT = ZOOM_OUT;
export const ICON_HIGHLIGHT_TEXT = HIGHLIGHT_TEXT;
export const ICON_REGION_COMMENT = REGION_COMMENT;
export const ICON_ARROW_LEFT = ARROW_LEFT;
export const ICON_ARROW_RIGHT = ARROW_RIGHT;
Expand Down
Loading

0 comments on commit c0869fe

Please sign in to comment.