From af861cfc33c6c4648a2cd31598c22a9dcf06819e Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Thu, 27 Jul 2017 14:12:59 -0700 Subject: [PATCH] Fix: Don't trigger highlight dialogs while mouse is over another dialog (#242) --- src/lib/annotations/AnnotationDialog.js | 41 ++++++++----------- src/lib/annotations/annotationConstants.js | 10 +++++ src/lib/annotations/doc/DocAnnotator.js | 6 +++ src/lib/annotations/doc/DocHighlightDialog.js | 10 ++--- .../doc/__tests__/docAnnotatorUtil-test.js | 19 ++++++++- src/lib/annotations/doc/docAnnotatorUtil.js | 41 ++++++++++++++++++- 6 files changed, 93 insertions(+), 34 deletions(-) diff --git a/src/lib/annotations/AnnotationDialog.js b/src/lib/annotations/AnnotationDialog.js index e4b689325..8e1adf5b7 100644 --- a/src/lib/annotations/AnnotationDialog.js +++ b/src/lib/annotations/AnnotationDialog.js @@ -17,15 +17,6 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog'; const CLASS_DELETE_CONFIRMATION = 'delete-confirmation'; const CLASS_BUTTON_DELETE_CONFIRM = 'confirm-delete-btn'; -const DATA_TYPE_POST = 'post-annotation-btn'; -const DATA_TYPE_CANCEL = 'cancel-annotation-btn'; -const DATA_TYPE_REPLY_TEXTAREA = 'reply-textarea'; -const DATA_TYPE_CANCEL_REPLY = 'cancel-reply-btn'; -const DATA_TYPE_POST_REPLY = 'post-reply-btn'; -const DATA_TYPE_DELETE = 'delete-btn'; -const DATA_TYPE_CANCEL_DELETE = 'cancel-delete-btn'; -const DATA_TYPE_CONFIRM_DELETE = 'confirm-delete-btn'; - @autobind class AnnotationDialog extends EventEmitter { //-------------------------------------------------------------------------- @@ -409,11 +400,11 @@ class AnnotationDialog extends EventEmitter { switch (dataType) { // Clicking 'Post' button to create an annotation - case DATA_TYPE_POST: + case constants.DATA_TYPE_POST: this.postAnnotation(); break; // Clicking 'Cancel' button to cancel the annotation - case DATA_TYPE_CANCEL: + case constants.DATA_TYPE_CANCEL: if (this.isMobile) { // Hide mobile dialog without destroying the thread this.hideMobileDialog(); @@ -425,27 +416,27 @@ class AnnotationDialog extends EventEmitter { this.deactivateReply(true); break; // Clicking inside reply text area - case DATA_TYPE_REPLY_TEXTAREA: + case constants.DATA_TYPE_REPLY_TEXTAREA: this.activateReply(); break; // Canceling a reply - case DATA_TYPE_CANCEL_REPLY: + case constants.DATA_TYPE_CANCEL_REPLY: this.deactivateReply(true); break; // Clicking 'Post' button to create a reply annotation - case DATA_TYPE_POST_REPLY: + case constants.DATA_TYPE_POST_REPLY: this.postReply(); break; // Clicking trash icon to initiate deletion - case DATA_TYPE_DELETE: + case constants.DATA_TYPE_DELETE: this.showDeleteConfirmation(annotationID); break; // Clicking 'Cancel' button to cancel deletion - case DATA_TYPE_CANCEL_DELETE: + case constants.DATA_TYPE_CANCEL_DELETE: this.hideDeleteConfirmation(annotationID); break; // Clicking 'Delete' button to confirm deletion - case DATA_TYPE_CONFIRM_DELETE: { + case constants.DATA_TYPE_CONFIRM_DELETE: { this.deleteAnnotation(annotationID); break; } @@ -500,7 +491,7 @@ class AnnotationDialog extends EventEmitter {
${text}
@@ -508,10 +499,10 @@ class AnnotationDialog extends EventEmitter { ${__('annotation_delete_confirmation_message')}
- -
@@ -666,10 +657,10 @@ class AnnotationDialog extends EventEmitter {
- -
@@ -680,12 +671,12 @@ class AnnotationDialog extends EventEmitter { + )}" data-type="${constants.DATA_TYPE_REPLY_TEXTAREA}">
- -
diff --git a/src/lib/annotations/annotationConstants.js b/src/lib/annotations/annotationConstants.js index fc79ed8ef..0785b6065 100644 --- a/src/lib/annotations/annotationConstants.js +++ b/src/lib/annotations/annotationConstants.js @@ -25,6 +25,16 @@ export const CLASS_ANNOTATION_BUTTON_DRAW_ENTER = 'bp-btn-annotate-draw-enter'; export const DATA_TYPE_ANNOTATION_DIALOG = 'annotation-dialog'; export const DATA_TYPE_ANNOTATION_INDICATOR = 'annotation-indicator'; +export const DATA_TYPE_HIGHLIGHT = 'highlight-btn'; +export const DATA_TYPE_ADD_HIGHLIGHT_COMMENT = 'add-highlight-comment-btn'; +export const DATA_TYPE_POST = 'post-annotation-btn'; +export const DATA_TYPE_CANCEL = 'cancel-annotation-btn'; +export const DATA_TYPE_REPLY_TEXTAREA = 'reply-textarea'; +export const DATA_TYPE_CANCEL_REPLY = 'cancel-reply-btn'; +export const DATA_TYPE_POST_REPLY = 'post-reply-btn'; +export const DATA_TYPE_DELETE = 'delete-btn'; +export const DATA_TYPE_CANCEL_DELETE = 'cancel-delete-btn'; +export const DATA_TYPE_CONFIRM_DELETE = 'confirm-delete-btn'; export const SECTION_CREATE = '[data-section="create"]'; export const SECTION_SHOW = '[data-section="show"]'; diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 3dd11cfc4..70421b4ea 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -576,7 +576,13 @@ class DocAnnotator extends Annotator { return; } + // Determine if mouse is over any highlight dialog currently + // and ignore hover events of any highlights below const event = this.mouseMoveEvent; + if (docAnnotatorUtil.isDialogDataType(event.target)) { + return; + } + this.mouseMoveEvent = null; this.throttleTimer = performance.now(); // Only filter through highlight threads on the current page diff --git a/src/lib/annotations/doc/DocHighlightDialog.js b/src/lib/annotations/doc/DocHighlightDialog.js index 1adc60cf4..ebba86e7d 100644 --- a/src/lib/annotations/doc/DocHighlightDialog.js +++ b/src/lib/annotations/doc/DocHighlightDialog.js @@ -11,8 +11,6 @@ const CLASS_HIGHLIGHT_DIALOG = 'bp-highlight-dialog'; const CLASS_ANNOTATION_HIGHLIGHT_DIALOG = 'bp-annotation-highlight-dialog'; const CLASS_TEXT_HIGHLIGHTED = 'bp-is-text-highlighted'; const CLASS_HIGHLIGHT_LABEL = 'bp-annotation-highlight-label'; -const DATA_TYPE_HIGHLIGHT = 'highlight-btn'; -const DATA_TYPE_ADD_HIGHLIGHT_COMMENT = 'add-highlight-comment-btn'; const HIGHLIGHT_DIALOG_HEIGHT = 38; const PAGE_PADDING_BOTTOM = 15; @@ -330,11 +328,11 @@ class DocHighlightDialog extends AnnotationDialog { switch (dataType) { // Clicking 'Highlight' button to create or remove a highlight - case DATA_TYPE_HIGHLIGHT: + case constants.DATA_TYPE_HIGHLIGHT: this.drawAnnotation(); break; // Clicking 'Highlight' button to create a highlight - case DATA_TYPE_ADD_HIGHLIGHT_COMMENT: + case constants.DATA_TYPE_ADD_HIGHLIGHT_COMMENT: this.emit('annotationdraw'); this.toggleHighlightCommentsReply(false); this.toggleHighlightDialogs(); @@ -481,12 +479,12 @@ class DocHighlightDialog extends AnnotationDialog { diff --git a/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js b/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js index 1498382bd..52616342f 100644 --- a/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js +++ b/src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js @@ -11,13 +11,16 @@ import { getBrowserCoordinatesFromLocation, getLowerRightCornerOfLastQuadPoint, getContext, - getPageEl + getPageEl, + isDialogDataType } from '../docAnnotatorUtil'; import { SELECTOR_ANNOTATION_DIALOG, SELECTOR_ANNOTATION_CONTAINER, - CLASS_ANNOTATION_DIALOG + CLASS_ANNOTATION_DIALOG, + DATA_TYPE_ANNOTATION_DIALOG } from '../../annotationConstants'; +import * as annotatorUtil from '../../annotatorUtil'; const sandbox = sinon.sandbox.create(); @@ -239,4 +242,16 @@ describe('lib/annotations/doc/docAnnotatorUtil', () => { assert.equal(pageEl, truePageEl); }); }); + + describe('isDialogDataType()', () => { + it('should return true if the mouse event occured in a highlight dialog', () => { + sandbox.stub(annotatorUtil, 'findClosestDataType').returns(DATA_TYPE_ANNOTATION_DIALOG); + expect(isDialogDataType({})).to.be.true; + }); + + it('should return false if the mouse event occured outside a highlight dialog', () => { + sandbox.stub(annotatorUtil, 'findClosestDataType').returns('something'); + expect(isDialogDataType({})).to.be.false; + }); + }); }); diff --git a/src/lib/annotations/doc/docAnnotatorUtil.js b/src/lib/annotations/doc/docAnnotatorUtil.js index 6307c2a05..b0234fc6a 100644 --- a/src/lib/annotations/doc/docAnnotatorUtil.js +++ b/src/lib/annotations/doc/docAnnotatorUtil.js @@ -4,7 +4,19 @@ import { CLASS_ANNOTATION_HIGHLIGHT_DIALOG, SELECTOR_ANNOTATION_CONTAINER, PAGE_PADDING_TOP, - PAGE_PADDING_BOTTOM + PAGE_PADDING_BOTTOM, + DATA_TYPE_ANNOTATION_DIALOG, + DATA_TYPE_ANNOTATION_INDICATOR, + DATA_TYPE_HIGHLIGHT, + DATA_TYPE_ADD_HIGHLIGHT_COMMENT, + DATA_TYPE_POST, + DATA_TYPE_CANCEL, + DATA_TYPE_REPLY_TEXTAREA, + DATA_TYPE_CANCEL_REPLY, + DATA_TYPE_POST_REPLY, + DATA_TYPE_DELETE, + DATA_TYPE_CANCEL_DELETE, + DATA_TYPE_CONFIRM_DELETE } from '../annotationConstants'; const PREVIEW_PRESENTATION_CLASS = 'bp-doc-presentation'; @@ -14,6 +26,21 @@ const PDF_UNIT_TO_CSS_PIXEL = 4 / 3; const CSS_PIXEL_TO_PDF_UNIT = 3 / 4; const HIGHLIGHT_DIALOG_HEIGHT = 48; +const DIALOG_DATATYPES = [ + DATA_TYPE_ANNOTATION_DIALOG, + DATA_TYPE_ANNOTATION_INDICATOR, + DATA_TYPE_HIGHLIGHT, + DATA_TYPE_ADD_HIGHLIGHT_COMMENT, + DATA_TYPE_POST, + DATA_TYPE_CANCEL, + DATA_TYPE_REPLY_TEXTAREA, + DATA_TYPE_CANCEL_REPLY, + DATA_TYPE_POST_REPLY, + DATA_TYPE_DELETE, + DATA_TYPE_CANCEL_DELETE, + DATA_TYPE_CONFIRM_DELETE +]; + /** * Checks whether this annotator is on a presentation (PPT) or not. * @@ -359,3 +386,15 @@ export function getContext(pageEl, annotationLayerClass, paddingTop, paddingBott export function getPageEl(annotatedEl, pageNum) { return annotatedEl.querySelector(`[data-page-number="${pageNum}"]`); } + +/** + * Checks whether the mouse event occured in a highlight dialog + * + * @private + * @param {HTMLElement} eventTarget mouse event target element + * @return {boolean} Whether mouse event occured in a highlight dialog + */ +export function isDialogDataType(eventTarget) { + const dataType = annotatorUtil.findClosestDataType(eventTarget); + return DIALOG_DATATYPES.indexOf(dataType) !== -1; +}