Skip to content

Commit

Permalink
feat(annotations): Add region comment button in toolbar (#1183)
Browse files Browse the repository at this point in the history
* feat(annotations): Add region comment button in toolbar

* feat(annotations): Correct styles

* feat(annotations): Address comments

* feat(annotations): Rename variables

* feat(annotations): Add tests, SUIT style, and typescript

* feat(annotations): Fix typescript

* feat(annotations): Address comments

* feat(annotations): Address comments

* feat(annotations): Address comments
  • Loading branch information
Mingze authored Mar 27, 2020
1 parent 45f92a5 commit 04ea830
Show file tree
Hide file tree
Showing 13 changed files with 238 additions and 13 deletions.
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 RegionHandler = ({ isRegionActive, event }: { isRegionActive: boolean; event: MouseEvent }) => void;
export type Options = {
onRegionClick?: RegionHandler;
};

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 {RegionHandler} onRegionClick - region click handler in options
* @param {MouseEvent} event - mouse event
* @return {void}
*/
private handleRegionClick = (onRegionClick: RegionHandler) => (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 {RegionHandler} [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

0 comments on commit 04ea830

Please sign in to comment.