diff --git a/package.json b/package.json index 3891dced5..210598400 100644 --- a/package.json +++ b/package.json @@ -198,6 +198,9 @@ "coverageDirectory": "/reports", "collectCoverageFrom": [ "src/**/*.js", + "!src/utils/*.js", + "!third-party/*.js", + "!src/**/messages.js", "!**/node_modules/**", "!**/__tests__/**" ], diff --git a/src/components/ActionControls/__tests__/ActionControls-test.js b/src/components/ActionControls/__tests__/ActionControls-test.js index 5f0de3661..08e0d062f 100644 --- a/src/components/ActionControls/__tests__/ActionControls-test.js +++ b/src/components/ActionControls/__tests__/ActionControls-test.js @@ -3,7 +3,15 @@ import { shallow } from 'enzyme'; import ActionControls from '../ActionControls'; -const fn = jest.fn(); +const onCreate = jest.fn(); +const onCommentClick = jest.fn(); +const onCancel = jest.fn(); +const onDelete = jest.fn(); + +const event = { + stopPropagation: jest.fn(), + preventDefault: jest.fn() +}; describe('components/ActionControls', () => { const render = (props = {}) => @@ -11,46 +19,146 @@ describe('components/ActionControls', () => { ); - test('should correctly render controls for plain highlight annotations without delete permissions', () => { + test('should render no controls if the type does not match a valid type', () => { + const wrapper = render({ type: 'invalid' }); + expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeFalsy(); + expect(wrapper.find('DrawingControls').exists()).toBeFalsy(); + expect(wrapper.find('ApprovalCommentForm').exists()).toBeFalsy(); + }); + + test('should render no controls if user cannot delete or comment on a plain highlight annotation', () => { const wrapper = render({ type: 'highlight' }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeFalsy(); + expect(wrapper.find('DrawingControls').exists()).toBeFalsy(); + expect(wrapper.find('ApprovalCommentForm').exists()).toBeFalsy(); + }); + + test('should correctly render controls for plain highlight annotations without delete permissions', () => { + const wrapper = render({ type: 'highlight', canComment: true }); + expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeTruthy(); + }); + + test('should correctly render controls for plain highlight annotations without comment permissions', () => { + const wrapper = render({ type: 'highlight', canDelete: true }); + expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeTruthy(); }); test('should correctly render controls for plain highlight annotations', () => { const wrapper = render({ canDelete: true, type: 'highlight' }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeTruthy(); }); - test('should correctly render controls for highlight comment annotations without delete permissions', () => { - const wrapper = render({ type: 'highlight-comment' }); + test('should correctly render controls for pending highlight comment annotations', () => { + const wrapper = render({ isPending: true, type: 'highlight-comment' }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('ApprovalCommentForm').exists()).toBeTruthy(); + }); + + test('should correctly render controls for highlight comment annotations without any existing comments', () => { + const wrapper = render({ type: 'highlight-comment', hasComments: true }); + expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('ApprovalCommentForm').exists()).toBeTruthy(); }); test('should correctly render controls for highlight comment annotations', () => { - const wrapper = render({ canDelete: true, type: 'highlight-comment' }); + const wrapper = render({ type: 'highlight-comment' }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeTruthy(); }); - test('should correctly render controls for pending highlight comment annotations', () => { - const wrapper = render({ isPending: true, type: 'highlight-comment' }); + test('should render no controls if user cannot delete or comment on a plain highlight annotation', () => { + const wrapper = render({ type: 'draw' }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('HighlightControls').exists()).toBeFalsy(); + expect(wrapper.find('DrawingControls').exists()).toBeFalsy(); + expect(wrapper.find('ApprovalCommentForm').exists()).toBeFalsy(); }); test('should correctly render controls for drawing annotations', () => { - const wrapper = render({ type: 'draw' }); + const wrapper = render({ type: 'draw', canDelete: true }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('DrawingControls').exists()).toBeTruthy(); }); test('should correctly render controls for point annotations', () => { const wrapper = render({ type: 'point' }); expect(wrapper).toMatchSnapshot(); + expect(wrapper.find('ApprovalCommentForm').exists()).toBeTruthy(); + }); + + test('should update state and call onDelete() prop on delete button click', () => { + const wrapper = render({ canDelete: true, type: 'highlight' }); + const instance = wrapper.instance(); + + instance.onDelete(event); + wrapper.update(); + + expect(event.stopPropagation).toBeCalled(); + expect(event.preventDefault).toBeCalled(); + expect(onDelete).toBeCalled(); + expect(wrapper.state().isInputOpen).toEqual(false); + }); + + test('should update state and call onCreate() prop on create button click', () => { + const wrapper = render({ canDelete: true, type: 'highlight' }); + const instance = wrapper.instance(); + + instance.onCreate(event); + wrapper.update(); + + expect(onCreate).toBeCalled(); + expect(wrapper.state().isInputOpen).toEqual(true); + }); + + test('should update state and call onCancel() prop on cancel button click', () => { + const wrapper = render({ canDelete: true, type: 'highlight' }); + const instance = wrapper.instance(); + + instance.onCancel(event); + wrapper.update(); + + expect(event.stopPropagation).toBeCalled(); + expect(event.preventDefault).toBeCalled(); + expect(onCancel).toBeCalled(); + expect(wrapper.state().isInputOpen).toEqual(false); + }); + + test('should update state to open the input onFocus()', () => { + const wrapper = render({ canDelete: true, type: 'highlight' }); + const instance = wrapper.instance(); + + instance.onFocus(event); + wrapper.update(); + + expect(wrapper.state().isInputOpen).toEqual(true); + }); + + test('should update state to close the input componentWillUnmount()', () => { + const wrapper = render({ canDelete: true, type: 'highlight' }); + const instance = wrapper.instance(); + + instance.componentWillUnmount(event); + wrapper.update(); + + expect(wrapper.state().isInputOpen).toEqual(false); + }); + + test('should focus the input on componentDidMount()', () => { + const wrapper = render({ canDelete: true, type: 'highlight' }); + expect(wrapper.state().isInputOpen).toEqual(true); }); }); diff --git a/src/components/ActionControls/__tests__/__snapshots__/ActionControls-test.js.snap b/src/components/ActionControls/__tests__/__snapshots__/ActionControls-test.js.snap index febc4c931..52ef93f59 100644 --- a/src/components/ActionControls/__tests__/__snapshots__/ActionControls-test.js.snap +++ b/src/components/ActionControls/__tests__/__snapshots__/ActionControls-test.js.snap @@ -3,7 +3,14 @@ exports[`components/ActionControls should correctly render controls for drawing annotations 1`] = `
+> + +
`; exports[`components/ActionControls should correctly render controls for highlight comment annotations 1`] = ` @@ -11,7 +18,7 @@ exports[`components/ActionControls should correctly render controls for highligh className="ba-action-controls" > `; -exports[`components/ActionControls should correctly render controls for highlight comment annotations without delete permissions 1`] = ` +exports[`components/ActionControls should correctly render controls for highlight comment annotations without any existing comments 1`] = `
-
`; @@ -69,10 +79,34 @@ exports[`components/ActionControls should correctly render controls for plain hi `; +exports[`components/ActionControls should correctly render controls for plain highlight annotations without comment permissions 1`] = ` +
+ +
+`; + exports[`components/ActionControls should correctly render controls for plain highlight annotations without delete permissions 1`] = `
+> + +
`; exports[`components/ActionControls should correctly render controls for point annotations 1`] = ` @@ -92,3 +126,21 @@ exports[`components/ActionControls should correctly render controls for point an /> `; + +exports[`components/ActionControls should render no controls if the type does not match a valid type 1`] = ` +
+`; + +exports[`components/ActionControls should render no controls if user cannot delete or comment on a plain highlight annotation 1`] = ` +
+`; + +exports[`components/ActionControls should render no controls if user cannot delete or comment on a plain highlight annotation 2`] = ` +
+`; diff --git a/src/components/AnnotationPopover/AnnotatorLabel.js b/src/components/AnnotationPopover/AnnotatorLabel.js index 7c29f7dc0..9c6e53e81 100644 --- a/src/components/AnnotationPopover/AnnotatorLabel.js +++ b/src/components/AnnotationPopover/AnnotatorLabel.js @@ -20,36 +20,30 @@ type Props = { intl: any }; -class AnnotatorLabel extends React.PureComponent { - /** - * Return a string describes the action completed by the annotator - * @return {string} Localized annotator label message - */ - getAnnotatorLabelMessage(): string { - const { type, createdBy, intl } = this.props; - const anonymousUserName = intl.formatMessage(messages.anonymousUserName); - const name = get(createdBy, 'name', anonymousUserName); - - let labelMessage = messages.whoAnnotated; - if (isHighlightAnnotation(type)) { - labelMessage = messages.whoHighlighted; - } else if (isDrawingAnnotation(type)) { - labelMessage = messages.whoDrew; - } - - return intl.formatMessage(labelMessage, { name }); +const AnnotatorLabel = ({ id, isPending, type, createdBy, intl }: Props) => { + if (isPending) { + return null; } - render() { - const { id, isPending } = this.props; - return ( - !isPending && ( - - - - ) - ); + const anonymousUserName = intl.formatMessage(messages.anonymousUserName); + const name = get(createdBy, 'name', anonymousUserName); + + let labelMessage = messages.whoAnnotated; + if (isHighlightAnnotation(type)) { + labelMessage = messages.whoHighlighted; + } else if (isDrawingAnnotation(type)) { + labelMessage = messages.whoDrew; } -} + + return ( + + + + ); +}; export default injectIntl(AnnotatorLabel); diff --git a/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js b/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js index fe7101505..ca3bb76d0 100644 --- a/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js +++ b/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js @@ -5,6 +5,7 @@ import AnnotationPopover from '../AnnotationPopover'; import { TYPES } from '../../../constants'; const fnMock = jest.fn(); +const position = jest.fn(); const TIME_STRING_SEPT_27_2017 = '2017-09-27T10:40:41-07:00'; @@ -41,17 +42,22 @@ describe('components/AnnotationPopover', () => { onCreate={fnMock} onCancel={fnMock} onDelete={fnMock} - position={fnMock} + position={position} {...props} /> ); + test('should position the component on componentDidUpdate()', () => { + const wrapper = render({ comments }); + wrapper.setProps({ canAnnotate: true }); + expect(position).toBeCalled(); + }); + test('should correctly render a view-only popover with comments', () => { - const wrapper = render({ - comments - }); + const wrapper = render({ comments }); expect(wrapper).toMatchSnapshot(); expect(wrapper.find('.ba-inline-popover').length).toEqual(0); + expect(position).toBeCalled(); }); test('should render a view-only annotation with a annotator label and no comments', () => { diff --git a/src/components/AnnotationPopover/__tests__/AnnotatorLabel-test.js b/src/components/AnnotationPopover/__tests__/AnnotatorLabel-test.js index e557486b3..d96328713 100644 --- a/src/components/AnnotationPopover/__tests__/AnnotatorLabel-test.js +++ b/src/components/AnnotationPopover/__tests__/AnnotatorLabel-test.js @@ -11,7 +11,12 @@ const createdBy = { }; describe('components/AnnotationPopover/AnnotatorLabel', () => { - const render = (props = {}) => shallow(); + const render = (props = {}) => shallow().dive(); + + test('should render nothing if label is pending', () => { + const wrapper = render({ isPending: true }); + expect(wrapper).toMatchSnapshot(); + }); test('should render a highlight annotation label', () => { const wrapper = render({ diff --git a/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap b/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap index d2d255d99..e33e64b9b 100644 --- a/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap +++ b/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap @@ -10,17 +10,7 @@ exports[`components/AnnotationPopover should correctly render a div without a Fo defaultMessage="Close Popover" id="ba.closePopover" />, - "handler": [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - }, + "handler": [MockFunction], "key": "esc", "type": "Close", }, @@ -42,19 +32,7 @@ exports[`components/AnnotationPopover should correctly render a div without a Fo >
@@ -170,17 +100,7 @@ exports[`components/AnnotationPopover should correctly render a pending annotati defaultMessage="Close Popover" id="ba.closePopover" />, - "handler": [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - }, + "handler": [MockFunction], "key": "esc", "type": "Close", }, @@ -206,46 +126,10 @@ exports[`components/AnnotationPopover should correctly render a pending annotati canDelete={false} hasComments={false} isPending={true} - onCancel={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } + onCancel={[MockFunction]} onCommentClick={[Function]} - onCreate={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } - onDelete={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } + onCreate={[MockFunction]} + onDelete={[MockFunction]} />
@@ -263,17 +147,7 @@ exports[`components/AnnotationPopover should correctly render a popover with com defaultMessage="Close Popover" id="ba.closePopover" />, - "handler": [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - }, + "handler": [MockFunction], "key": "esc", "type": "Close", }, @@ -320,65 +194,17 @@ exports[`components/AnnotationPopover should correctly render a popover with com }, ] } - onDelete={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } + onDelete={[MockFunction]} /> @@ -396,17 +222,7 @@ exports[`components/AnnotationPopover should correctly render a view-only popove defaultMessage="Close Popover" id="ba.closePopover" />, - "handler": [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - }, + "handler": [MockFunction], "key": "esc", "type": "Close", }, @@ -453,19 +269,7 @@ exports[`components/AnnotationPopover should correctly render a view-only popove }, ] } - onDelete={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } + onDelete={[MockFunction]} /> @@ -483,17 +287,7 @@ exports[`components/AnnotationPopover should correctly render an annotation with defaultMessage="Close Popover" id="ba.closePopover" />, - "handler": [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - }, + "handler": [MockFunction], "key": "esc", "type": "Close", }, @@ -520,46 +314,10 @@ exports[`components/AnnotationPopover should correctly render an annotation with canDelete={false} hasComments={false} isPending={false} - onCancel={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } + onCancel={[MockFunction]} onCommentClick={[Function]} - onCreate={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } - onDelete={ - [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - } - } + onCreate={[MockFunction]} + onDelete={[MockFunction]} type="highlight" /> @@ -578,17 +336,7 @@ exports[`components/AnnotationPopover should render a view-only annotation with defaultMessage="Close Popover" id="ba.closePopover" />, - "handler": [MockFunction] { - "calls": Array [ - Array [], - ], - "results": Array [ - Object { - "isThrow": false, - "value": undefined, - }, - ], - }, + "handler": [MockFunction], "key": "esc", "type": "Close", }, diff --git a/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotatorLabel-test.js.snap b/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotatorLabel-test.js.snap index c3b0fe396..391460421 100644 --- a/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotatorLabel-test.js.snap +++ b/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotatorLabel-test.js.snap @@ -1,69 +1,51 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`components/AnnotationPopover/AnnotatorLabel should render a drawing annotation label 1`] = ` - + + + `; exports[`components/AnnotationPopover/AnnotatorLabel should render a highlight annotation label 1`] = ` - + + + `; exports[`components/AnnotationPopover/AnnotatorLabel should render an anonymous drawing annotation label 1`] = ` - + + + `; exports[`components/AnnotationPopover/AnnotatorLabel should render an anonymous highlight annotation label 1`] = ` - + + + `; + +exports[`components/AnnotationPopover/AnnotatorLabel should render nothing if label is pending 1`] = `""`; diff --git a/src/components/CommentList/__tests__/CommentList-test.js b/src/components/CommentList/__tests__/CommentList-test.js index a0d5e5e48..6556c3109 100644 --- a/src/components/CommentList/__tests__/CommentList-test.js +++ b/src/components/CommentList/__tests__/CommentList-test.js @@ -37,4 +37,13 @@ describe('components/CommentList', () => { expect(wrapper).toMatchSnapshot(); expect(wrapper.find('.ba-comment-list-item').length).toEqual(2); }); + + test('should scroll to the bottom on componentDidUpdate()', () => { + const wrapper = shallow(); + const instance = wrapper.instance(); + instance.scrollToBottom = jest.fn(); + + wrapper.setProps({ comments: [...comments, ...comments] }); + expect(instance.scrollToBottom).toBeCalled(); + }); }); diff --git a/src/components/Internationalize.js b/src/components/Internationalize.js index e8c843259..5d0231f26 100644 --- a/src/components/Internationalize.js +++ b/src/components/Internationalize.js @@ -7,7 +7,7 @@ import React, { Children } from 'react'; import { IntlProvider, addLocaleData } from 'react-intl'; -import i18n from '../i18n'; +import i18n from '../utils/i18n'; type Props = { children?: any diff --git a/src/doc/__tests__/DocPointThread-test.js b/src/doc/__tests__/DocPointThread-test.js index 3f8e24f0f..29d244193 100644 --- a/src/doc/__tests__/DocPointThread-test.js +++ b/src/doc/__tests__/DocPointThread-test.js @@ -2,7 +2,7 @@ import DocPointThread from '../DocPointThread'; import * as util from '../../util'; import * as docUtil from '../docUtil'; -import { STATES, SELECTOR_ANNOTATED_ELEMENT } from '../../constants'; +import { STATES, SELECTOR_ANNOTATED_ELEMENT, CLASS_FLIPPED_POPOVER } from '../../constants'; let thread; const html = '
'; @@ -69,4 +69,33 @@ describe('doc/DocPointThread', () => { expect(thread.renderAnnotationPopover).not.toBeCalled(); }); }); + + describe('position()', () => { + beforeEach(() => { + util.shouldDisplayMobileUI = jest.fn().mockReturnValue(false); + util.findElement = jest.fn().mockReturnValue(rootElement); + thread.getPopoverParent = jest.fn().mockReturnValue(rootElement); + util.repositionCaret = jest.fn().mockReturnValue(0); + }); + + it('should not re-position the popover on mobile devices', () => { + util.shouldDisplayMobileUI = jest.fn().mockReturnValue(true); + thread.position(); + expect(util.findElement).not.toBeCalled(); + }); + + it('should position desktop popovers', () => { + util.isInUpperHalf = jest.fn().mockReturnValue(true); + thread.position(); + expect(util.findElement).toBeCalled(); + expect(rootElement.classList).not.toContain(CLASS_FLIPPED_POPOVER); + }); + + it('should flip popovers in the lower half of the viewport', () => { + util.isInUpperHalf = jest.fn().mockReturnValue(false); + thread.position(); + expect(util.findElement).toBeCalled(); + expect(rootElement.classList).toContain(CLASS_FLIPPED_POPOVER); + }); + }); }); diff --git a/src/drawing/DrawingPath.js b/src/drawing/DrawingPath.js index 3219334f6..9fb010185 100644 --- a/src/drawing/DrawingPath.js +++ b/src/drawing/DrawingPath.js @@ -27,20 +27,40 @@ class DrawingPath { * @return {DrawingPath} A DrawingPath instance */ constructor(drawingPathData: ?DrawingPath) { - if (drawingPathData) { - // $FlowFixMe - this.path = drawingPathData.path.map((num) => { - const x = +num.x; - const y = +num.y; - - this.minX = Math.min(this.minX, x); - this.maxX = Math.max(this.maxX, x); - this.minY = Math.min(this.minY, y); - this.maxY = Math.max(this.maxY, y); - - return createLocation(x, y); - }); + this.initPath(drawingPathData); + } + + /** + * Updates the path min/max coordinates if necessary + * @param {number} x - X path coordinate + * @param {number} y - Y path coordinate + * @return {void} + */ + updatePathBounds(x: number, y: number) { + this.minX = Math.min(this.minX, x); + this.maxX = Math.max(this.maxX, x); + this.minY = Math.min(this.minY, y); + this.maxY = Math.max(this.maxY, y); + } + + /** + * Initialize a path from passed in data + * @param {DrawingPath} drawingPathData - The drawingPath object data to be instantiated into an object + * @return {void} + */ + initPath(drawingPathData: ?DrawingPath) { + if (!drawingPathData) { + return; } + + // $FlowFixMe + this.path = drawingPathData.path.map((num: Coordinates) => { + const x = +num.x; + const y = +num.y; + + this.updatePathBounds(x, y); + return createLocation(x, y); + }); } /** @@ -103,24 +123,23 @@ class DrawingPath { return; } - const pathLen = this.browserPath.length; - for (let i = 0; i < pathLen; i++) { + this.browserPath.forEach((coordinate, index) => { let xLast; let yLast; - if (i > 0) { - xLast = this.browserPath[i - 1].x; - yLast = this.browserPath[i - 1].y; + if (index > 0) { + xLast = this.browserPath[index - 1].x; + yLast = this.browserPath[index - 1].y; } else { - xLast = this.browserPath[i].x; - yLast = this.browserPath[i].y; + xLast = coordinate.x; + yLast = coordinate.y; ctx.moveTo(xLast, yLast); } - const xMid = (this.browserPath[i].x + xLast) / 2; - const yMid = (this.browserPath[i].y + yLast) / 2; + const xMid = (coordinate.x + xLast) / 2; + const yMid = (coordinate.y + yLast) / 2; ctx.quadraticCurveTo(xLast, yLast, xMid, yMid); - } + }); } /** diff --git a/src/drawing/DrawingThread.js b/src/drawing/DrawingThread.js index 5b5490814..1312dc607 100644 --- a/src/drawing/DrawingThread.js +++ b/src/drawing/DrawingThread.js @@ -75,19 +75,28 @@ class DrawingThread extends AnnotationThread { this.redo = this.redo.bind(this); this.canComment = false; - // Recreate stored paths - if (this.location && this.location.paths) { - this.regenerateBoundary(); - - if (this.pathContainer.isEmpty()) { - this.unmountPopover(); - } - - this.location.paths.forEach((drawingPathData) => { - const pathInstance = new DrawingPath(drawingPathData); - this.pathContainer.insert(pathInstance); - }); + this.initStoredPaths(); + } + + /** + * Recreate stored paths + * @return {void} + */ + initStoredPaths() { + if (!this.location || !this.location.paths) { + return; } + + this.regenerateBoundary(); + + if (this.pathContainer.isEmpty()) { + this.unmountPopover(); + } + + this.location.paths.forEach((drawingPathData) => { + const pathInstance = new DrawingPath(drawingPathData); + this.pathContainer.insert(pathInstance); + }); } /** diff --git a/src/drawing/__tests__/DrawingPath-test.js b/src/drawing/__tests__/DrawingPath-test.js index 95ddfa21f..22c3817fc 100644 --- a/src/drawing/__tests__/DrawingPath-test.js +++ b/src/drawing/__tests__/DrawingPath-test.js @@ -1,4 +1,3 @@ -/* eslint-disable no-unused-expressions */ import DrawingPath from '../DrawingPath'; let drawingPath; @@ -12,6 +11,25 @@ describe('drawing/DrawingPath', () => { drawingPath = null; }); + describe('initPath()', () => { + it('should do nothing if no path data was provided', () => { + drawingPath.initPath(); + expect(drawingPath.path.length).toEqual(0); + }); + + it('should generate path of Location objects from provided data', () => { + const data = { + path: [{ x: 1, y: 1 }, { x: 2, y: 2 }] + }; + drawingPath.initPath(data); + expect(drawingPath.path.length).toEqual(2); + expect(drawingPath.minX).toEqual(1); + expect(drawingPath.minY).toEqual(1); + expect(drawingPath.maxX).toEqual(2); + expect(drawingPath.maxY).toEqual(2); + }); + }); + describe('addCoordinate()', () => { it('should do nothing if x or y is empty', () => { let lengthBefore = drawingPath.path.length; @@ -108,6 +126,16 @@ describe('drawing/DrawingPath', () => { }); describe('drawPath()', () => { + it('should do nothing when no context or browser path exist', () => { + drawingPath.drawPath({}); + + drawingPath.browserPath = { + forEach: jest.fn() + }; + drawingPath.drawPath(); + expect(drawingPath.browserPath.forEach).not.toBeCalled(); + }); + it('should draw when there are browser coordinates', () => { const context = { quadraticCurveTo: jest.fn(), @@ -122,11 +150,12 @@ describe('drawing/DrawingPath', () => { y: 1 }; + drawingPath.addCoordinate(docCoord, browserCoord); drawingPath.addCoordinate(docCoord, browserCoord); drawingPath.drawPath(context); - expect(context.quadraticCurveTo).toBeCalled(); - expect(context.moveTo).toBeCalled(); + expect(context.quadraticCurveTo).toBeCalledTimes(2); + expect(context.moveTo).toBeCalledTimes(1); }); it('should not draw when there are no browser coordinates', () => { @@ -149,6 +178,13 @@ describe('drawing/DrawingPath', () => { }); describe('generateBrowserPath()', () => { + it('should do nothing when no path exists', () => { + const transform = jest.fn(); + drawingPath.path = undefined; + drawingPath.generateBrowserPath(transform); + expect(transform).not.toBeCalled(); + }); + it('should generate the browser path', () => { const lengthBefore = drawingPath.browserPath.length; diff --git a/src/drawing/__tests__/DrawingThread-test.js b/src/drawing/__tests__/DrawingThread-test.js index cc5651f4b..844d6ee4a 100644 --- a/src/drawing/__tests__/DrawingThread-test.js +++ b/src/drawing/__tests__/DrawingThread-test.js @@ -1,7 +1,11 @@ /* eslint-disable no-unused-expressions */ import DrawingThread from '../DrawingThread'; -import { STATES } from '../../constants'; +import { STATES, TYPES } from '../../constants'; import * as util from '../../util'; +import DrawingPath from '../DrawingPath'; +import DrawingContainer from '../DrawingContainer'; + +jest.mock('../DrawingPath'); let thread; @@ -35,6 +39,35 @@ describe('drawing/DrawingThread', () => { thread = null; }); + describe('initStoredPaths', () => { + beforeEach(() => { + thread.regenerateBoundary = jest.fn(); + thread.unmountPopover = jest.fn(); + thread.pathContainer = new DrawingContainer(); + thread.pathContainer.insert = jest.fn(); + thread.location = { + paths: [{}, {}] + }; + }); + + it('should do nothing if no location or paths exist', () => { + thread.location = undefined; + thread.initStoredPaths(); + + thread.location = {}; + thread.initStoredPaths(); + expect(thread.regenerateBoundary).not.toBeCalled(); + }); + + it('should unmount popover if the path container is empty', () => { + thread.pathContainer.isEmpty = jest.fn().mockReturnValue(true); + thread.initStoredPaths(); + expect(thread.regenerateBoundary).toBeCalled(); + expect(thread.unmountPopover).toBeCalled(); + expect(thread.pathContainer.insert).toBeCalledTimes(2); + }); + }); + describe('destroy()', () => { beforeEach(() => { thread.state = STATES.pending; @@ -63,29 +96,6 @@ describe('drawing/DrawingThread', () => { }); }); - describe('deleteThread()', () => { - it('should delete all attached annotations, clear the drawn rectangle, and call destroy', () => { - thread.clearBoundary = jest.fn(); - thread.delete = jest.fn(); - thread.getBrowserRectangularBoundary = jest.fn().mockReturnValue(['a', 'b', 'c', 'd']); - thread.concreteContext = { - clearRect: jest.fn() - }; - - thread.pathContainer = { - destroy: jest.fn() - }; - - thread.comments = [{ id: '123abc' }]; - - thread.deleteThread(); - expect(thread.getBrowserRectangularBoundary).toBeCalled(); - expect(thread.concreteContext.clearRect).toBeCalled(); - expect(thread.clearBoundary).toBeCalled(); - expect(thread.pathContainer).toEqual(null); - }); - }); - describe('bindDrawingListeners()', () => { beforeEach(() => { thread.hasTouch = false; @@ -114,6 +124,14 @@ describe('drawing/DrawingThread', () => { }); }); + describe('unbindDOMListeners()', () => { + it('should also unbind drawing listeners', () => { + thread.unbindDrawingListeners = jest.fn(); + thread.unbindDOMListeners(); + expect(thread.unbindDrawingListeners).toBeCalled(); + }); + }); + describe('unbindDrawingListeners()', () => { beforeEach(() => { thread.annotatedElement = { @@ -133,60 +151,53 @@ describe('drawing/DrawingThread', () => { }); }); - describe('setContextStyles()', () => { - it('should set configurable context properties', () => { - thread.drawingContext = { - lineCap: 'not set', - lineJoin: 'not set', - strokeStyle: 'no color', - lineWidth: 'no width' + describe('deleteThread()', () => { + it('should delete all attached annotations, clear the drawn rectangle, and call destroy', () => { + thread.clearBoundary = jest.fn(); + thread.delete = jest.fn(); + thread.getBrowserRectangularBoundary = jest.fn().mockReturnValue(['a', 'b', 'c', 'd']); + thread.concreteContext = { + clearRect: jest.fn() }; - const config = { - scale: 2, - color: 'blue' + thread.pathContainer = { + destroy: jest.fn() }; - thread.setContextStyles(config); - expect(thread.drawingContext).toStrictEqual({ - lineCap: 'round', - lineJoin: 'round', - strokeStyle: 'blue', - lineWidth: thread.drawingContext.lineWidth - }); - expect(thread.drawingContext.lineWidth % config.scale).toEqual(0); - }); - }); - - describe('render()', () => { - beforeEach(() => { - thread.draw = jest.fn(); - }); - - it('should draw the pending path when the context is not empty', () => { - const timeStamp = 20000; - thread.render(timeStamp); - expect(thread.draw).toBeCalled(); - }); + thread.comments = [{ id: '123abc' }]; - it('should do nothing when the timeElapsed is less than the refresh rate', () => { - const timeStamp = 100; - thread.lastRenderTimestamp = 100; - thread.render(timeStamp); - expect(thread.draw).not.toBeCalled(); + thread.deleteThread(); + expect(thread.getBrowserRectangularBoundary).toBeCalled(); + expect(thread.concreteContext.clearRect).toBeCalled(); + expect(thread.clearBoundary).toBeCalled(); + expect(thread.pathContainer).toEqual(null); }); }); - describe('setup()', () => { - it('should set the state to be pending when there are no saved annotations', () => { - thread.setup(); - expect(thread.state).toEqual(STATES.pending); + describe('setContextStyles()', () => { + const config = { + scale: 2, + color: 'blue' + }; + + const context = { + lineCap: 'not set', + lineJoin: 'not set', + strokeStyle: 'no color', + lineWidth: 'no width' + }; + + it('should do nothing when no context exists', () => { + thread.setContextStyles(config); + expect(thread.drawingContext).not.toStrictEqual(context); }); - it('should set the state to be inactive when there are saved annotations', () => { - thread.threadNumber = '123'; - thread.setup(); - expect(thread.state).toEqual(STATES.inactive); + it('should set configurable context properties', () => { + thread.drawingContext = context; + + thread.setContextStyles(config); + expect(thread.drawingContext).toStrictEqual(context); + expect(thread.drawingContext.lineWidth % config.scale).toEqual(0); }); }); @@ -261,6 +272,19 @@ describe('drawing/DrawingThread', () => { }); }); + describe('setup()', () => { + it('should set the state to be pending when there are no saved annotations', () => { + thread.setup(); + expect(thread.state).toEqual(STATES.pending); + }); + + it('should set the state to be inactive when there are saved annotations', () => { + thread.threadNumber = '123'; + thread.setup(); + expect(thread.state).toEqual(STATES.inactive); + }); + }); + describe('draw()', () => { let context; @@ -337,7 +361,72 @@ describe('drawing/DrawingThread', () => { }); }); + describe('drawBoundary()', () => { + it('should clear the boundary', () => { + thread.clearBoundary = jest.fn(); + thread.drawBoundary(); + expect(thread.clearBoundary).toBeCalled(); + }); + }); + + describe('render()', () => { + beforeEach(() => { + thread.draw = jest.fn(); + }); + + it('should draw the pending path when the context is not empty', () => { + const timeStamp = 20000; + thread.render(timeStamp); + expect(thread.draw).toBeCalled(); + }); + + it('should do nothing when the timeElapsed is less than the refresh rate', () => { + const timeStamp = 100; + thread.lastRenderTimestamp = 100; + thread.render(timeStamp); + expect(thread.draw).not.toBeCalled(); + }); + }); + + describe('renderAnnotationPopover()', () => { + it('should re-draw the boundary and render the popover', () => { + thread.drawBoundary = jest.fn(); + thread.position = jest.fn(); + thread.renderAnnotationPopover(); + expect(thread.drawBoundary).toBeCalled(); + }); + }); + + describe('updateBoundary()', () => { + beforeEach(() => { + thread.pathContainer = { + getAxisAlignedBoundingBox: jest.fn() + }; + DrawingPath.extractDrawingInfo = jest.fn(); + }); + + it('should recompute boundary when no item is provided', () => { + thread.updateBoundary(); + expect(thread.pathContainer.getAxisAlignedBoundingBox).toBeCalled(); + }); + + it('should extract drawing info from drawing path when item is provided', () => { + thread.updateBoundary({}); + expect(thread.pathContainer.getAxisAlignedBoundingBox).not.toBeCalled(); + }); + }); + describe('regenerateBoundary()', () => { + it('should do nothing when drawing has no location', () => { + thread.location = undefined; + + thread.regenerateBoundary(); + expect(thread.minX).toBeUndefined(); + expect(thread.maxX).toBeUndefined(); + expect(thread.minY).toBeUndefined(); + expect(thread.maxY).toBeUndefined(); + }); + it('should do nothing when no drawingPaths have been saved', () => { thread.location = {}; @@ -375,4 +464,21 @@ describe('drawing/DrawingThread', () => { expect(rootElement.removeChild).toBeCalled(); }); }); + + describe('createAnnotationData()', () => { + it('should set the drawingPaths in the details blob of the annotation', () => { + const pathContainer = 'container'; + thread.pathContainer = pathContainer; + const data = thread.createAnnotationData(TYPES.draw); + expect(data.details.drawingPaths).toEqual(pathContainer); + }); + }); + + describe('cleanupAnnotationOnDelete()', () => { + it('should set the threadID to null', () => { + thread.threadID = '123'; + thread.cleanupAnnotationOnDelete(); + expect(thread.threadID).toBeNull(); + }); + }); }); diff --git a/src/image/__tests__/ImageAnnotator-test.js b/src/image/__tests__/ImageAnnotator-test.js index 5002eaead..2d3dd8248 100644 --- a/src/image/__tests__/ImageAnnotator-test.js +++ b/src/image/__tests__/ImageAnnotator-test.js @@ -167,6 +167,16 @@ describe('image/ImageAnnotator', () => { }); }); + describe('scaleAnnotations()', () => { + it('should set the scale and rotate the annotations appropriately', () => { + annotator.setScale = jest.fn(); + annotator.rotateAnnotations = jest.fn(); + annotator.scaleAnnotations({ scale: 2, rotationAngle: 90, pageNum: 1 }); + expect(annotator.setScale).toBeCalledWith(2); + expect(annotator.rotateAnnotations).toBeCalledWith(90, 1); + }); + }); + describe('rotateAnnotations()', () => { beforeEach(() => { annotator.permissions.can_annotate = true; @@ -180,12 +190,19 @@ describe('image/ImageAnnotator', () => { annotator.modeButtons = {}; }); + it('should only re-render the specified page annotations', () => { + annotator.rotateAnnotations(90, 1); + expect(annotator.renderPage).toBeCalled(); + expect(annotator.render).not.toBeCalled(); + }); + it('should only render annotations if user cannot annotate', () => { annotator.permissions.can_annotate = false; annotator.rotateAnnotations(); expect(util.hideElement).not.toBeCalled(); expect(util.showElement).not.toBeCalled(); expect(annotator.render).toBeCalled(); + expect(annotator.renderPage).not.toBeCalled(); }); it('should hide point annotation button if image is rotated', () => { @@ -193,6 +210,7 @@ describe('image/ImageAnnotator', () => { expect(util.hideElement).toBeCalled(); expect(util.showElement).not.toBeCalled(); expect(annotator.render).toBeCalled(); + expect(annotator.renderPage).not.toBeCalled(); }); it('should show point annotation button if image is rotated', () => { @@ -200,6 +218,23 @@ describe('image/ImageAnnotator', () => { expect(util.hideElement).not.toBeCalled(); expect(util.showElement).toBeCalled(); expect(annotator.render).toBeCalled(); + expect(annotator.renderPage).not.toBeCalled(); + }); + }); + + describe('bindDOMListeners()', () => { + it('should bind the mouseup handler', () => { + annotator.annotatedElement.addEventListener = jest.fn(); + annotator.bindDOMListeners(); + expect(annotator.annotatedElement.addEventListener).toBeCalledWith('mouseup', annotator.hideAnnotations); + }); + }); + + describe('unbindDOMListeners()', () => { + it('should unbind the mouseup handler', () => { + annotator.annotatedElement.removeEventListener = jest.fn(); + annotator.unbindDOMListeners(); + expect(annotator.annotatedElement.removeEventListener).toBeCalledWith('mouseup', annotator.hideAnnotations); }); }); }); diff --git a/src/image/__tests__/ImagePointThread-test.js b/src/image/__tests__/ImagePointThread-test.js index e84b0920a..bb25f72a5 100644 --- a/src/image/__tests__/ImagePointThread-test.js +++ b/src/image/__tests__/ImagePointThread-test.js @@ -1,7 +1,7 @@ /* eslint-disable no-unused-expressions */ import ImagePointThread from '../ImagePointThread'; import * as util from '../../util'; -import { STATES, SELECTOR_ANNOTATED_ELEMENT } from '../../constants'; +import { STATES, SELECTOR_ANNOTATED_ELEMENT, CLASS_FLIPPED_POPOVER } from '../../constants'; import * as imageUtil from '../imageUtil'; let thread; @@ -82,6 +82,35 @@ describe('image/ImagePointThread', () => { }); }); + describe('position()', () => { + beforeEach(() => { + util.shouldDisplayMobileUI = jest.fn().mockReturnValue(false); + util.findElement = jest.fn().mockReturnValue(rootElement); + thread.getPopoverParent = jest.fn().mockReturnValue(rootElement); + util.repositionCaret = jest.fn().mockReturnValue(0); + }); + + it('should not re-position the popover on mobile devices', () => { + util.shouldDisplayMobileUI = jest.fn().mockReturnValue(true); + thread.position(); + expect(util.findElement).not.toBeCalled(); + }); + + it('should position desktop popovers', () => { + util.isInUpperHalf = jest.fn().mockReturnValue(true); + thread.position(); + expect(util.findElement).toBeCalled(); + expect(rootElement.classList).not.toContain(CLASS_FLIPPED_POPOVER); + }); + + it('should flip popovers in the lower half of the viewport', () => { + util.isInUpperHalf = jest.fn().mockReturnValue(false); + thread.position(); + expect(util.findElement).toBeCalled(); + expect(rootElement.classList).toContain(CLASS_FLIPPED_POPOVER); + }); + }); + describe('getPopoverParent()', () => { beforeEach(() => { util.shouldDisplayMobileUI = jest.fn(); diff --git a/src/MomentShim.js b/src/utils/MomentShim.js similarity index 100% rename from src/MomentShim.js rename to src/utils/MomentShim.js diff --git a/src/i18n.js b/src/utils/i18n.js similarity index 100% rename from src/i18n.js rename to src/utils/i18n.js diff --git a/src/polyfill.js b/src/utils/polyfill.js similarity index 100% rename from src/polyfill.js rename to src/utils/polyfill.js