From e01324ba76964d07401a82c2725121ca953ed713 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Thu, 31 Aug 2017 14:47:18 -0700 Subject: [PATCH] Chore: Don't hide dialog on mouseout if mouse is in annotations dialog (#352) --- src/lib/annotations/AnnotationThread.js | 4 ++- src/lib/annotations/annotatorUtil.js | 32 +++++++++++++++++++ src/lib/annotations/doc/DocAnnotator.js | 2 +- src/lib/annotations/doc/DocHighlightThread.js | 4 +-- .../doc/__tests__/DocAnnotator-test.js | 2 +- .../doc/__tests__/DocHighlightThread-test.js | 18 +++++------ .../doc/__tests__/docAnnotatorUtil-test.js | 6 ++-- src/lib/annotations/doc/docAnnotatorUtil.js | 31 ------------------ 8 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/lib/annotations/AnnotationThread.js b/src/lib/annotations/AnnotationThread.js index be6488811..104b66b86 100644 --- a/src/lib/annotations/AnnotationThread.js +++ b/src/lib/annotations/AnnotationThread.js @@ -436,7 +436,9 @@ class AnnotationThread extends EventEmitter { * @return {void} */ mouseoutHandler() { - if (this.annotations.length !== 0) { + const mouseInDialog = annotatorUtil.isInDialog(event, this.dialog.element); + + if (this.annotations.length !== 0 && !mouseInDialog) { this.hideDialog(); } } diff --git a/src/lib/annotations/annotatorUtil.js b/src/lib/annotations/annotatorUtil.js index c5a8ee7ff..d9984da68 100644 --- a/src/lib/annotations/annotatorUtil.js +++ b/src/lib/annotations/annotatorUtil.js @@ -209,6 +209,37 @@ export function resetTextarea(element, clearText) { } } +/** + * Checks whether mouse is inside the dialog represented by this thread. + * + * @private + * @param {Event} event - Mouse event + * @param {HTMLElement} dialogEl - Dialog element + * @return {boolean} Whether or not mouse is inside dialog + */ +export function isInDialog(event, dialogEl) { + if (!dialogEl) { + return false; + } + + // DOM coordinates with respect to the page + const x = event.clientX; + const y = event.clientY; + + // Get dialog dimensions + const dialogDimensions = dialogEl.getBoundingClientRect(); + + if ( + y >= dialogDimensions.top && + y <= dialogDimensions.bottom && + x >= dialogDimensions.left && + x <= dialogDimensions.right + ) { + return true; + } + return false; +} + //------------------------------------------------------------------------------ // Point Utils //------------------------------------------------------------------------------ @@ -379,6 +410,7 @@ export function repositionCaret(dialogEl, dialogX, highlightDialogWidth, browser // Reset caret to center annotationCaretEl.style.left = '50%'; + return dialogX; } diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 4ea91b1ea..1514087e6 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -545,7 +545,7 @@ class DocAnnotator extends Annotator { let mouseInDialog = false; threads.some((thread) => { - mouseInDialog = docAnnotatorUtil.isInDialog(event, thread.dialog.element); + mouseInDialog = annotatorUtil.isInDialog(event, thread.dialog.element); return mouseInDialog; }); return mouseInDialog; diff --git a/src/lib/annotations/doc/DocHighlightThread.js b/src/lib/annotations/doc/DocHighlightThread.js index c95579f32..4c7e67487 100644 --- a/src/lib/annotations/doc/DocHighlightThread.js +++ b/src/lib/annotations/doc/DocHighlightThread.js @@ -168,7 +168,7 @@ class DocHighlightThread extends AnnotationThread { * the annotations dialog */ isOnHighlight(event) { - return docAnnotatorUtil.isInDialog(event, this.dialog.element) || this.isInHighlight(event); + return annotatorUtil.isInDialog(event, this.dialog.element) || this.isInHighlight(event); } /** @@ -199,7 +199,7 @@ class DocHighlightThread extends AnnotationThread { */ onMousemove(event) { // If mouse is in dialog, change state to hover or active-hover - if (docAnnotatorUtil.isInDialog(event, this.dialog.element)) { + if (annotatorUtil.isInDialog(event, this.dialog.element)) { // Keeps dialog open if comment is pending if (this.state === STATES.pending_active) { return false; diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index 9fa8abebb..3ae483c9a 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -660,7 +660,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { beforeEach(() => { const threads = [{ dialog: { element: {} } }]; sandbox.stub(annotator, 'getThreadsOnPage').returns(threads); - stubs.inDialog = sandbox.stub(docAnnotatorUtil, 'isInDialog'); + stubs.inDialog = sandbox.stub(annotatorUtil, 'isInDialog'); }); it('should return true if mouse is hovering over an open dialog', () => { diff --git a/src/lib/annotations/doc/__tests__/DocHighlightThread-test.js b/src/lib/annotations/doc/__tests__/DocHighlightThread-test.js index 8e1b0f379..69d0722f2 100644 --- a/src/lib/annotations/doc/__tests__/DocHighlightThread-test.js +++ b/src/lib/annotations/doc/__tests__/DocHighlightThread-test.js @@ -240,7 +240,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should return true if mouse event is over highlight dialog', () => { sandbox.stub(highlightThread, 'isInHighlight').returns(false); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(true); + sandbox.stub(annotatorUtil, 'isInDialog').returns(true); const result = highlightThread.isOnHighlight({}); @@ -249,7 +249,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should return false if mouse event is neither over the highlight or the dialog', () => { sandbox.stub(highlightThread, 'isInHighlight').returns(false); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(false); + sandbox.stub(annotatorUtil, 'isInDialog').returns(false); const result = highlightThread.isOnHighlight({}); @@ -259,7 +259,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { describe('activateDialog()', () => { it('should set to hover and trigger dialog mouseenter event if thread is not in the active or active-hover state', () => { - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(true); + sandbox.stub(annotatorUtil, 'isInDialog').returns(true); sandbox.stub(highlightThread.dialog, 'mouseenterHandler'); highlightThread.state = STATES.inactive; @@ -271,7 +271,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should ensure element is set up before calling mouse events', () => { highlightThread.dialog.element = null; - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(true); + sandbox.stub(annotatorUtil, 'isInDialog').returns(true); sandbox.stub(highlightThread.dialog, 'setup'); sandbox.stub(highlightThread.dialog, 'mouseenterHandler'); @@ -283,7 +283,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { describe('onMousemove()', () => { it('should delay drawing highlight if mouse is hovering over a highlight dialog and not pending comment', () => { sandbox.stub(highlightThread, 'getPageEl').returns(highlightThread.annotatedElement); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(true); + sandbox.stub(annotatorUtil, 'isInDialog').returns(true); highlightThread.state = STATES.inactive; const result = highlightThread.onMousemove({}); @@ -294,7 +294,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should do nothing if mouse is hovering over a highlight dialog and pending comment', () => { sandbox.stub(highlightThread, 'getPageEl').returns(highlightThread.annotatedElement); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(true); + sandbox.stub(annotatorUtil, 'isInDialog').returns(true); sandbox.stub(highlightThread, 'activateDialog'); highlightThread.state = STATES.pending_active; @@ -306,7 +306,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should delay drawing highlight if mouse is hovering over a highlight', () => { sandbox.stub(highlightThread, 'getPageEl').returns(highlightThread.annotatedElement); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(false); + sandbox.stub(annotatorUtil, 'isInDialog').returns(false); sandbox.stub(highlightThread, 'isInHighlight').returns(true); sandbox.stub(highlightThread, 'activateDialog'); highlightThread.state = STATES.hover; @@ -319,7 +319,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should not delay drawing highlight if mouse is not in highlight and the state is not already inactive', () => { sandbox.stub(highlightThread, 'getPageEl').returns(highlightThread.annotatedElement); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(false); + sandbox.stub(annotatorUtil, 'isInDialog').returns(false); sandbox.stub(highlightThread, 'isInHighlight').returns(false); highlightThread.state = STATES.hover; @@ -331,7 +331,7 @@ describe('lib/annotations/doc/DocHighlightThread', () => { it('should not delay drawing highlight if the state is already inactive', () => { sandbox.stub(highlightThread, 'getPageEl').returns(highlightThread.annotatedElement); - sandbox.stub(docAnnotatorUtil, 'isInDialog').returns(false); + sandbox.stub(annotatorUtil, 'isInDialog').returns(false); sandbox.stub(highlightThread, 'isInHighlight').returns(false); highlightThread.state = STATES.inactive; diff --git a/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js b/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js index 82c309dcd..bd15953ff 100644 --- a/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js +++ b/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js @@ -43,19 +43,19 @@ describe('lib/annotations/doc/docAnnotatorUtil', () => { describe('isInDialog()', () => { it('should return false if no dialog element exists', () => { - const result = docAnnotatorUtil.isInDialog({ clientX: 8, clientY: 8 }); + const result = annotatorUtil.isInDialog({ clientX: 8, clientY: 8 }); expect(result).to.be.false; }); it('should return true if the event is in the given dialog', () => { const dialogEl = document.querySelector(SELECTOR_ANNOTATION_DIALOG); - const result = docAnnotatorUtil.isInDialog({ clientX: 8, clientY: 8 }, dialogEl); + const result = annotatorUtil.isInDialog({ clientX: 8, clientY: 8 }, dialogEl); expect(result).to.be.true; }); it('should return false if the event is in the given dialog', () => { const dialogEl = document.querySelector(SELECTOR_ANNOTATION_DIALOG); - const result = docAnnotatorUtil.isInDialog({ clientX: 100, clientY: 100 }, dialogEl); + const result = annotatorUtil.isInDialog({ clientX: 100, clientY: 100 }, dialogEl); expect(result).to.be.false; }); }); diff --git a/src/lib/annotations/doc/docAnnotatorUtil.js b/src/lib/annotations/doc/docAnnotatorUtil.js index 46b9be6ce..0d5c3e8d9 100644 --- a/src/lib/annotations/doc/docAnnotatorUtil.js +++ b/src/lib/annotations/doc/docAnnotatorUtil.js @@ -55,37 +55,6 @@ export function isPresentation(annotatedElement) { // DOM Utils //------------------------------------------------------------------------------ -/** - * Checks whether mouse is inside the dialog represented by this thread. - * - * @private - * @param {Event} event - Mouse event - * @param {HTMLElement} dialogEl - Dialog element - * @return {boolean} Whether or not mouse is inside dialog - */ -export function isInDialog(event, dialogEl) { - if (!dialogEl) { - return false; - } - - // DOM coordinates with respect to the page - const x = event.clientX; - const y = event.clientY; - - // Get dialog dimensions - const dialogDimensions = dialogEl.getBoundingClientRect(); - - if ( - y >= dialogDimensions.top && - y <= dialogDimensions.bottom && - x >= dialogDimensions.left && - x <= dialogDimensions.right - ) { - return true; - } - return false; -} - /** * Checks if there is an active annotation in the annotated document *