diff --git a/src/__tests__/AnnotationThread-test.js b/src/__tests__/AnnotationThread-test.js index e78dae169..e65166fcf 100644 --- a/src/__tests__/AnnotationThread-test.js +++ b/src/__tests__/AnnotationThread-test.js @@ -1,4 +1,5 @@ /* eslint-disable no-unused-expressions */ +import * as ReactDOM from 'react-dom'; import AnnotationThread from '../AnnotationThread'; import * as util from '../util'; import { @@ -117,6 +118,19 @@ describe('AnnotationThread', () => { }); }); + describe('renderAnnotationPopover()', () => { + it('should render and display the popover for this annotation', () => { + thread.getPopoverParent = jest.fn().mockReturnValue(rootElement); + util.getPopoverLayer = jest.fn().mockReturnValue(rootElement); + util.shouldDisplayMobileUI = jest.fn().mockReturnValue(false); + ReactDOM.render = jest.fn(); + thread.position = jest.fn(); + + thread.renderAnnotationPopover(); + expect(thread.popoverComponent).not.toBeUndefined(); + }); + }); + describe('hide()', () => { it('should hide the thread element', () => { thread.unmountPopover = jest.fn(); diff --git a/src/_common.scss b/src/_common.scss index 44ec2ee9c..9a2ad2962 100644 --- a/src/_common.scss +++ b/src/_common.scss @@ -14,8 +14,6 @@ src: local('Lato Bold'), local('Lato-Bold'), url('https://cdn01.boxcdn.net/fonts/1.0.2/lato/Lato-Bold.woff2') format('woff2'), url('https://cdn01.boxcdn.net/fonts/1.0.2/lato/Lato-Bold.woff') format('woff'); } -@import '~box-react-ui/lib/styles/common/overlay'; - .ba { @include reset; @@ -96,11 +94,6 @@ @import '~box-react-ui/lib/styles/common/buttons'; } -.flyout-overlay { - @import '~box-react-ui/lib/styles/common/links'; - @import '~box-react-ui/lib/styles/common/buttons'; -} - /************************************** * Accessibility **************************************/ diff --git a/src/components/AnnotationPopover/AnnotationPopover.js b/src/components/AnnotationPopover/AnnotationPopover.js index 0068b94f4..0ab916f35 100644 --- a/src/components/AnnotationPopover/AnnotationPopover.js +++ b/src/components/AnnotationPopover/AnnotationPopover.js @@ -6,7 +6,6 @@ import PlainButton from 'box-react-ui/lib/components/plain-button'; import IconClose from 'box-react-ui/lib/icons/general/IconClose'; import Internationalize from '../Internationalize'; -import Overlay from './Overlay'; import CommentList from '../CommentList'; import { TYPES, CLASS_ANNOTATION_POPOVER, CLASS_ANNOTATION_CARET } from '../../constants'; @@ -19,6 +18,7 @@ const CLASS_ANIMATE_POPOVER = 'ba-animate-popover'; const CLASS_CREATE_POPOVER = 'ba-create-popover'; const CLASS_MOBILE_HEADER = 'ba-mobile-header'; const CLASS_MOBILE_CLOSE_BTN = 'ba-mobile-close-btn'; +const CLASS_POPOVER_OVERLAY = 'ba-popover-overlay'; type Props = { isMobile: boolean, @@ -99,7 +99,7 @@ class AnnotationPopover extends React.PureComponent { ) : ( )} - +
{hasComments ? ( ) : ( @@ -121,7 +121,7 @@ class AnnotationPopover extends React.PureComponent { onCommentClick={onCommentClick} /> )} - +
); diff --git a/src/components/AnnotationPopover/AnnotationPopover.scss b/src/components/AnnotationPopover/AnnotationPopover.scss index 5e7a20f62..bc307db92 100644 --- a/src/components/AnnotationPopover/AnnotationPopover.scss +++ b/src/components/AnnotationPopover/AnnotationPopover.scss @@ -40,7 +40,8 @@ } } - .overlay { + .ba-popover-overlay { + background: white; border: 1px solid $seesee; border-radius: 4px; display: block; @@ -50,7 +51,7 @@ white-space: normal; } - .ba-inline-popover .overlay { + .ba-inline-popover .ba-popover-overlay { align-items: center; display: inline-flex; } diff --git a/src/components/AnnotationPopover/Overlay.js b/src/components/AnnotationPopover/Overlay.js deleted file mode 100644 index 0bea9a533..000000000 --- a/src/components/AnnotationPopover/Overlay.js +++ /dev/null @@ -1,25 +0,0 @@ -// @flow -import * as React from 'react'; - -import OverlayComponent from 'box-react-ui/lib/components/flyout/Overlay'; - -const CLASS_POPOVER_OVERLAY = 'ba-popover-overlay'; - -type Props = { - shouldDefaultFocus: boolean, - children: React.Node -}; - -const Overlay = ({ shouldDefaultFocus, children }: Props) => { - if (shouldDefaultFocus) { - return ( - - {children} - - ); - } - - return
{children}
; -}; - -export default Overlay; diff --git a/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js b/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js index 5b6aa52ff..fe7101505 100644 --- a/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js +++ b/src/components/AnnotationPopover/__tests__/AnnotationPopover-test.js @@ -73,24 +73,14 @@ describe('components/AnnotationPopover', () => { expect(wrapper.find('.ba-inline-popover').length).toEqual(1); }); - test('should correctly render a BRUI Overlay if not on mobile', () => { - const wrapper = render({ - canAnnotate: true, - comments, - isMobile: false - }); - expect(wrapper).toMatchSnapshot(); - expect(wrapper.find('Overlay').prop('shouldDefaultFocus')).toBeTruthy(); - }); - - test('should correctly render a div without a Focus Trap if on mobile', () => { + test('should correctly render a div without a Focus Trap', () => { const wrapper = render({ canAnnotate: true, comments, isMobile: true }); expect(wrapper).toMatchSnapshot(); - expect(wrapper.find('Overlay').prop('shouldDefaultFocus')).toBeFalsy(); + expect(wrapper.find('Overlay').exists()).toBeFalsy(); }); test('should correctly render a popover with comments and reply textarea', () => { diff --git a/src/components/AnnotationPopover/__tests__/Overlay-test.js b/src/components/AnnotationPopover/__tests__/Overlay-test.js deleted file mode 100644 index aa2cb5076..000000000 --- a/src/components/AnnotationPopover/__tests__/Overlay-test.js +++ /dev/null @@ -1,18 +0,0 @@ -import * as React from 'react'; -import { shallow } from 'enzyme'; - -import Overlay from '../Overlay'; - -describe('components/Overlay', () => { - test('should correctly render a Focus Trap if not on mobile', () => { - const wrapper = shallow().dive(); - expect(wrapper).toMatchSnapshot(); - expect(wrapper.find('FocusTrap').length).toEqual(1); - }); - - test('should correctly render a div without a Focus Trap if on mobile', () => { - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); - expect(wrapper.find('FocusTrap').length).toEqual(0); - }); -}); diff --git a/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap b/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap index b58a8ef0e..ee816424b 100644 --- a/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap +++ b/src/components/AnnotationPopover/__tests__/__snapshots__/AnnotationPopover-test.js.snap @@ -1,111 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`components/AnnotationPopover should correctly render a BRUI Overlay if not on mobile 1`] = ` - -
- - - - - -
- -`; - -exports[`components/AnnotationPopover should correctly render a div without a Focus Trap if on mobile 1`] = ` +exports[`components/AnnotationPopover should correctly render a div without a Focus Trap 1`] = `
- - +
`; @@ -245,8 +140,8 @@ exports[`components/AnnotationPopover should correctly render a pending annotati - - + `; @@ -310,8 +205,8 @@ exports[`components/AnnotationPopover should correctly render a popover with com - - + `; @@ -415,8 +310,8 @@ exports[`components/AnnotationPopover should correctly render a view-only popove - - + `; @@ -474,8 +369,8 @@ exports[`components/AnnotationPopover should correctly render an annotation with - - + `; @@ -541,14 +436,14 @@ exports[`components/AnnotationPopover should render a view-only annotation with - - + `; diff --git a/src/components/AnnotationPopover/__tests__/__snapshots__/Overlay-test.js.snap b/src/components/AnnotationPopover/__tests__/__snapshots__/Overlay-test.js.snap deleted file mode 100644 index 421c7334c..000000000 --- a/src/components/AnnotationPopover/__tests__/__snapshots__/Overlay-test.js.snap +++ /dev/null @@ -1,20 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`components/Overlay should correctly render a Focus Trap if not on mobile 1`] = ` - -
- -`; - -exports[`components/Overlay should correctly render a div without a Focus Trap if on mobile 1`] = ` -
-`; diff --git a/src/doc/DocAnnotator.js b/src/doc/DocAnnotator.js index 9260a2f0c..90271a4d3 100644 --- a/src/doc/DocAnnotator.js +++ b/src/doc/DocAnnotator.js @@ -26,6 +26,7 @@ import { CLASS_ANNOTATION_POPOVER } from '../constants'; +const DOUBLE_CLICK_COUNT = 2; const SELECTION_TIMEOUT = 500; const CLASS_RANGY_HIGHLIGHT = 'rangy-highlight'; @@ -245,7 +246,7 @@ class DocAnnotator extends Annotator { /** @inheritdoc */ resetAnnotationUI(pageNum?: number) { // $FlowFixMe - document.getSelection().removeAllRanges(); + window.getSelection().removeAllRanges(); if (this.highlighter) { this.highlighter.removeAllHighlights(); } @@ -350,7 +351,6 @@ class DocAnnotator extends Annotator { document.addEventListener('selectionchange', this.onSelectionChange); } else { this.annotatedElement.addEventListener('mouseup', this.highlightMouseupHandler); - this.annotatedElement.addEventListener('dblclick', this.highlightMouseupHandler); this.annotatedElement.addEventListener('mousedown', this.highlightMousedownHandler); this.annotatedElement.addEventListener('contextmenu', this.highlightMousedownHandler); } @@ -373,7 +373,6 @@ class DocAnnotator extends Annotator { this.annotatedElement.removeEventListener('touchend', this.hideCreateDialog); this.annotatedElement.removeEventListener('click', this.clickHandler); this.annotatedElement.removeEventListener('mouseup', this.highlightMouseupHandler); - this.annotatedElement.removeEventListener('dblclick', this.highlightMouseupHandler); this.annotatedElement.removeEventListener('mousedown', this.highlightMousedownHandler); this.annotatedElement.removeEventListener('contextmenu', this.highlightMousedownHandler); } @@ -396,10 +395,10 @@ class DocAnnotator extends Annotator { /** * Handles click events when not in an annotation mode * - * @param {Event} event - Mouse event + * @param {MouseEvent} event - Mouse event * @return {void} */ - clickHandler = (event: Event) => { + clickHandler = (event: MouseEvent) => { let mouseEvent = event; // $FlowFixMe @@ -414,10 +413,10 @@ class DocAnnotator extends Annotator { // Hide the create dialog if click was not in the popover if ( - !this.isCreatingHighlight && // $FlowFixMe + this.hasMouseMoved(this.lastHighlightEvent, event) && + this.createHighlightDialog && this.createHighlightDialog.isVisible && - // $FlowFixMe !this.createHighlightDialog.isInHighlight(mouseEvent) ) { mouseEvent.stopPropagation(); @@ -464,7 +463,7 @@ class DocAnnotator extends Annotator { this.hideCreateDialog(event); // $FlowFixMe - document.getSelection().removeAllRanges(); + window.getSelection().removeAllRanges(); if (this.highlighter) { this.highlighter.removeAllHighlights(); } @@ -558,13 +557,7 @@ class DocAnnotator extends Annotator { this.selectionEndTimeout = null; } - // Bail if mid highlight and tapping on the screen - const isClickOutsideCreateDialog = this.isCreatingHighlight && util.isInDialog(event); - if (!docUtil.isValidSelection(selection) || isClickOutsideCreateDialog) { - this.lastHighlightEvent = null; - this.resetHighlightSelection(event); - return; - } + this.resetHighlightOnOutsideClick(event); // Do nothing if in a text area or mobile dialog or mobile create dialog is already open const pointController = this.modeControllers[TYPES.point]; @@ -602,7 +595,6 @@ class DocAnnotator extends Annotator { } let mouseEvent = event; - // $FlowFixMe if (this.hasTouch && event.targetTouches) { mouseEvent = event.targetTouches[0]; @@ -652,10 +644,11 @@ class DocAnnotator extends Annotator { * Mousedown handler on annotated element. Also delegates to mousedown * handler for each thread. * - * @param {Event} event DOM event + * @param {MouseEvent} event DOM event * @return {void} */ - highlightMousedownHandler = (event: Event) => { + highlightMousedownHandler = (event: MouseEvent) => { + const prevMouseDownEvent = this.mouseDownEvent; this.mouseDownEvent = event; // $FlowFixMe @@ -669,7 +662,9 @@ class DocAnnotator extends Annotator { } this.isCreatingHighlight = true; - this.resetHighlightSelection(this.mouseDownEvent); + if (this.hasMouseMoved(prevMouseDownEvent, event)) { + this.resetHighlightSelection(this.mouseEvent); + } if (this.plainHighlightEnabled) { this.modeControllers[TYPES.highlight].destroyPendingThreads(); @@ -700,17 +695,29 @@ class DocAnnotator extends Annotator { return isPending || this.createHighlightDialog.isVisible; } + /** + * Determines whether the mouse position has changed between the two provided mouse events + * @param {MouseEvent} prevEvent - Previous mouse event + * @param {MouseEvent} event - Current mouse event + * @return {boolean} Whether or not mouse has moved + */ + hasMouseMoved(prevEvent: ?MouseEvent, event: ?MouseEvent) { + if (!prevEvent || !event) { + return false; + } + + return prevEvent.clientX !== event.clientX || prevEvent.clientY !== event.clientY; + } + /** * Mouseup handler. Switches between creating a highlight and delegating * to highlight click handlers depending on whether mouse moved since * mousedown. * - * @param {Event} event DOM event + * @param {MouseEvent} event DOM event * @return {void} */ - highlightMouseupHandler = (event: Event) => { - this.isCreatingHighlight = false; - + highlightMouseupHandler = (event: MouseEvent) => { if (util.isInAnnotationOrMarker(event, this.container)) { return; } @@ -720,25 +727,17 @@ class DocAnnotator extends Annotator { } let mouseUpEvent = event; - // $FlowFixMe if (this.hasTouch && event.targetTouches) { mouseUpEvent = event.targetTouches[0]; } - const { clientX, clientY } = this.mouseDownEvent; - const hasMouseMoved = - // $FlowFixMe - (clientX && clientX !== mouseUpEvent.clientX) || (clientY && clientY !== mouseUpEvent.clientY); - // Creating highlights is disabled on mobile for now since the // event we would listen to, selectionchange, fires continuously and // is unreliable. If the mouse moved or we double clicked text, // we trigger the create handler instead of the click handler - if ((this.createHighlightDialog && hasMouseMoved) || event.type === 'dblclick') { + if (this.createHighlightDialog && this.hasMouseMoved(this.mouseDownEvent, mouseUpEvent)) { this.highlightCreateHandler(event); - } else { - this.resetHighlightSelection(event); } }; @@ -771,20 +770,42 @@ class DocAnnotator extends Annotator { this.lastHighlightEvent = event; }; + /** + * Bail if mid highlight and click is outside highlight/selection + * + * @param {Event} event - Mouse event + * @return {void} + */ + resetHighlightOnOutsideClick(event: Event) { + const selection = window.getSelection(); + const isClickOutsideCreateDialog = this.isCreatingHighlight && !util.isInDialog(event); + if (!docUtil.isValidSelection(selection) && isClickOutsideCreateDialog) { + this.lastHighlightEvent = null; + this.resetHighlightSelection(event); + } + } + /** * Highlight click handler. Delegates click event to click handlers for * threads on the page. * - * @param {Event} event DOM event - * @return {void} + * @param {MouseEvent} event DOM event + * @return {boolean} Whether or not mouse event was consumed */ - highlightClickHandler(event: Event) { + highlightClickHandler(event: MouseEvent) { if (!this.plainHighlightEnabled && !this.commentHighlightEnabled) { return false; } - // $FlowFixMe - if (this.createHighlightDialog.isVisible) { + // Does nothing if the user is creating a highlight + if (this.createHighlightDialog && this.createHighlightDialog.isVisible) { + this.resetHighlightOnOutsideClick(event); + return true; + } + + // Create a highlight if the user has double clicked + if (event.detail === DOUBLE_CLICK_COUNT) { + this.highlightCreateHandler(event); return true; } @@ -803,8 +824,6 @@ class DocAnnotator extends Annotator { commentThreads = this.modeControllers[TYPES.highlight_comment].getIntersectingThreads(event, location); } - this.hideAnnotations(event); - const intersectingThreads = [].concat(plainThreads, commentThreads); intersectingThreads.forEach((thread) => this.clickThread(event, thread)); @@ -814,18 +833,13 @@ class DocAnnotator extends Annotator { return true; } - this.resetHighlightSelection(event); return false; } /** @inheritdoc */ - hideAnnotations(event: ?Event) { - if (event && util.isInDialog(event, this.container)) { - return; - } - - this.resetHighlightSelection(event); - super.hideAnnotations(); + toggleAnnotationMode(mode: AnnotationType) { + this.resetAnnotationUI(); + super.toggleAnnotationMode(mode); } /** diff --git a/src/doc/DocDrawingThread.js b/src/doc/DocDrawingThread.js index 7badbe272..8ce8ec125 100644 --- a/src/doc/DocDrawingThread.js +++ b/src/doc/DocDrawingThread.js @@ -181,6 +181,13 @@ class DocDrawingThread extends DrawingThread { this.draw(context, false); } + /** + * Do nothing for drawing annotations + * + * @return {void} + */ + scrollIntoView() {} + /** @inheritdoc */ hide() { this.clearBoundary(); diff --git a/src/doc/DocHighlightThread.js b/src/doc/DocHighlightThread.js index bc587b535..c01d56b35 100644 --- a/src/doc/DocHighlightThread.js +++ b/src/doc/DocHighlightThread.js @@ -275,7 +275,7 @@ class DocHighlightThread extends AnnotationThread { const [yPos] = docUtil.getLowerRightCornerOfLastQuadPoint(this.location.quadPoints); // Adjust scroll to highlight position - this.adjustScroll(this.annotatedElement.scrollTop + yPos); + this.centerAnnotation(this.annotatedElement.scrollTop + yPos); } /** diff --git a/src/doc/__tests__/DocAnnotator-test.js b/src/doc/__tests__/DocAnnotator-test.js index ecdefadb5..2170fb8ca 100644 --- a/src/doc/__tests__/DocAnnotator-test.js +++ b/src/doc/__tests__/DocAnnotator-test.js @@ -411,7 +411,7 @@ describe('doc/DocAnnotator', () => { describe('resetAnnotationUI()', () => { beforeEach(() => { - document.getSelection = jest.fn().mockReturnValue({ + window.getSelection = jest.fn().mockReturnValue({ removeAllRanges: jest.fn() }); annotator.highlighter = { @@ -531,10 +531,6 @@ describe('doc/DocAnnotator', () => { annotator.highlightMouseupHandler ); expect(annotator.annotatedElement.addEventListener).toBeCalledWith('wheel', annotator.hideCreateDialog); - expect(annotator.annotatedElement.addEventListener).toBeCalledWith( - 'dblclick', - annotator.highlightMouseupHandler - ); expect(annotator.annotatedElement.addEventListener).toBeCalledWith( 'mousedown', annotator.highlightMousedownHandler @@ -601,10 +597,6 @@ describe('doc/DocAnnotator', () => { 'mouseup', annotator.highlightMouseupHandler ); - expect(annotator.annotatedElement.addEventListener).not.toBeCalledWith( - 'dblclick', - annotator.highlightMouseupHandler - ); expect(annotator.annotatedElement.addEventListener).not.toBeCalledWith( 'mousedown', annotator.highlightMousedownHandler @@ -648,10 +640,6 @@ describe('doc/DocAnnotator', () => { 'touchend', annotator.hideCreateDialog ); - expect(annotator.annotatedElement.removeEventListener).toBeCalledWith( - 'dblclick', - annotator.highlightMouseupHandler - ); expect(annotator.annotatedElement.removeEventListener).toBeCalledWith( 'mousedown', annotator.highlightMousedownHandler @@ -697,7 +685,7 @@ describe('doc/DocAnnotator', () => { const selection = { removeAllRanges: jest.fn() }; - document.getSelection = jest.fn().mockReturnValue(selection); + window.getSelection = jest.fn().mockReturnValue(selection); annotator.hideCreateDialog = jest.fn(); annotator.resetHighlightSelection({}); @@ -735,6 +723,7 @@ describe('doc/DocAnnotator', () => { }; annotator.resetHighlightSelection = jest.fn(); util.isInAnnotationOrMarker = jest.fn().mockReturnValue(false); + annotator.hasMouseMoved = jest.fn().mockReturnValue(true); }); it('should do nothing if event occured inside an annotation or marker', () => { @@ -750,6 +739,12 @@ describe('doc/DocAnnotator', () => { expect(controller.destroyPendingThreads).not.toBeCalled(); }); + it('should not reset highlights if the user is double clicking', () => { + annotator.hasMouseMoved = jest.fn().mockReturnValue(false); + annotator.highlightMousedownHandler({ clientX: 1, clientY: 1 }); + expect(annotator.resetHighlightSelection).not.toBeCalled(); + }); + it('should get highlights on page and call their onMouse down method', () => { annotator.plainHighlightEnabled = true; annotator.highlightMousedownHandler({ clientX: 1, clientY: 1 }); @@ -758,29 +753,42 @@ describe('doc/DocAnnotator', () => { }); }); + describe('hasMouseMoved()', () => { + let prevEvent; + let currEvent; + + beforeEach(() => { + prevEvent = { clientX: 1, clientY: 1 }; + currEvent = { clientX: 1, clientY: 1 }; + }); + + it('should return false if either a previous OR current event is non-existent', () => { + expect(annotator.hasMouseMoved(prevEvent, null)).toBeFalsy(); + expect(annotator.hasMouseMoved(null, currEvent)).toBeFalsy(); + }); + + it('should return true if the mouse has moved', () => { + expect(annotator.hasMouseMoved(prevEvent, currEvent)).toBeFalsy(); + + currEvent.clientY = 2; + expect(annotator.hasMouseMoved(prevEvent, currEvent)).toBeTruthy(); + }); + }); + describe('highlightMouseupHandler()', () => { beforeEach(() => { annotator.highlightCreateHandler = jest.fn(); - annotator.resetHighlightSelection = jest.fn(); annotator.mouseDownEvent = { clientX: 100, clientY: 100 }; }); it('should call highlightCreateHandler if on desktop and the mouse moved', () => { annotator.highlightMouseupHandler({ x: 0, y: 0 }); expect(annotator.highlightCreateHandler).toBeCalled(); - expect(annotator.resetHighlightSelection).not.toBeCalled(); }); it('should call highlightCreateHandler if on desktop and the user double clicked', () => { annotator.highlightMouseupHandler({ type: 'dblclick' }); expect(annotator.highlightCreateHandler).toBeCalled(); - expect(annotator.resetHighlightSelection).not.toBeCalled(); - }); - - it('should reset highlight selection when mouse has not moved', () => { - annotator.highlightMouseupHandler(annotator.mouseDownEvent); - expect(annotator.highlightCreateHandler).not.toBeCalled(); - expect(annotator.resetHighlightSelection).toBeCalled(); }); it('should call highlighter.removeAllHighlghts', () => { @@ -807,15 +815,17 @@ describe('doc/DocAnnotator', () => { }; docUtil.isValidSelection = jest.fn().mockReturnValue(true); + docUtil.hasSelectionChanged = jest.fn().mockReturnValue(true); annotator.lastSelection = {}; annotator.highlighter = { removeAllHighlights: jest.fn() }; - annotator.resetHighlightSelection = jest.fn(); + annotator.resetHighlightOnOutsideClick = jest.fn(); window.getSelection = jest.fn().mockReturnValue({ anchorNode: { parentNode: 'Node' - } + }, + getRangeAt: jest.fn() }); util.getPageInfo = jest.fn().mockReturnValue({ page: 1 }); util.findClosestElWithClass = jest.fn().mockReturnValue(null); @@ -831,26 +841,22 @@ describe('doc/DocAnnotator', () => { }); it('should reset the selectionEndTimeout', () => { - annotator.selectionEndTimeout = 1; - docUtil.isValidSelection = jest.fn().mockReturnValue(false); + annotator.selectionEndTimeout = 123; annotator.onSelectionChange(event); - expect(annotator.selectionEndTimeout).toBeNull(); + expect(annotator.selectionEndTimeout).not.toEqual(123); }); it('should clear selection if the user is currently creating a highlight annotation', () => { annotator.isCreatingHighlight = true; util.isInDialog = jest.fn().mockReturnValue(true); annotator.onSelectionChange(event); - expect(annotator.resetHighlightSelection).toBeCalled(); + expect(annotator.resetHighlightOnOutsideClick).toBeCalled(); }); it('should clear out highlights and exit "annotation creation" mode if an invalid selection', () => { - annotator.lastHighlightEvent = event; docUtil.isValidSelection = jest.fn().mockReturnValue(false); - annotator.onSelectionChange(event); - expect(annotator.lastHighlightEvent).toBeNull(); - expect(annotator.resetHighlightSelection).toBeCalled(); + expect(annotator.resetHighlightOnOutsideClick).toBeCalled(); }); it('should do nothing the the user is currently creating a point annotation', () => { @@ -986,6 +992,8 @@ describe('doc/DocAnnotator', () => { annotator.clickThread = jest.fn(); annotator.hideAnnotations = jest.fn(); annotator.resetHighlightSelection = jest.fn(); + annotator.resetHighlightOnOutsideClick = jest.fn(); + annotator.highlightCreateHandler = jest.fn(); annotator.modeControllers = { highlight: controller }; @@ -994,26 +1002,33 @@ describe('doc/DocAnnotator', () => { }; annotator.activeThread = undefined; annotator.plainHighlightEnabled = false; - annotator.commentHighlightEnabled = false; + annotator.createHighlightDialog.isVisible = false; + annotator.createHighlightDialog = { isVisible: false }; }); it('should do nothing and return false if no highlight types are enabled', () => { expect(annotator.highlightClickHandler(event)).toBeFalsy(); - expect(annotator.hideAnnotations).not.toBeCalled(); }); - it('should do nothing and return true if a createHighlightDialog is visible', () => { + it('should do nothing if the user is creating a highlight and resets on outside click when necessary', () => { annotator.plainHighlightEnabled = true; annotator.createHighlightDialog.isVisible = true; expect(annotator.highlightClickHandler(event)).toBeTruthy(); - expect(annotator.hideAnnotations).not.toBeCalled(); + expect(annotator.resetHighlightOnOutsideClick).toBeCalled(); + }); + + it('should create a highlight if the user has double clicked', () => { + annotator.plainHighlightEnabled = true; + expect(annotator.highlightClickHandler(event)).toBeFalsy(); + event.detail = 2; + expect(annotator.highlightClickHandler(event)).toBeTruthy(); + expect(annotator.highlightCreateHandler).toBeCalledTimes(1); }); it('should find the active plain highlight', () => { annotator.plainHighlightEnabled = true; - annotator.highlightClickHandler(event); + expect(annotator.highlightClickHandler(event)).toBeFalsy(); expect(controller.getIntersectingThreads).toBeCalled(); - expect(annotator.hideAnnotations).toBeCalled(); expect(thread.show).not.toBeCalled(); }); @@ -1022,9 +1037,9 @@ describe('doc/DocAnnotator', () => { 'highlight-comment': controller }; annotator.commentHighlightEnabled = true; - annotator.highlightClickHandler(event); + annotator.activeThread = thread; + expect(annotator.highlightClickHandler(event)).toBeFalsy(); expect(controller.getIntersectingThreads).toBeCalled(); - expect(annotator.hideAnnotations).toBeCalled(); expect(thread.show).not.toBeCalled(); }); @@ -1033,8 +1048,7 @@ describe('doc/DocAnnotator', () => { controller.getIntersectingThreads = jest.fn(() => { annotator.activeThread = thread; }); - annotator.highlightClickHandler(event); - expect(annotator.hideAnnotations).toBeCalled(); + expect(annotator.highlightClickHandler(event)).toBeTruthy(); expect(thread.show).toBeCalled(); }); @@ -1043,36 +1057,19 @@ describe('doc/DocAnnotator', () => { util.shouldDisplayMobileUI = jest.fn().mockReturnValue(true); annotator.getLocationFromEvent = jest.fn().mockReturnValue({}); - annotator.highlightClickHandler(event); - expect(annotator.hideAnnotations).toBeCalled(); + expect(annotator.highlightClickHandler(event)).toBeFalsy(); expect(thread.show).not.toBeCalled(); }); - - it('should reset highlight selection if not on mobile and no active threads exist', () => { - annotator.plainHighlightEnabled = true; - document.getSelection = jest.fn().mockReturnValue({ - removeAllRanges: jest.fn() - }); - - annotator.highlightClickHandler(event); - expect(annotator.resetHighlightSelection).toBeCalled(); - }); }); - describe('hideAnnotations()', () => { - beforeEach(() => { - annotator.resetHighlightSelection = jest.fn(); - util.isInDialog = jest.fn().mockReturnValue(false); - }); + describe('toggleAnnotationMode()', () => { + it('should reset the annotation UI and toggle the annotation mode', () => { + annotator.modeControllers.something = controller; + annotator.hideAnnotations = jest.fn(); + annotator.resetAnnotationUI = jest.fn(); - it('should reset the highlight selection', () => { - util.isInDialog = jest.fn().mockReturnValue(true); - annotator.hideAnnotations({}); - expect(annotator.resetHighlightSelection).not.toBeCalled(); - }); - it('should reset the highlight selection', () => { - annotator.hideAnnotations({}); - expect(annotator.resetHighlightSelection).toBeCalled(); + annotator.toggleAnnotationMode('something'); + expect(annotator.resetAnnotationUI).toBeCalled(); }); });