Skip to content

Commit

Permalink
Chore: Don't hide dialog on mouseout if mouse is in annotations dialog (
Browse files Browse the repository at this point in the history 
#352)
  • Loading branch information
@pramodsum
pramodsum authored Aug 31, 2017
1 parent eab7da5 commit e01324b
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 48 deletions.
4 changes: 3 additions & 1 deletion src/lib/annotations/AnnotationThread.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/lib/annotations/annotatorUtil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -379,6 +410,7 @@ export function repositionCaret(dialogEl, dialogX, highlightDialogWidth, browser

// Reset caret to center
annotationCaretEl.style.left = '50%';

return dialogX;
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/annotations/doc/DocAnnotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/annotations/doc/DocHighlightThread.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/annotations/doc/__tests__/DocAnnotator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
18 changes: 9 additions & 9 deletions src/lib/annotations/doc/__tests__/DocHighlightThread-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({});

Expand All @@ -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({});

Expand All @@ -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;

Expand All @@ -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');

Expand All @@ -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({});
Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -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;

Expand All @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});
Expand Down
31 changes: 0 additions & 31 deletions src/lib/annotations/doc/docAnnotatorUtil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down

0 comments on commit e01324b

Please sign in to comment.