Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(annotations): Add region comment button in toolbar #1183

Merged
merged 9 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ zoom_out=Zoom out
enter_fullscreen=Enter fullscreen
# Button tooltip for exiting a full screen preview
exit_fullscreen=Exit fullscreen
# Button tooltip for annotation region comment
region_comment=Comment on Region
# 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
22 changes: 22 additions & 0 deletions src/lib/AnnotationControls.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
.bp-AnnotationControls-group {
padding: 0 4px 0 8px;
border-left: 1px solid $twos;

.bp-AnnotationControls-regionBtn {
width: $controls-button-width;
height: $controls-button-width;
border-radius: 4px;

svg {
fill: $white;
}

&.is-active {
background-color: $white;

svg {
fill: $black;
}
}
}
}
74 changes: 74 additions & 0 deletions src/lib/AnnotationControls.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import noop from 'lodash/noop';
import { ICON_REGION_COMMENT } from './icons/icons';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from './Controls';

export const CLASS_ANNOTATIONS_GROUP = 'bp-AnnotationControls-group';
export const CLASS_REGION_BUTTON = 'bp-AnnotationControls-regionBtn';
export const CLASS_BUTTON_ACTIVE = 'is-active';

export type RegionHandlerType = ({ isRegionActive, event }: { isRegionActive: boolean; event: MouseEvent }) => void;
mxiao6 marked this conversation as resolved.
Show resolved Hide resolved
export type Options = {
onRegionClick?: RegionHandlerType;
};

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

export default class AnnotationControls {
/** @property {Controls} - Controls object */
private controls: Controls;

/** @property {boolean} - Region comment mode active state */
private isRegionActive = false;

/**
* [constructor]
*
* @param {Controls} controls - Viewer controls
* @return {AnnotationControls} Instance of AnnotationControls
*/
constructor(controls: Controls) {
if (!controls || !(controls instanceof Controls)) {
throw Error('controls must be an instance of Controls');
}

this.controls = controls;
}

/**
* Region comment button click handler
*
* @param {RegionHandlerType} onRegionClick - region click handler in options
* @param {MouseEvent} event - mouse event
* @return {void}
*/
private handleRegionClick = (onRegionClick: RegionHandlerType) => (event: MouseEvent): void => {
const regionButtonElement = event.target as HTMLButtonElement;

this.isRegionActive = !this.isRegionActive;
if (this.isRegionActive) {
regionButtonElement.classList.add(CLASS_BUTTON_ACTIVE);
} else {
regionButtonElement.classList.remove(CLASS_BUTTON_ACTIVE);
}

onRegionClick({ isRegionActive: this.isRegionActive, event });
};

/**
* Initialize the annotation controls with options.
*
* @param {RegionHandlerType} [options.onRegionClick] - Callback when region comment button is clicked
* @return {void}
*/
public init({ onRegionClick = noop }: Options = {}): void {
const groupElement = this.controls.addGroup(CLASS_ANNOTATIONS_GROUP);
this.controls.add(
__('region_comment'),
this.handleRegionClick(onRegionClick),
`${CLASS_BOX_CONTROLS_GROUP_BUTTON} ${CLASS_REGION_BUTTON}`,
ICON_REGION_COMMENT,
'button',
groupElement,
);
}
}
16 changes: 7 additions & 9 deletions src/lib/Controls.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
$controls-button-width: 32px;

@import './AnnotationControls';
@import './ZoomControls';

.bp-controls-wrapper {
position: absolute;
bottom: 25px;
Expand All @@ -11,7 +16,7 @@
position: relative;
left: -50%;
display: flex;
background: fade-out($twos, .05);
background: fade-out($black, .2);
border-radius: 3px;
opacity: 0;
transition: opacity .5s;
Expand Down Expand Up @@ -159,21 +164,14 @@
}
}

.bp-zoom-current-scale {
min-width: 48px;
color: $white;
font-size: 14px;
text-align: center;
}

.bp-controls-group {
display: flex;
align-items: center;
margin-right: 4px;
margin-left: 4px;

.bp-controls-group-btn {
width: 32px;
width: $controls-button-width;
}

& + .bp-controls-cell {
Expand Down
5 changes: 4 additions & 1 deletion src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -921,9 +921,12 @@ class Preview extends EventEmitter {
// Whether download button should be shown
this.options.showDownload = !!options.showDownload;

// Whether annotations and annotation controls should be shown
// Whether annotations v2 should be shown
this.options.showAnnotations = !!options.showAnnotations;

// Whether annotations v4 buttons should be shown in toolbar
this.options.showAnnotationsControls = !!options.showAnnotationsControls;

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

Expand Down
2 changes: 1 addition & 1 deletion src/lib/ZoomControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import noop from 'lodash/noop';
import { ICON_ZOOM_IN, ICON_ZOOM_OUT } from './icons/icons';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from './Controls';

const CLASS_ZOOM_CURRENT_SCALE = 'bp-zoom-current-scale';
const CLASS_ZOOM_CURRENT_SCALE = 'bp-ZoomControls-currentScale';
const CLASS_ZOOM_CURRENT_SCALE_VALUE = 'bp-zoom-current-scale-value';
const CLASS_ZOOM_IN_BUTTON = 'bp-zoom-in-btn';
const CLASS_ZOOM_OUT_BUTTON = 'bp-zoom-out-btn';
Expand Down
6 changes: 6 additions & 0 deletions src/lib/ZoomControls.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.bp-ZoomControls-currentScale {
min-width: 48px;
color: $white;
font-size: 14px;
text-align: center;
}
1 change: 1 addition & 0 deletions src/lib/__tests__/AnnotationControls-test.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div id="test-annotation-controls-container"></div>
97 changes: 97 additions & 0 deletions src/lib/__tests__/AnnotationControls-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/* eslint-disable no-unused-expressions */
import AnnotationControls, { CLASS_REGION_BUTTON, CLASS_BUTTON_ACTIVE } from '../AnnotationControls';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from '../Controls';
import { ICON_REGION_COMMENT } from '../icons/icons';

let annotationControls;
let stubs = {};

const sandbox = sinon.sandbox.create();

describe('lib/AnnotationControls', () => {
before(() => {
fixture.setBase('src/lib');
});

beforeEach(() => {
fixture.load('__tests__/AnnotationControls-test.html');
const controls = new Controls(document.getElementById('test-annotation-controls-container'));
annotationControls = new AnnotationControls(controls);
stubs.onRegionClick = sandbox.stub();
});

afterEach(() => {
fixture.cleanup();
sandbox.verifyAndRestore();

annotationControls = null;
stubs = {};
});

describe('constructor()', () => {
it('should create the correct DOM structure', () => {
expect(annotationControls.controls).not.to.be.undefined;
});

it('should throw an exception if controls is not provided', () => {
expect(() => new AnnotationControls()).to.throw(Error, 'controls must be an instance of Controls');
});
});

describe('init()', () => {
beforeEach(() => {
stubs.add = sandbox.stub(annotationControls.controls, 'add');
stubs.regionHandler = sandbox.stub();
sandbox.stub(annotationControls, 'handleRegionClick').returns(stubs.regionHandler);
});

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

expect(stubs.add).to.be.calledWith(
__('region_comment'),
stubs.regionHandler,
`${CLASS_BOX_CONTROLS_GROUP_BUTTON} ${CLASS_REGION_BUTTON}`,
ICON_REGION_COMMENT,
'button',
sinon.match.any,
);
});
});

describe('handleRegionClick()', () => {
beforeEach(() => {
stubs.classListAdd = sandbox.stub();
stubs.classListRemove = sandbox.stub();
stubs.event = sandbox.stub({
target: {
classList: {
add: stubs.classListAdd,
remove: stubs.classListRemove,
},
},
});
});

it('should activate region button then deactivate', () => {
expect(annotationControls.isRegionActive).to.be.false;

annotationControls.handleRegionClick(stubs.onRegionClick)(stubs.event);
expect(annotationControls.isRegionActive).to.be.true;
expect(stubs.classListAdd).to.be.calledWith(CLASS_BUTTON_ACTIVE);

annotationControls.handleRegionClick(stubs.onRegionClick)(stubs.event);
expect(annotationControls.isRegionActive).to.be.false;
expect(stubs.classListRemove).to.be.calledWith(CLASS_BUTTON_ACTIVE);
});

it('should call onRegionClick', () => {
annotationControls.handleRegionClick(stubs.onRegionClick)(stubs.event);

expect(stubs.onRegionClick).to.be.calledWith({
isRegionActive: true,
event: stubs.event,
});
});
});
});
4 changes: 2 additions & 2 deletions src/lib/__tests__/ZoomControls-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('lib/ZoomControls', () => {
expect(stubs.add).to.be.calledWith(
__('zoom_current_scale'),
undefined,
'bp-zoom-current-scale',
'bp-ZoomControls-currentScale',
sinon.match.string,
'div',
sinon.match.any,
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('lib/ZoomControls', () => {
expect(stubs.add).to.be.calledWith(
__('zoom_current_scale'),
undefined,
'bp-zoom-current-scale',
'bp-ZoomControls-currentScale',
sinon.match.string,
'div',
sinon.match.any,
Expand Down
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 REGION_COMMENT from './region_comment.svg';
import ARROW_LEFT from './arrow_left_24px.svg';
import ARROW_RIGHT from './arrow_right_24px.svg';
import CHECK_MARK from './checkmark_24px.svg';
Expand Down Expand Up @@ -55,6 +56,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_REGION_COMMENT = REGION_COMMENT;
export const ICON_ARROW_LEFT = ARROW_LEFT;
export const ICON_ARROW_RIGHT = ARROW_RIGHT;
export const ICON_CHECK_MARK = CHECK_MARK;
Expand Down
1 change: 1 addition & 0 deletions src/lib/icons/region_comment.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
19 changes: 19 additions & 0 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import throttle from 'lodash/throttle';
import AnnotationControls from '../../AnnotationControls';
import BaseViewer from '../BaseViewer';
import Browser from '../../Browser';
import Controls from '../../Controls';
Expand Down Expand Up @@ -96,6 +97,7 @@ class DocBaseViewer extends BaseViewer {
this.pinchToZoomEndHandler = this.pinchToZoomEndHandler.bind(this);
this.pinchToZoomStartHandler = this.pinchToZoomStartHandler.bind(this);
this.print = this.print.bind(this);
this.regionClickHandler = this.regionClickHandler.bind(this);
this.setPage = this.setPage.bind(this);
this.throttledScrollHandler = this.getScrollHandler().bind(this);
this.toggleThumbnails = this.toggleThumbnails.bind(this);
Expand Down Expand Up @@ -1009,6 +1011,9 @@ class DocBaseViewer extends BaseViewer {
this.controls = new Controls(this.containerEl);
this.pageControls = new PageControls(this.controls, this.docEl);
this.zoomControls = new ZoomControls(this.controls);
if (this.options.showAnnotationsControls) {
this.annotationControls = new AnnotationControls(this.controls);
}
this.pageControls.addListener('pagechange', this.setPage);
this.bindControlListeners();
}
Expand Down Expand Up @@ -1090,8 +1095,22 @@ class DocBaseViewer extends BaseViewer {
ICON_FULLSCREEN_IN,
);
this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT);

if (this.options.showAnnotationsControls) {
this.annotationControls.init({
onRegionClick: this.regionClickHandler,
});
}
}

/**
* Handler for annotation toolbar region comment button click event.
*
* @private
* @return {void}
*/
regionClickHandler() {}

/**
* Handler for 'pagesinit' event.
*
Expand Down