Skip to content

Commit

Permalink
Fix: Don't trigger highlight dialogs while mouse is over another dial…
Browse files Browse the repository at this point in the history 
…og (#242)
  • Loading branch information
@pramodsum
pramodsum authored Jul 27, 2017
1 parent 6080055 commit af861cf
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 34 deletions.
41 changes: 16 additions & 25 deletions src/lib/annotations/AnnotationDialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
Expand Down Expand Up @@ -500,18 +491,18 @@ class AnnotationDialog extends EventEmitter {
<div class="comment-text">${text}</div>
<button class="bp-btn-plain ${CLASS_BUTTON_DELETE_COMMENT} ${annotation.permissions.can_delete
? ''
: CLASS_HIDDEN}" data-type="${DATA_TYPE_DELETE}" title="${__('annotation_delete')}">
: CLASS_HIDDEN}" data-type="${constants.DATA_TYPE_DELETE}" title="${__('annotation_delete')}">
${ICON_DELETE}
</button>
<div class="${CLASS_DELETE_CONFIRMATION} ${CLASS_HIDDEN}">
<div class="delete-confirmation-message">
${__('annotation_delete_confirmation_message')}
</div>
<div class="${constants.CLASS_BUTTON_CONTAINER}">
<button class="bp-btn ${CLASS_CANCEL_DELETE}" data-type="${DATA_TYPE_CANCEL_DELETE}">
<button class="bp-btn ${CLASS_CANCEL_DELETE}" data-type="${constants.DATA_TYPE_CANCEL_DELETE}">
${__('annotation_cancel')}
</button>
<button class="bp-btn bp-btn-primary ${CLASS_BUTTON_DELETE_CONFIRM}" data-type="${DATA_TYPE_CONFIRM_DELETE}">
<button class="bp-btn bp-btn-primary ${CLASS_BUTTON_DELETE_CONFIRM}" data-type="${constants.DATA_TYPE_CONFIRM_DELETE}">
${__('annotation_delete')}
</button>
</div>
Expand Down Expand Up @@ -666,10 +657,10 @@ class AnnotationDialog extends EventEmitter {
<textarea class="${constants.CLASS_TEXTAREA} ${constants.CLASS_ANNOTATION_TEXTAREA}"
placeholder="${__('annotation_add_comment_placeholder')}"></textarea>
<div class="${constants.CLASS_BUTTON_CONTAINER}">
<button class="bp-btn ${constants.CLASS_ANNOTATION_BUTTON_CANCEL}" data-type="${DATA_TYPE_CANCEL}">
<button class="bp-btn ${constants.CLASS_ANNOTATION_BUTTON_CANCEL}" data-type="${constants.DATA_TYPE_CANCEL}">
${__('annotation_cancel')}
</button>
<button class="bp-btn bp-btn-primary ${constants.CLASS_ANNOTATION_BUTTON_POST}" data-type="${DATA_TYPE_POST}">
<button class="bp-btn bp-btn-primary ${constants.CLASS_ANNOTATION_BUTTON_POST}" data-type="${constants.DATA_TYPE_POST}">
${__('annotation_post')}
</button>
</div>
Expand All @@ -680,12 +671,12 @@ class AnnotationDialog extends EventEmitter {
<textarea class="${constants.CLASS_TEXTAREA} ${CLASS_REPLY_TEXTAREA}"
placeholder="${__(
'annotation_reply_placeholder'
)}" data-type="${DATA_TYPE_REPLY_TEXTAREA}"></textarea>
)}" data-type="${constants.DATA_TYPE_REPLY_TEXTAREA}"></textarea>
<div class="${constants.CLASS_BUTTON_CONTAINER} ${CLASS_HIDDEN}">
<button class="bp-btn ${constants.CLASS_ANNOTATION_BUTTON_CANCEL}" data-type="${DATA_TYPE_CANCEL_REPLY}">
<button class="bp-btn ${constants.CLASS_ANNOTATION_BUTTON_CANCEL}" data-type="${constants.DATA_TYPE_CANCEL_REPLY}">
${__('annotation_cancel')}
</button>
<button class="bp-btn bp-btn-primary ${constants.CLASS_ANNOTATION_BUTTON_POST}" data-type="${DATA_TYPE_POST_REPLY}">
<button class="bp-btn bp-btn-primary ${constants.CLASS_ANNOTATION_BUTTON_POST}" data-type="${constants.DATA_TYPE_POST_REPLY}">
${__('annotation_post')}
</button>
</div>
Expand Down
10 changes: 10 additions & 0 deletions src/lib/annotations/annotationConstants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"]';
Expand Down
6 changes: 6 additions & 0 deletions src/lib/annotations/doc/DocAnnotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions src/lib/annotations/doc/DocHighlightDialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -481,12 +479,12 @@ class DocHighlightDialog extends AnnotationDialog {
<span class="${CLASS_HIGHLIGHT_LABEL} ${CLASS_HIDDEN}"></span>
<span class="${constants.CLASS_HIGHLIGHT_BTNS} ${this.isMobile ? CLASS_HIDDEN : ''}">
<button class="bp-btn-plain ${constants.CLASS_ADD_HIGHLIGHT_BTN}"
data-type="${DATA_TYPE_HIGHLIGHT}"
data-type="${constants.DATA_TYPE_HIGHLIGHT}"
title="${__('annotation_highlight_toggle')}">
${ICON_HIGHLIGHT}
</button>
<button class="bp-btn-plain ${constants.CLASS_ADD_HIGHLIGHT_COMMENT_BTN}"
data-type="${DATA_TYPE_ADD_HIGHLIGHT_COMMENT}"
data-type="${constants.DATA_TYPE_ADD_HIGHLIGHT_COMMENT}"
title="${__('annotation_highlight_comment')}">
${ICON_HIGHLIGHT_COMMENT}
</button>
Expand Down
19 changes: 17 additions & 2 deletions src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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;
});
});
});
41 changes: 40 additions & 1 deletion src/lib/annotations/doc/docAnnotatorUtil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
*
Expand Down Expand Up @@ -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;
}

0 comments on commit af861cf

Please sign in to comment.