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 drawing button to preview controls #1296

Merged
merged 17 commits into from
Nov 19, 2020
Merged
Show file tree
Hide file tree
Changes from all 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 drawing comment
drawing_comment=Markup
# Button tooltip for annotation region comment
region_comment=Comment on Region
# Button tooltip for annotation highlight text
Expand Down
1 change: 1 addition & 0 deletions src/lib/AnnotationControls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const CLASS_BUTTON_ACTIVE = 'is-active';
export const CLASS_GROUP_HIDE = 'is-hidden';

export enum AnnotationMode {
DRAWING = 'drawing',
HIGHLIGHT = 'highlight',
NONE = 'none',
REGION = 'region',
Expand Down
4 changes: 4 additions & 0 deletions src/lib/AnnotationControlsFSM.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { AnnotationMode } from './AnnotationControls';

export enum AnnotationState {
DRAWING = 'drawing',
HIGHLIGHT = 'highlight',
HIGHLIGHT_TEMP = 'highlight_temp',
NONE = 'none',
Expand All @@ -21,18 +22,21 @@ export enum AnnotationInput {
type Subscription = (state: AnnotationState) => void;

export const modeStateMap = {
[AnnotationMode.DRAWING]: AnnotationState.DRAWING,
[AnnotationMode.HIGHLIGHT]: AnnotationState.HIGHLIGHT,
[AnnotationMode.NONE]: AnnotationState.NONE,
[AnnotationMode.REGION]: AnnotationState.REGION,
};

export const modeTempStateMap = {
[AnnotationMode.DRAWING]: AnnotationState.NONE,
ChenCodes marked this conversation as resolved.
Show resolved Hide resolved
[AnnotationMode.HIGHLIGHT]: AnnotationState.HIGHLIGHT_TEMP,
[AnnotationMode.NONE]: AnnotationState.NONE,
[AnnotationMode.REGION]: AnnotationState.REGION_TEMP,
};

export const stateModeMap = {
[AnnotationState.DRAWING]: AnnotationMode.DRAWING,
[AnnotationState.HIGHLIGHT]: AnnotationMode.HIGHLIGHT,
[AnnotationState.HIGHLIGHT_TEMP]: AnnotationMode.HIGHLIGHT,
[AnnotationState.NONE]: AnnotationMode.NONE,
Expand Down
2 changes: 2 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,8 @@ class Preview extends EventEmitter {
// Whether annotations v4 buttons should be shown in toolbar
this.options.showAnnotationsControls = !!options.showAnnotationsControls;

this.options.showAnnotationsDrawingCreate = !!options.showAnnotationsDrawingCreate;

this.options.enableAnnotationsDiscoverability = !!options.enableAnnotationsDiscoverability;

this.options.enableAnnotationsImageDiscoverability = !!options.enableAnnotationsImageDiscoverability;
Expand Down
21 changes: 21 additions & 0 deletions src/lib/__tests__/AnnotationControlsFSM-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ describe('lib/AnnotationControlsFSM', () => {
nextState: AnnotationState.REGION_TEMP,
output: AnnotationMode.REGION,
},
{
input: AnnotationInput.CLICK,
mode: AnnotationMode.DRAWING,
nextState: AnnotationState.DRAWING,
output: AnnotationMode.DRAWING,
},
].forEach(({ input, mode, nextState, output }) => {
it(`should go to state ${nextState} and output ${output} if input is ${input} and mode is ${mode}`, () => {
const annotationControlsFSM = new AnnotationControlsFSM();
Expand Down Expand Up @@ -72,6 +78,21 @@ describe('lib/AnnotationControlsFSM', () => {
});
});

describe('AnnotationState.DRAWING', () => {
describe('AnnotationState.DRAWING', () => {
test('should output AnnotationMode.NONE if input is AnnotationInput.CLICK and mode is AnnotationMode.DRAWING', () => {
const annotationControlsFSM = new AnnotationControlsFSM(AnnotationState.DRAWING);
const input = AnnotationInput.CLICK;
const mode = AnnotationMode.DRAWING;
const output = AnnotationMode.NONE;

expect(annotationControlsFSM.transition(input, mode)).toEqual(output);
expect(annotationControlsFSM.getMode()).toBe(output);
expect(annotationControlsFSM.getState()).toEqual(output);
});
});
});

describe('AnnotationState.HIGHLIGHT/REGION', () => {
// Stay in the same state
[AnnotationState.HIGHLIGHT, AnnotationState.REGION].forEach(state => {
Expand Down
17 changes: 16 additions & 1 deletion src/lib/viewers/controls/annotations/AnnotationsControls.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import noop from 'lodash/noop';
import AnnotationsButton from './AnnotationsButton';
import IconDrawing24 from '../icons/IconDrawing24';
import IconHighlightText16 from '../icons/IconHighlightText16';
import IconRegion24 from '../icons/IconRegion24';
import useFullscreen from '../hooks/useFullscreen';
Expand All @@ -9,6 +10,7 @@ import './AnnotationsControls.scss';

export type Props = {
annotationMode?: AnnotationMode;
hasDrawing?: boolean;
hasHighlight?: boolean;
hasRegion?: boolean;
onAnnotationModeClick?: ({ mode }: { mode: AnnotationMode }) => void;
Expand All @@ -17,12 +19,14 @@ export type Props = {

export default function AnnotationsControls({
annotationMode = AnnotationMode.NONE,
hasDrawing = false,
hasHighlight = false,
hasRegion = false,
onAnnotationModeClick = noop,
onAnnotationModeEscape = noop,
}: Props): JSX.Element | null {
const isFullscreen = useFullscreen();
const showDrawing = !isFullscreen && hasDrawing;
const showHighlight = !isFullscreen && hasHighlight;
const showRegion = !isFullscreen && hasRegion;

Expand Down Expand Up @@ -54,12 +58,23 @@ export default function AnnotationsControls({
}, [annotationMode, onAnnotationModeEscape]);

// Prevent empty group from being displayed
if (!showHighlight && !showRegion) {
if (!showDrawing && !showHighlight && !showRegion) {
return null;
}

return (
<div className="bp-AnnotationsControls">
<AnnotationsButton
data-resin-target="draw"
data-testid="bp-AnnotationsControls-drawBtn"
isActive={annotationMode === AnnotationMode.DRAWING}
isEnabled={showDrawing}
mode={AnnotationMode.DRAWING}
onClick={handleModeClick}
title={__('drawing_comment')}
ChenCodes marked this conversation as resolved.
Show resolved Hide resolved
>
<IconDrawing24 />
</AnnotationsButton>
<AnnotationsButton
data-resin-target="highlightRegion"
data-testid="bp-AnnotationsControls-regionBtn"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,18 @@ describe('AnnotationsControls', () => {

describe('event handlers', () => {
test.each`
current | selector | result
${AnnotationMode.NONE} | ${'bp-AnnotationsControls-regionBtn'} | ${AnnotationMode.REGION}
${AnnotationMode.REGION} | ${'bp-AnnotationsControls-regionBtn'} | ${AnnotationMode.NONE}
${AnnotationMode.REGION} | ${'bp-AnnotationsControls-highlightBtn'} | ${AnnotationMode.HIGHLIGHT}
${AnnotationMode.NONE} | ${'bp-AnnotationsControls-highlightBtn'} | ${AnnotationMode.HIGHLIGHT}
current | selector | result
${AnnotationMode.NONE} | ${'bp-AnnotationsControls-regionBtn'} | ${AnnotationMode.REGION}
${AnnotationMode.REGION} | ${'bp-AnnotationsControls-regionBtn'} | ${AnnotationMode.NONE}
${AnnotationMode.REGION} | ${'bp-AnnotationsControls-highlightBtn'} | ${AnnotationMode.HIGHLIGHT}
${AnnotationMode.NONE} | ${'bp-AnnotationsControls-highlightBtn'} | ${AnnotationMode.HIGHLIGHT}
${AnnotationMode.NONE} | ${'bp-AnnotationsControls-drawBtn'} | ${AnnotationMode.DRAWING}
${AnnotationMode.DRAWING} | ${'bp-AnnotationsControls-drawBtn'} | ${AnnotationMode.NONE}
`('in $current mode returns $result when $selector is clicked', ({ current, result, selector }) => {
const onClick = jest.fn();
const element = getElement({
annotationMode: current,
hasDrawing: true,
hasHighlight: true,
hasRegion: true,
onAnnotationModeClick: onClick,
Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/controls/annotations/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// eslint-disable-next-line import/prefer-default-export
export enum AnnotationMode {
DRAWING = 'drawing',
HIGHLIGHT = 'highlight',
NONE = 'none',
REGION = 'region',
Expand Down
11 changes: 11 additions & 0 deletions src/lib/viewers/controls/icons/IconDrawing24.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as React from 'react';

function IconDrawing24(props: React.SVGProps<SVGSVGElement>): JSX.Element {
return (
<svg focusable="false" height={24} viewBox="0 0 18 18" width={24} {...props}>
<path d="M12.444 13.4l.677-.673a.876.876 0 00.15.995c.33.33.77.23 1.078-.14.743-.855.855-1.934.302-2.523-.743-.792-1.716-.2-2.656.736l-.973 1.01 1.17-1.366c1.5-1.7 2.13-3.16 1.236-4.03-.835-.827-1.986-.105-3.827 1.696-1.552 1.52-2.347 2.347-2.63 2.04-.217-.23.15-.736 2.02-2.964 2.558-3.048 3.636-5.185 2.53-6.25-1.13-1.086-2.44-.133-5.582 2.593C3.765 6.424 2.4 7.355 1.522 7.84c-.44.245-.63.708-.46 1.12.178.427.644.56 1.157.287.88-.462 2.433-1.563 4.662-3.504C9.616 3.36 10.182 2.9 10.465 3.2c.342.35-.276 1.33-2.65 4.106-2.65 3.083-2.94 4.26-2.07 5.185 1.006 1.072 2.38.2 4.655-2.01 1.802-1.76 1.913-1.913 2.02-1.794.112.12-.125.308-1.736 2.312-1.27 1.563-1.532 2.46-.874 3.16.605.652 1.466.35 2.637-.75z" />
</svg>
);
}

export default IconDrawing24;
3 changes: 3 additions & 0 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1083,11 +1083,13 @@ class DocBaseViewer extends BaseViewer {
if (this.controls && this.options.useReactControls) {
const canAnnotate = this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission();
const canDownload = checkPermission(this.options.file, PERMISSION_DOWNLOAD);
const canDraw = canAnnotate && this.options.showAnnotationsDrawingCreate;
jstoffan marked this conversation as resolved.
Show resolved Hide resolved
const canHighlight = canAnnotate && canDownload;

this.controls.render(
<DocControls
annotationMode={this.annotationControlsFSM.getMode()}
hasDrawing={canDraw}
hasHighlight={canHighlight}
hasRegion={canAnnotate}
maxScale={MAX_SCALE}
Expand Down Expand Up @@ -1194,6 +1196,7 @@ class DocBaseViewer extends BaseViewer {
fileId: this.options.file.id,
onClick: this.handleAnnotationControlsClick,
onEscape: this.handleAnnotationControlsEscape,
showDrawing: this.options.showAnnotationsDrawingCreate,
showHighlightText: canDownload,
});
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/doc/DocControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type Props = AnnotationsControlsProps &

export default function DocControls({
annotationMode,
hasDrawing,
hasHighlight,
hasRegion,
maxScale,
Expand Down Expand Up @@ -53,6 +54,7 @@ export default function DocControls({
<FullscreenToggle onFullscreenToggle={onFullscreenToggle} />
<AnnotationsControls
annotationMode={annotationMode}
hasDrawing={hasDrawing}
hasHighlight={hasHighlight}
hasRegion={hasRegion}
onAnnotationModeClick={onAnnotationModeClick}
Expand Down
42 changes: 41 additions & 1 deletion src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1704,19 +1704,23 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
});

describe('loadUIReact()', () => {
test('should create controls root and render the react controls', () => {
beforeEach(() => {
docBase.pdfViewer = {
pagesCount: 3,
currentPageNumber: 2,
currentScale: 1,
};
docBase.options.useReactControls = true;
});

test('should create controls root and render the react controls', () => {
docBase.loadUIReact();

expect(docBase.controls).toBeInstanceOf(ControlsRoot);
expect(docBase.controls.render).toBeCalledWith(
<DocControls
annotationMode="none"
hasDrawing={false}
hasHighlight={false}
hasRegion={false}
maxScale={10}
Expand All @@ -1736,6 +1740,42 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
/>,
);
});

test.each`
areNewAnnotationsEnabled | hasAnnotationCreatePermission | hasDrawing | showAnnotationsDrawingCreate
${false} | ${false} | ${false} | ${false}
${false} | ${false} | ${false} | ${true}
${true} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${true}
${true} | ${false} | ${false} | ${false}
${false} | ${true} | ${false} | ${true}
${false} | ${true} | ${false} | ${false}
${true} | ${true} | ${true} | ${true}
`(
'should create controls root and render the react controls with hasDrawing set to $hasDrawing',
({
areNewAnnotationsEnabled,
hasAnnotationCreatePermission,
hasDrawing,
showAnnotationsDrawingCreate,
}) => {
docBase.options.showAnnotationsDrawingCreate = showAnnotationsDrawingCreate;
jest.spyOn(docBase, 'areNewAnnotationsEnabled').mockReturnValue(areNewAnnotationsEnabled);
jest.spyOn(docBase, 'hasAnnotationCreatePermission').mockReturnValue(hasAnnotationCreatePermission);
stubs.checkPermission.mockReturnValue(false);

docBase.loadUIReact();

expect(docBase.controls).toBeInstanceOf(ControlsRoot);
expect(docBase.controls.render).toBeCalledWith(
expect.objectContaining({
props: expect.objectContaining({
hasDrawing,
}),
}),
);
},
);
});

describe('bindDOMListeners()', () => {
Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/image/ImageControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type Props = AnnotationsControlsProps & FullscreenToggleProps & RotateCon

export default function ImageControls({
annotationMode,
hasDrawing,
hasHighlight,
hasRegion,
onAnnotationModeClick,
Expand All @@ -26,6 +27,7 @@ export default function ImageControls({
<FullscreenToggle onFullscreenToggle={onFullscreenToggle} />
<AnnotationsControls
annotationMode={annotationMode}
hasDrawing={hasDrawing}
hasHighlight={hasHighlight}
hasRegion={hasRegion}
onAnnotationModeClick={onAnnotationModeClick}
Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/image/ImageViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,12 @@ class ImageViewer extends ImageBaseViewer {
this.areNewAnnotationsEnabled() &&
this.hasAnnotationCreatePermission() &&
this.currentRotationAngle === 0;
const canDraw = canAnnotate && this.options.showAnnotationsDrawingCreate;

this.controls.render(
<ImageControls
annotationMode={this.annotationControlsFSM.getMode()}
hasDrawing={canDraw}
hasHighlight={false}
hasRegion={canAnnotate}
onAnnotationModeClick={this.handleAnnotationControlsClick}
Expand Down
36 changes: 35 additions & 1 deletion src/lib/viewers/image/__tests__/ImageViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,18 @@ describe('lib/viewers/image/ImageViewer', () => {
});

describe('loadUIReact()', () => {
test('should create controls root and render the react controls', () => {
beforeEach(() => {
image.options.useReactControls = true;
});

test('should create controls root and render the react controls', () => {
image.loadUIReact();

expect(image.controls).toBeInstanceOf(ControlsRoot);
expect(image.controls.render).toBeCalledWith(
<ImageControls
annotationMode="none"
hasDrawing={false}
ChenCodes marked this conversation as resolved.
Show resolved Hide resolved
hasHighlight={false}
hasRegion={false}
onAnnotationModeClick={image.handleAnnotationControlsClick}
Expand All @@ -441,6 +445,36 @@ describe('lib/viewers/image/ImageViewer', () => {
/>,
);
});

test.each`
areNewAnnotationsEnabled | hasAnnotationCreatePermission | hasDrawing | showAnnotationsDrawingCreate
${false} | ${false} | ${false} | ${false}
${false} | ${false} | ${false} | ${true}
${true} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${true}
${true} | ${false} | ${false} | ${false}
${false} | ${true} | ${false} | ${true}
${false} | ${true} | ${false} | ${false}
${true} | ${true} | ${true} | ${true}
`(
'should create controls root and render the react controls with hasDrawing set to $hasDrawing',
({ areNewAnnotationsEnabled, hasAnnotationCreatePermission, hasDrawing, showAnnotationsDrawingCreate }) => {
image.options.showAnnotationsDrawingCreate = showAnnotationsDrawingCreate;
jest.spyOn(image, 'areNewAnnotationsEnabled').mockReturnValue(areNewAnnotationsEnabled);
jest.spyOn(image, 'hasAnnotationCreatePermission').mockReturnValue(hasAnnotationCreatePermission);

image.loadUIReact();

expect(image.controls).toBeInstanceOf(ControlsRoot);
expect(image.controls.render).toBeCalledWith(
expect.objectContaining({
props: expect.objectContaining({
hasDrawing,
}),
}),
);
},
);
});

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