From 5b66c82322f51aa95fdbb90edbc074a4bd83407f Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Mon, 14 Aug 2017 16:13:34 -0700 Subject: [PATCH] New: Plain and Comment highlight annotations on mobile (#276) --- src/lib/annotations/AnnotationDialog.js | 32 +-- src/lib/annotations/AnnotationThread.js | 2 +- src/lib/annotations/Annotator.js | 3 + src/lib/annotations/CommentBox.js | 61 ++++-- src/lib/annotations/MobileAnnotator.scss | 63 +++++- .../__tests__/AnnotationDialog-test.js | 11 +- .../__tests__/AnnotationThread-test.js | 2 +- .../annotations/__tests__/CommentBox-test.js | 36 ++-- src/lib/annotations/annotationConstants.js | 4 + src/lib/annotations/annotatorUtil.js | 16 +- .../annotations/doc/CreateHighlightDialog.js | 111 +++++++---- src/lib/annotations/doc/DocAnnotator.js | 184 ++++++++++++------ src/lib/annotations/doc/DocHighlightDialog.js | 66 ++++++- src/lib/annotations/doc/DocHighlightThread.js | 19 +- .../__tests__/CreateHighlightDialog-test.js | 65 ++++++- .../doc/__tests__/DocAnnotator-test.js | 136 +++++++++++++ .../doc/__tests__/DocHighlightDialog-test.js | 116 +++++++++++ .../doc/__tests__/DocHighlightThread-test.js | 2 +- .../doc/__tests__/docAnnotatorUtil-test.js | 38 ++++ src/lib/annotations/doc/docAnnotatorUtil.js | 19 +- src/lib/viewers/BaseViewer.js | 1 + 21 files changed, 783 insertions(+), 204 deletions(-) diff --git a/src/lib/annotations/AnnotationDialog.js b/src/lib/annotations/AnnotationDialog.js index af0ae863c..8fd463e8b 100644 --- a/src/lib/annotations/AnnotationDialog.js +++ b/src/lib/annotations/AnnotationDialog.js @@ -4,14 +4,13 @@ import * as annotatorUtil from './annotatorUtil'; import * as constants from './annotationConstants'; import { ICON_CLOSE, ICON_DELETE } from '../icons/icons'; -const CLASS_ANNOTATION_PLAIN_HIGHLIGHT = 'bp-plain-highlight'; const CLASS_BUTTON_DELETE_COMMENT = 'delete-comment-btn'; const CLASS_CANCEL_DELETE = 'cancel-delete-btn'; const CLASS_CANNOT_ANNOTATE = 'cannot-annotate'; +const CLASS_COMMENT = 'annotation-comment'; const CLASS_COMMENTS_CONTAINER = 'annotation-comments'; const CLASS_REPLY_CONTAINER = 'reply-container'; const CLASS_REPLY_TEXTAREA = 'reply-textarea'; -const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog'; const CLASS_DELETE_CONFIRMATION = 'delete-confirmation'; const CLASS_BUTTON_DELETE_CONFIRM = 'confirm-delete-btn'; @@ -23,6 +22,7 @@ class AnnotationDialog extends EventEmitter { /** * The data object for constructing a dialog. + * * @typedef {Object} AnnotationDialogData * @property {HTMLElement} annotatedElement HTML element being annotated on * @property {Annotation[]} annotations Annotations in dialog, can be an @@ -81,8 +81,9 @@ class AnnotationDialog extends EventEmitter { annotatorUtil.showElement(this.element); this.element.appendChild(this.dialogEl); - if (this.highlightDialogEl && !this.hasComments) { - this.element.classList.add(CLASS_ANNOTATION_PLAIN_HIGHLIGHT); + const commentEls = this.element.querySelectorAll(`.${CLASS_COMMENT}`); + if (this.highlightDialogEl && !commentEls.length) { + this.element.classList.add(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT); const headerEl = this.element.querySelector(constants.SELECTOR_MOBILE_DIALOG_HEADER); headerEl.classList.add(constants.CLASS_HIDDEN); @@ -91,7 +92,7 @@ class AnnotationDialog extends EventEmitter { const dialogCloseButtonEl = this.element.querySelector(constants.SELECTOR_DIALOG_CLOSE); dialogCloseButtonEl.addEventListener('click', this.hideMobileDialog); - this.element.classList.add(CLASS_ANIMATE_DIALOG); + this.element.classList.add(constants.CLASS_ANIMATE_DIALOG); this.bindDOMListeners(); } @@ -146,14 +147,18 @@ class AnnotationDialog extends EventEmitter { return; } - this.element.classList.remove(CLASS_ANIMATE_DIALOG); + if (this.dialogEl && this.dialogEl.parentNode) { + this.dialogEl.parentNode.removeChild(this.dialogEl); + } + + this.element.classList.remove(constants.CLASS_ANIMATE_DIALOG); // Clear annotations from dialog this.element.innerHTML = `
`.trim(); - this.element.classList.remove(CLASS_ANNOTATION_PLAIN_HIGHLIGHT); + this.element.classList.remove(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT); const dialogCloseButtonEl = this.element.querySelector(constants.SELECTOR_DIALOG_CLOSE); dialogCloseButtonEl.removeEventListener('click', this.hideMobileDialog); @@ -162,9 +167,7 @@ class AnnotationDialog extends EventEmitter { this.unbindDOMListeners(); // Cancel any unsaved annotations - if (!this.hasAnnotations) { - this.cancelAnnotation(); - } + this.cancelAnnotation(); } /** @@ -173,6 +176,10 @@ class AnnotationDialog extends EventEmitter { * @return {void} */ hide() { + if (this.element && this.element.classList.contains(constants.CLASS_HIDDEN)) { + return; + } + if (this.isMobile) { this.hideMobileDialog(); } @@ -404,8 +411,7 @@ class AnnotationDialog extends EventEmitter { // Clicking 'Cancel' button to cancel the annotation case constants.DATA_TYPE_CANCEL: if (this.isMobile) { - // Hide mobile dialog without destroying the thread - this.hideMobileDialog(); + this.hide(); } else { // Cancels + destroys the annotation thread this.cancelAnnotation(); @@ -478,7 +484,7 @@ class AnnotationDialog extends EventEmitter { const text = annotatorUtil.htmlEscape(annotation.text); const annotationEl = document.createElement('div'); - annotationEl.classList.add('annotation-comment'); + annotationEl.classList.add(CLASS_COMMENT); annotationEl.setAttribute('data-annotation-id', annotation.annotationID); annotationEl.innerHTML = `
${avatarHtml}
diff --git a/src/lib/annotations/AnnotationThread.js b/src/lib/annotations/AnnotationThread.js index 45c2947ea..2ca882f66 100644 --- a/src/lib/annotations/AnnotationThread.js +++ b/src/lib/annotations/AnnotationThread.js @@ -367,7 +367,7 @@ class AnnotationThread extends EventEmitter { * @return {void} */ cancelUnsavedAnnotation() { - if (!this.isMobile && !annotatorUtil.isPending(this.state)) { + if (!annotatorUtil.isPending(this.state)) { return; } this.destroy(); diff --git a/src/lib/annotations/Annotator.js b/src/lib/annotations/Annotator.js index 7a35704ce..eec154e21 100644 --- a/src/lib/annotations/Annotator.js +++ b/src/lib/annotations/Annotator.js @@ -13,6 +13,7 @@ import { CLASS_ANNOTATION_MODE, CLASS_MOBILE_DIALOG_HEADER, CLASS_DIALOG_CLOSE, + ID_MOBILE_ANNOTATION_DIALOG, SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL, SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER, SELECTOR_ANNOTATION_BUTTON_DRAW_POST, @@ -56,6 +57,7 @@ class Annotator extends EventEmitter { this.locale = data.locale; this.validationErrorEmitted = false; this.isMobile = data.isMobile; + this.hasTouch = data.hasTouch; this.previewUI = data.previewUI; this.modeButtons = data.modeButtons; this.annotationModeHandlers = []; @@ -195,6 +197,7 @@ class Annotator extends EventEmitter { mobileDialogEl.classList.add(CLASS_MOBILE_ANNOTATION_DIALOG); mobileDialogEl.classList.add(CLASS_ANNOTATION_DIALOG); mobileDialogEl.classList.add(CLASS_HIDDEN); + mobileDialogEl.id = ID_MOBILE_ANNOTATION_DIALOG; mobileDialogEl.innerHTML = `
diff --git a/src/lib/annotations/CommentBox.js b/src/lib/annotations/CommentBox.js index 5130f0230..5cb694641 100644 --- a/src/lib/annotations/CommentBox.js +++ b/src/lib/annotations/CommentBox.js @@ -64,6 +64,9 @@ class CommentBox extends EventEmitter { */ parentEl; + /** Whether or not we should use touch events */ + hasTouch; + /* Events that the comment box can emit. */ static CommentEvents = { cancel: 'comment_cancel', @@ -87,6 +90,7 @@ class CommentBox extends EventEmitter { this.cancelText = config.cancel || this.cancelText; this.postText = config.post || this.postText; this.placeholderText = config.placeholder || this.placeholderText; + this.hasTouch = config.hasTouch; // Explicit scope binding for event listeners this.onCancel = this.onCancel.bind(this); @@ -99,11 +103,20 @@ class CommentBox extends EventEmitter { * @return {void} */ focus() { - if (!this.containerEl) { - return; + if (this.textAreaEl) { + this.textAreaEl.focus(); } + } - this.textAreaEl.focus(); + /** + * Unfocus the text box. + * + * @return {void} + */ + blur() { + if (document.activeElement) { + document.activeElement.blur(); + } } /** @@ -112,11 +125,9 @@ class CommentBox extends EventEmitter { * @return {void} */ clear() { - if (!this.containerEl) { - return; + if (this.textAreaEl) { + this.textAreaEl.value = ''; } - - this.textAreaEl.value = ''; } /** @@ -125,11 +136,9 @@ class CommentBox extends EventEmitter { * @return {void} */ hide() { - if (!this.containerEl) { - return; + if (this.containerEl) { + hideElement(this.containerEl); } - - hideElement(this.containerEl); } /** @@ -161,6 +170,10 @@ class CommentBox extends EventEmitter { this.containerEl = null; this.cancelEl.removeEventListener('click', this.onCancel); this.postEl.removeEventListener('click', this.onPost); + if (this.hasTouch) { + this.cancelEl.removeEventListener('touchstart', this.onCancel); + this.postEl.removeEventListener('touchstart', this.onPost); + } } //-------------------------------------------------------------------------- @@ -191,13 +204,27 @@ class CommentBox extends EventEmitter { return containerEl; } + /** + * Stop default behaviour of an element. + * + * @param {Event} event Event created by an input event. + * @return {void} + */ + preventDefaultAndPropagation(event) { + event.preventDefault(); + event.stopPropagation(); + } + /** * Clear the current text in the textarea element and notify listeners. * * @private + * @param {Event} event Event created by input event * @return {void} */ - onCancel() { + onCancel(event) { + // stops touch propogating to a click event + this.preventDefaultAndPropagation(event); this.clear(); this.emit(CommentBox.CommentEvents.cancel); } @@ -206,9 +233,12 @@ class CommentBox extends EventEmitter { * Notify listeners of submit event and then clear textarea element. * * @private + * @param {Event} event Event created by input event * @return {void} */ - onPost() { + onPost(event) { + // stops touch propogating to a click event + this.preventDefaultAndPropagation(event); this.emit(CommentBox.CommentEvents.post, this.textAreaEl.value); this.clear(); } @@ -231,6 +261,11 @@ class CommentBox extends EventEmitter { // Add event listeners this.cancelEl.addEventListener('click', this.onCancel); this.postEl.addEventListener('click', this.onPost); + if (this.hasTouch) { + containerEl.addEventListener('touchend', this.preventDefaultAndPropagation.bind(this)); + this.cancelEl.addEventListener('touchend', this.onCancel); + this.postEl.addEventListener('touchend', this.onPost); + } return containerEl; } diff --git a/src/lib/annotations/MobileAnnotator.scss b/src/lib/annotations/MobileAnnotator.scss index f904abed8..81a3fdbb1 100644 --- a/src/lib/annotations/MobileAnnotator.scss +++ b/src/lib/annotations/MobileAnnotator.scss @@ -1,5 +1,13 @@ +@import '../_boxuiVariables'; + $tablet: "(min-width: 768px)"; +@mixin border-top-bottom { + border-color: $seesee; + border-style: solid; + border-width: 1px 0; +} + .bp-mobile-annotation-dialog { background: white; border-top: 0; @@ -18,7 +26,7 @@ $tablet: "(min-width: 768px)"; animation: show-dialog-tablet; animation-duration: .2s; animation-fill-mode: forwards; - border-left: solid 1px #ccc; + border-left: solid 1px $seesee; width: 450px; } } @@ -31,6 +39,14 @@ $tablet: "(min-width: 768px)"; } } +.bp-create-annotation-dialog.bp-mobile-annotation-dialog { + height: auto; + + .bp-annotation-highlight-dialog { + bottom: 0; + } +} + @keyframes show-dialog-small { 0% { top: 100%; @@ -57,12 +73,11 @@ $tablet: "(min-width: 768px)"; } 100% { - top: 1px; + top: 0; } } -.bp-mobile-annotation-dialog.bp-annotation-dialog, -.bp-mobile-annotation-dialog.bp-temp-annotation-dialog { +.bp-mobile-annotation-dialog.bp-annotation-dialog { .annotation-container { background-color: $white; border: 0; @@ -70,7 +85,7 @@ $tablet: "(min-width: 768px)"; height: 100%; overflow-x: hidden; overflow-y: auto; - padding: 15px 15px 60px; + padding: 15px; position: absolute; width: 100%; } @@ -78,11 +93,12 @@ $tablet: "(min-width: 768px)"; .bp-annotation-mobile-header { align-items: center; background-color: $white; - border-bottom: 1px solid #ccc; display: flex; height: 48px; justify-content: space-between; padding: 0; + + @include border-top-bottom; } .bp-annotation-dialog-close { @@ -153,9 +169,23 @@ $tablet: "(min-width: 768px)"; /* Highlight dialog */ .bp-mobile-annotation-dialog.bp-plain-highlight { - border-bottom: 1px solid #ccc; height: 47px; // includes mobile header & highlight dialog top: auto; + + @include border-top-bottom; +} + +.bp-mobile-annotation-dialog .bp-annotation-highlight-btns, +.bp-mobile-annotation-dialog .bp-create-highlight-comment, +.bp-mobile-annotation-dialog .annotation-container section[data-section="create"] { + background: white; + bottom: 0; + left: 0; + padding: 15px; + position: fixed; + width: 100%; + + @include border-top-bottom; } .bp-mobile-annotation-dialog .bp-annotation-highlight-dialog { @@ -169,8 +199,23 @@ $tablet: "(min-width: 768px)"; text-align: center; z-index: 9999; - .bp-annotations-highlight-btns button { - width: 50%; + .bp-annotation-highlight-btns { + padding: 0; + + button { + float: left; + padding: 15px 0; + width: 50%; + + &:first-child::after { + border-right: 1px solid $seesee; + content: ""; + bottom: 17px; + height: 25px; + left: 50%; + position: absolute; + } + } } &.cannot-annotate { diff --git a/src/lib/annotations/__tests__/AnnotationDialog-test.js b/src/lib/annotations/__tests__/AnnotationDialog-test.js index e5200344a..0fb47d161 100644 --- a/src/lib/annotations/__tests__/AnnotationDialog-test.js +++ b/src/lib/annotations/__tests__/AnnotationDialog-test.js @@ -4,7 +4,6 @@ import AnnotationDialog from '../AnnotationDialog'; import * as annotatorUtil from '../annotatorUtil'; import * as constants from '../annotationConstants'; -const CLASS_ANNOTATION_PLAIN_HIGHLIGHT = 'bp-plain-highlight'; const CLASS_CANCEL_DELETE = 'cancel-delete-btn'; const CLASS_CANNOT_ANNOTATE = 'cannot-annotate'; const CLASS_REPLY_TEXTAREA = 'reply-textarea'; @@ -165,15 +164,15 @@ describe('lib/annotations/AnnotationDialog', () => { expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.true; }); - it('should hide the mobile header if a plain highlight', () => { + it('should reset the annotation dialog to be a plain highlight if no comments are present', () => { dialog.isMobile = true; dialog.highlightDialogEl = {}; - dialog.hasComments = false; + sandbox.stub(dialog.element, 'querySelectorAll').withArgs('.annotation-comment').returns([]); stubs.show = sandbox.stub(annotatorUtil, 'showElement'); stubs.bind = sandbox.stub(dialog, 'bindDOMListeners'); - dialog.show(); - expect(dialog.element).to.have.class(CLASS_ANNOTATION_PLAIN_HIGHLIGHT); + + expect(dialog.element.classList.contains(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT)).to.be.true; }); }); @@ -195,7 +194,7 @@ describe('lib/annotations/AnnotationDialog', () => { dialog.hideMobileDialog(); expect(stubs.hide).to.be.called; expect(stubs.unbind).to.be.called; - expect(stubs.cancel).to.not.be.called; + expect(stubs.cancel).to.be.called; expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.false; }); diff --git a/src/lib/annotations/__tests__/AnnotationThread-test.js b/src/lib/annotations/__tests__/AnnotationThread-test.js index 49efa49d9..b7e25ad91 100644 --- a/src/lib/annotations/__tests__/AnnotationThread-test.js +++ b/src/lib/annotations/__tests__/AnnotationThread-test.js @@ -526,7 +526,7 @@ describe('lib/annotations/AnnotationThread', () => { expect(thread.destroy).to.be.called; // 'pending-active' state - thread.state = STATES.pending_ACTIVE; + thread.state = STATES.pending_active; thread.cancelUnsavedAnnotation(); expect(thread.destroy).to.be.called; }); diff --git a/src/lib/annotations/__tests__/CommentBox-test.js b/src/lib/annotations/__tests__/CommentBox-test.js index 2487d0703..b37569fd8 100644 --- a/src/lib/annotations/__tests__/CommentBox-test.js +++ b/src/lib/annotations/__tests__/CommentBox-test.js @@ -62,12 +62,12 @@ describe('lib/annotations/CommentBox', () => { commentBox.show(); }); - it('should do nothing if the comment box HTML doesn\'t exist', () => { - commentBox.containerEl.remove(); - commentBox.containerEl = null; - + it('should do nothing if the textarea HTML doesn\'t exist', () => { const focus = sandbox.stub(commentBox.textAreaEl, 'focus'); + commentBox.textAreaEl.remove(); + commentBox.textAreaEl = null; + commentBox.focus(); expect(focus).to.not.be.called; }); @@ -84,16 +84,7 @@ describe('lib/annotations/CommentBox', () => { beforeEach(() => { // Kickstart creation of UI commentBox.show(); - }); - - it('should do nothing if the comment box HTML doesn\'t exist', () => { - commentBox.containerEl.remove(); - commentBox.containerEl = null; - const text = 'yay'; - commentBox.textAreaEl.value = text; - - commentBox.clear(); - expect(commentBox.textAreaEl.value).to.equal(text); + sandbox.stub(commentBox, 'preventDefaultAndPropagation'); }); it('should overwrite the text area\'s value with an empty string', () => { @@ -108,6 +99,7 @@ describe('lib/annotations/CommentBox', () => { beforeEach(() => { // Kickstart creation of UI commentBox.show(); + sandbox.stub(commentBox, 'preventDefaultAndPropagation'); }); it('should do nothing if the comment box HTML doesn\'t exist', () => { @@ -203,33 +195,41 @@ describe('lib/annotations/CommentBox', () => { }); describe('onCancel()', () => { + beforeEach(() => { + sandbox.stub(commentBox, 'preventDefaultAndPropagation'); + }); + it('should invoke clear()', () => { const clear = sandbox.stub(commentBox, 'clear'); - commentBox.onCancel(); + commentBox.onCancel({ preventDefault: () => {} }); expect(clear).to.be.called; }); it('should emit a cancel event', () => { const emit = sandbox.stub(commentBox, 'emit'); - commentBox.onCancel(); + commentBox.onCancel({ preventDefault: () => {} }); expect(emit).to.be.calledWith(CommentBox.CommentEvents.cancel); }); }); describe('onPost()', () => { + beforeEach(() => { + sandbox.stub(commentBox, 'preventDefaultAndPropagation'); + }); + it('should emit a post event with the value of the text box', () => { const emit = sandbox.stub(commentBox, 'emit'); const text = 'a comment'; commentBox.textAreaEl = { value: text }; - commentBox.onPost(); + commentBox.onPost({ preventDefault: () => {} }); expect(emit).to.be.calledWith(CommentBox.CommentEvents.post, text); }); it('should invoke clear()', () => { const clear = sandbox.stub(commentBox, 'clear'); - commentBox.onCancel(); + commentBox.onCancel({ preventDefault: () => {} }); expect(clear).to.be.called; }); }); diff --git a/src/lib/annotations/annotationConstants.js b/src/lib/annotations/annotationConstants.js index adec21ccb..8d5298518 100644 --- a/src/lib/annotations/annotationConstants.js +++ b/src/lib/annotations/annotationConstants.js @@ -5,12 +5,15 @@ export const CLASS_ANNOTATION_BUTTON_CANCEL = 'cancel-annotation-btn'; export const CLASS_ANNOTATION_BUTTON_POST = 'post-annotation-btn'; export const CLASS_ANNOTATION_DIALOG = 'bp-annotation-dialog'; export const CLASS_ANNOTATION_HIGHLIGHT_DIALOG = 'bp-annotation-highlight-dialog'; +export const CLASS_ANNOTATION_PLAIN_HIGHLIGHT = 'bp-plain-highlight'; +export const CLASS_ANNOTATION_POINT_BUTTON = 'bp-point-annotation-btn'; export const CLASS_ANNOTATION_POINT_MARKER = 'bp-point-annotation-marker'; export const CLASS_ANNOTATION_MODE = 'bp-annotation-mode'; export const CLASS_ANNOTATION_CARET = 'bp-annotation-caret'; export const CLASS_ANNOTATION_TEXTAREA = 'annotation-textarea'; export const CLASS_BUTTON_CONTAINER = 'button-container'; export const CLASS_ANNOTATION_CONTAINER = 'annotation-container'; +export const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog'; export const CLASS_MOBILE_ANNOTATION_DIALOG = 'bp-mobile-annotation-dialog'; export const CLASS_MOBILE_DIALOG_HEADER = 'bp-annotation-mobile-header'; export const CLASS_DIALOG_CLOSE = 'bp-annotation-dialog-close'; @@ -91,4 +94,5 @@ export const HIGHLIGHT_FILL = { export const PAGE_PADDING_TOP = 15; export const PAGE_PADDING_BOTTOM = 15; +export const ID_MOBILE_ANNOTATION_DIALOG = 'mobile-annotation-dialog'; export const DRAW_RENDER_THRESHOLD = 16.67; // 60 FPS target using 16.667ms/frame diff --git a/src/lib/annotations/annotatorUtil.js b/src/lib/annotations/annotatorUtil.js index a99908264..b7f5f8f36 100644 --- a/src/lib/annotations/annotatorUtil.js +++ b/src/lib/annotations/annotatorUtil.js @@ -48,11 +48,11 @@ export function findClosestElWithClass(element, className) { } /** -* Returns the page element and page number that the element is on. -* -* @param {HTMLElement} element - Element to find page and page number for -* @return {Object} Page element/page number if found or null/-1 if not -*/ + * Returns the page element and page number that the element is on. + * + * @param {HTMLElement} element - Element to find page and page number for + * @return {Object} Page element/page number if found or null/-1 if not + */ export function getPageInfo(element) { const pageEl = findClosestElWithClass(element, 'page') || null; let page = 1; @@ -139,7 +139,7 @@ export function showInvisibleElement(elementOrSelector) { /** * Hides the specified element or element with specified selector. The element - * will still take up DOM space but not be visible in the UI + * will still take up DOM space but not be visible in the UI. * * @param {HTMLElement|string} elementOrSelector - Element or CSS selector * @return {void} @@ -181,8 +181,8 @@ export function resetTextarea(element, clearText) { /** * Checks whether element is fully in viewport. * - * @param {HTMLElement} element - Element to check - * @return {boolean} Whether element is fully in viewport + * @param {HTMLElement} element - The element to check and see if it lies in the viewport + * @return {boolean} Whether the element is fully in viewport */ export function isElementInViewport(element) { const dimensions = element.getBoundingClientRect(); diff --git a/src/lib/annotations/doc/CreateHighlightDialog.js b/src/lib/annotations/doc/CreateHighlightDialog.js index 74f16e0f5..ea84fd90c 100644 --- a/src/lib/annotations/doc/CreateHighlightDialog.js +++ b/src/lib/annotations/doc/CreateHighlightDialog.js @@ -7,7 +7,7 @@ import * as constants from '../annotationConstants'; const CLASS_CREATE_DIALOG = 'bp-create-annotation-dialog'; const TITLE_HIGHLIGHT_TOGGLE = __('annotation_highlight_toggle'); const TITLE_HIGHLIGHT_COMMENT = __('annotation_highlight_comment'); -const DATA_TYPE_HIGHLIGHT = 'highlight-btn'; +const DATA_TYPE_HIGHLIGHT = 'add-highlight-btn'; const DATA_TYPE_ADD_HIGHLIGHT_COMMENT = 'add-highlight-comment-btn'; const CREATE_HIGHLIGHT_DIALOG_TEMPLATE = `
@@ -15,7 +15,7 @@ const CREATE_HIGHLIGHT_DIALOG_TEMPLATE = `
@@ -38,70 +38,56 @@ export const CreateEvents = { }; class CreateHighlightDialog extends EventEmitter { - /** - * Container element for the dialog. - * - * @property {HTMLElement} - */ + /** @property {HTMLElement} - Container element for the dialog. */ containerEl; - /** - * The clickable element for creating plain highlights. - * - * @property {HTMLElement} - */ + /** @property {HTMLElement} - The clickable element for creating plain highlights. */ highlightCreateEl; - /** - * The clickable element got creating comment highlights. - * - * @property {HTMLElement} - */ + /** @property {HTMLElement} - The clickable element got creating comment highlights. */ commentCreateEl; - /** - * The parent container to nest the dialog element in. - * - * @property {HTMLElement} - */ + /** @property {HTMLElement} - The parent container to nest the dialog element in. */ parentEl; - /** - * The element containing the buttons that can creaet highlights. - * - * @property {HTMLElement} - */ + /** @property {HTMLElement} - The element containing the buttons that can creaet highlights. */ buttonsEl; - /** - * The comment box instance. Contains area for text input and post/cancel buttons. - * - * @property {CommentBox} - */ + /** @property {CommentBox} - The comment box instance. Contains area for text input and post/cancel buttons. */ commentBox; - /** - * Position, on the DOM, to align the dialog to the end of a highlight. - * - * @property {Object} - */ + /** @property {Object} - Position, on the DOM, to align the dialog to the end of a highlight. */ position = { x: 0, y: 0 }; + /** @property {boolean} - Whether or not we're on a mobile device. */ + isMobile; + + /** @property {boolean} - Whether or not we support touch. */ + hasTouch; + + /** @property {boolean} - Whether or not this is visible. */ + isVisible; + /** * A dialog used to create plain and comment highlights. * * [constructor] * * @param {HTMLElement} parentEl - Parent element + * @param {Object} [config] - For configuring the dialog. + * @param {boolean} [config.hasTouch] - True to add touch events. + * @param {boolean} [config.isMobile] - True if on a mobile device. * @return {CreateHighlightDialog} CreateHighlightDialog instance */ - constructor(parentEl) { + constructor(parentEl, config = {}) { super(); this.parentEl = parentEl; + this.isMobile = config.isMobile || false; + this.hasTouch = config.hasTouch || false; // Explicit scope binding for event listeners this.onHighlightClick = this.onHighlightClick.bind(this); @@ -143,6 +129,7 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ show(newParentEl) { + this.isVisible = true; if (!this.containerEl) { this.containerEl = this.createElement(); } @@ -158,6 +145,7 @@ class CreateHighlightDialog extends EventEmitter { } this.setButtonVisibility(true); + showElement(this.containerEl); } @@ -167,11 +155,13 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ hide() { + this.isVisible = false; if (!this.containerEl) { return; } hideElement(this.containerEl); + this.commentBox.hide(); this.commentBox.clear(); } @@ -191,7 +181,6 @@ class CreateHighlightDialog extends EventEmitter { // Stop interacting with this element from triggering outside actions this.containerEl.removeEventListener('click', this.stopPropagation); this.containerEl.removeEventListener('mouseup', this.stopPropagation); - this.containerEl.removeEventListener('touchend', this.stopPropagation); this.containerEl.removeEventListener('dblclick', this.stopPropagation); // Event listeners @@ -200,6 +189,14 @@ class CreateHighlightDialog extends EventEmitter { this.commentBox.removeListener(CommentBox.CommentEvents.post, this.onCommentPost); this.commentBox.removeListener(CommentBox.CommentEvents.cancel, this.onCommentCancel); + if (this.hasTouch) { + this.highlightCreateEl.removeEventListener('touchstart', this.stopPropagation); + this.commentCreateEl.removeEventListener('touchstart', this.stopPropagation); + this.highlightCreateEl.removeEventListener('touchend', this.onHighlightClick); + this.commentCreateEl.removeEventListener('touchend', this.onCommentClick); + this.containerEl.removeEventListener('touchend', this.stopPropagation); + } + this.containerEl.remove(); this.containerEl = null; this.parentEl = null; @@ -219,6 +216,10 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ updatePosition() { + if (this.isMobile) { + return; + } + // Plus 1 pixel for caret this.containerEl.style.left = `${this.position.x - 1 - this.containerEl.clientWidth / 2}px`; // Plus 5 pixels for caret @@ -228,9 +229,12 @@ class CreateHighlightDialog extends EventEmitter { /** * Fire an event notifying that the plain highlight button has been clicked. * + * @param {Event} event - The DOM event coming from interacting with the element. * @return {void} */ - onHighlightClick() { + onHighlightClick(event) { + event.preventDefault(); + event.stopPropagation(); this.emit(CreateEvents.plain); } @@ -238,9 +242,12 @@ class CreateHighlightDialog extends EventEmitter { * Fire an event notifying that the comment button has been clicked. Also * show the comment box, and give focus to the text area conatined by it. * + * @param {Event} event - The DOM event coming from interacting with the element. * @return {void} */ - onCommentClick() { + onCommentClick(event) { + event.preventDefault(); + event.stopPropagation(); this.emit(CreateEvents.comment); this.commentBox.show(); @@ -258,7 +265,10 @@ class CreateHighlightDialog extends EventEmitter { */ onCommentPost(text) { this.emit(CreateEvents.commentPost, text); - this.commentBox.clear(); + if (text) { + this.commentBox.clear(); + this.commentBox.blur(); + } } /** @@ -309,6 +319,13 @@ class CreateHighlightDialog extends EventEmitter { highlightDialogEl.classList.add(CLASS_CREATE_DIALOG); highlightDialogEl.innerHTML = CREATE_HIGHLIGHT_DIALOG_TEMPLATE; + // Get rid of the caret + if (this.isMobile) { + highlightDialogEl.classList.add('bp-mobile-annotation-dialog'); + highlightDialogEl.classList.add('bp-annotation-dialog'); + highlightDialogEl.querySelector('.bp-annotation-caret').remove(); + } + const containerEl = highlightDialogEl.querySelector(constants.SELECTOR_ANNOTATION_HIGHLIGHT_DIALOG); // Reference HTML @@ -322,7 +339,6 @@ class CreateHighlightDialog extends EventEmitter { // Stop interacting with this element from triggering outside actions highlightDialogEl.addEventListener('click', this.stopPropagation); highlightDialogEl.addEventListener('mouseup', this.stopPropagation); - highlightDialogEl.addEventListener('touchend', this.stopPropagation); highlightDialogEl.addEventListener('dblclick', this.stopPropagation); // Event listeners @@ -331,6 +347,15 @@ class CreateHighlightDialog extends EventEmitter { this.commentBox.addListener(CommentBox.CommentEvents.post, this.onCommentPost); this.commentBox.addListener(CommentBox.CommentEvents.cancel, this.onCommentCancel); + // touch events + if (this.hasTouch) { + this.highlightCreateEl.addEventListener('touchstart', this.stopPropagation); + this.commentCreateEl.addEventListener('touchstart', this.stopPropagation); + this.highlightCreateEl.addEventListener('touchend', this.onHighlightClick); + this.commentCreateEl.addEventListener('touchend', this.onCommentClick); + highlightDialogEl.addEventListener('touchend', this.stopPropagation); + } + // Hide comment box, by default this.commentBox.hide(); diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index ccabbcdb3..211e679af 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -27,10 +27,14 @@ import { const MOUSEMOVE_THROTTLE_MS = 50; const HOVER_TIMEOUT_MS = 75; const MOUSE_MOVE_MIN_DISTANCE = 5; +const CLASS_RANGY_HIGHLIGHT = 'rangy-highlight'; const SELECTOR_PREVIEW_DOC = '.bp-doc'; const CLASS_DEFAULT_CURSOR = 'bp-use-default-cursor'; +// Required by rangy highlighter +const ID_ANNOTATED_ELEMENT = 'bp-rangy-annotated-element'; + /** * For filtering out and only showing the first thread in a list of threads. * @@ -58,49 +62,28 @@ function isThreadInHoverState(thread) { @autobind class DocAnnotator extends Annotator { - /** - * For tracking the most recent event fired by mouse move event. - * - * @property {Event} - */ + /** @property {Event} - For tracking the most recent event fired by mouse move event. */ mouseMoveEvent; - /** - * Event callback for mouse move events with for highlight annotations. - * - * @property {Function} - */ + /** @property {Function} - Event callback for mouse move events with for highlight annotations. */ highlightMousemoveHandler; - /** - * Handle to RAF used to throttle highlight collision checks. - * - * @property {Function} - */ + /** @property {Function} - Handle to RAF used to throttle highlight collision checks. */ highlightThrottleHandle; - /** - * Timer used to throttle highlight event process. - * - * @property {number} - */ + /** @property {number} - Timer used to throttle highlight event process. */ throttleTimer = 0; - /** - * UI used to create new highlight annotations. - * - * @property {CreateHighlightDialog} - */ + /** @property {CreateHighlightDialog} - UI used to create new highlight annotations. */ createHighlightDialog; - /** - * For delaying creation of highlight quad points and dialog. Tracks the - * current selection event, made in a previous event. - * - * @property {Event} - */ + /** @property {Event} - For delaying creation of highlight quad points and dialog. Tracks the + * current selection event, made in a previous event. */ lastHighlightEvent; + /** @property {Selection} - For tracking diffs in text selection, for mobile highlights creation. */ + lastSelection; + /** * Creates and mananges plain highlight and comment highlight and point annotations * on document files. @@ -117,12 +100,14 @@ class DocAnnotator extends Annotator { this.highlightCurrentSelection = this.highlightCurrentSelection.bind(this); this.createHighlightThread = this.createHighlightThread.bind(this); this.createPlainHighlight = this.createPlainHighlight.bind(this); + this.highlightCreateHandler = this.highlightCreateHandler.bind(this); - this.createHighlightDialog = new CreateHighlightDialog(); + this.createHighlightDialog = new CreateHighlightDialog(this.container, { + isMobile: this.isMobile, + hasTouch: this.hasTouch + }); this.createHighlightDialog.addListener(CreateEvents.plain, this.createPlainHighlight); - this.createHighlightDialog.addListener(CreateEvents.comment, this.highlightCurrentSelection); - this.createHighlightDialog.addListener(CreateEvents.commentPost, this.createHighlightThread); } @@ -135,14 +120,20 @@ class DocAnnotator extends Annotator { super.destroy(); this.createHighlightDialog.removeListener(CreateEvents.plain, this.createPlainHighlight); - this.createHighlightDialog.removeListener(CreateEvents.comment, this.highlightCurrentSelection); - this.createHighlightDialog.removeListener(CreateEvents.commentPost, this.createHighlightThread); this.createHighlightDialog.destroy(); this.createHighlightDialog = null; } + /** @inheritdoc */ + init(initialScale) { + super.init(initialScale); + + // Allow rangy to highlight this + this.annotatedElement.id = ID_ANNOTATED_ELEMENT; + } + //-------------------------------------------------------------------------- // Abstract Implementations //-------------------------------------------------------------------------- @@ -174,22 +165,22 @@ class DocAnnotator extends Annotator { let location = null; const zoomScale = annotatorUtil.getScale(this.annotatedElement); - let clientEvent = event; - if (this.isMobile) { - if (!event.targetTouches || event.targetTouches.length === 0) { - return location; + if (annotationType === TYPES.point) { + let clientEvent = event; + if (this.isMobile) { + if (!event.targetTouches || event.targetTouches.length === 0) { + return location; + } + clientEvent = event.targetTouches[0]; } - clientEvent = event.targetTouches[0]; - } - // If click isn't on a page, ignore - const eventTarget = clientEvent.target; - const { pageEl, page } = annotatorUtil.getPageInfo(eventTarget); - if (!pageEl) { - return location; - } + // If click isn't on a page, ignore + const eventTarget = clientEvent.target; + const { pageEl, page } = annotatorUtil.getPageInfo(eventTarget); + if (!pageEl) { + return location; + } - if (annotationType === TYPES.point) { // If there is a selection, ignore if (docAnnotatorUtil.isSelectionPresent()) { return location; @@ -238,6 +229,15 @@ class DocAnnotator extends Annotator { return location; } + // Get correct page + let { pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode); + if (!pageEl) { + // The ( .. ) around assignment is required syntax + ({ pageEl, page } = annotatorUtil.getPageInfo( + this.annotatedElement.querySelector(`.${CLASS_RANGY_HIGHLIGHT}`) + )); + } + // Use highlight module to calculate quad points const { highlightEls } = docAnnotatorUtil.getHighlightAndHighlightEls(this.highlighter, pageEl); @@ -344,6 +344,7 @@ class DocAnnotator extends Annotator { this.isCreatingHighlight = false; const location = this.getLocationFromEvent(this.lastHighlightEvent, TYPES.highlight); + this.highlighter.removeAllHighlights(); if (!location) { return null; } @@ -351,6 +352,7 @@ class DocAnnotator extends Annotator { const annotations = []; const thread = this.createAnnotationThread(annotations, location, TYPES.highlight); this.lastHighlightEvent = null; + this.lastSelection = null; if (!thread) { return null; @@ -406,7 +408,7 @@ class DocAnnotator extends Annotator { // Init rangy and rangy highlight this.highlighter = rangy.createHighlighter(); this.highlighter.addClassApplier( - rangy.createClassApplier('rangy-highlight', { + rangy.createClassApplier(CLASS_RANGY_HIGHLIGHT, { ignoreWhiteSpace: true, tagNames: ['span', 'a'] }) @@ -425,7 +427,13 @@ class DocAnnotator extends Annotator { this.annotatedElement.addEventListener('mouseup', this.highlightMouseupHandler); - if (this.canAnnotate) { + if (!this.canAnnotate) { + return; + } + + if (this.hasTouch && this.isMobile) { + document.addEventListener('selectionchange', this.onSelectionChange); + } else { this.annotatedElement.addEventListener('dblclick', this.highlightMouseupHandler); this.annotatedElement.addEventListener('mousedown', this.highlightMousedownHandler); this.annotatedElement.addEventListener('contextmenu', this.highlightMousedownHandler); @@ -445,17 +453,23 @@ class DocAnnotator extends Annotator { this.annotatedElement.removeEventListener('mouseup', this.highlightMouseupHandler); - if (this.canAnnotate) { + if (this.highlightThrottleHandle) { + cancelAnimationFrame(this.highlightThrottleHandle); + this.highlightThrottleHandle = null; + } + + if (!this.canAnnotate) { + return; + } + + if (this.hasTouch && this.isMobile) { + document.removeEventListener('selectionchange', this.onSelectionChange); + } else { this.annotatedElement.removeEventListener('dblclick', this.highlightMouseupHandler); this.annotatedElement.removeEventListener('mousedown', this.highlightMousedownHandler); this.annotatedElement.removeEventListener('contextmenu', this.highlightMousedownHandler); this.annotatedElement.removeEventListener('mousemove', this.highlightMousemoveHandler); this.highlightMousemoveHandler = null; - - if (this.highlightThrottleHandle) { - cancelAnimationFrame(this.highlightThrottleHandle); - this.highlightThrottleHandle = null; - } } } @@ -501,6 +515,50 @@ class DocAnnotator extends Annotator { // Private //-------------------------------------------------------------------------- + /** + * Handles changes in text selection. Used for mobile highlight creation. + * + * @param {Event} event - The DOM event coming from interacting with the element. + * @return {void} + */ + onSelectionChange(event) { + // Do nothing if in a text area + if (document.activeElement.nodeName.toLowerCase() === 'textarea') { + return; + } + + const selection = window.getSelection(); + + // If we're creating a new selection, make sure to clear out to avoid + // incorrect text being selected + if (!this.lastSelection || (this.lastSelection && selection.anchorNode !== this.lastSelection.anchorNode)) { + this.highlighter.removeAllHighlights(); + } + + // Bail if mid highlight and tapping on the screen + if (!docAnnotatorUtil.isValidSelection(selection)) { + this.lastSelection = null; + this.lastHighlightEvent = null; + this.createHighlightDialog.hide(); + this.highlighter.removeAllHighlights(); + return; + } + + if (!this.createHighlightDialog.isVisble) { + this.createHighlightDialog.show(this.container); + } + + // Set all annotations that are in the 'hover' state to 'inactive' + Object.keys(this.threads).forEach((threadPage) => { + this.getHighlightThreadsOnPage(threadPage).filter(isThreadInHoverState).forEach((thread) => { + thread.reset(); + }); + }); + + this.lastSelection = selection; + this.lastHighlightEvent = event; + } + /** * Highlight the current range of text that has been selected. * @@ -693,6 +751,7 @@ class DocAnnotator extends Annotator { if (this.highlighter) { this.highlighter.removeAllHighlights(); } + this.createHighlightDialog.hide(); this.isCreatingHighlight = false; @@ -721,13 +780,12 @@ class DocAnnotator extends Annotator { event.stopPropagation(); const selection = window.getSelection(); - if (selection.rangeCount <= 0 || selection.isCollapsed) { + if (!docAnnotatorUtil.isValidSelection(selection)) { return; } - // Only filter through highlight threads on the current page - // Reset active highlight threads before creating new highlight - const { pageEl } = annotatorUtil.getPageInfo(event.target); + // Select page of first node selected + const { pageEl } = annotatorUtil.getPageInfo(selection.anchorNode); if (!pageEl) { return; @@ -745,8 +803,10 @@ class DocAnnotator extends Annotator { const pageDimensions = pageEl.getBoundingClientRect(); const pageLeft = pageDimensions.left; const pageTop = pageDimensions.top + PAGE_PADDING_TOP; + const dialogParentEl = this.isMobile ? this.container : pageEl; + + this.createHighlightDialog.show(dialogParentEl); - this.createHighlightDialog.show(pageEl); if (!this.isMobile) { this.createHighlightDialog.setPosition(right - pageLeft, bottom - pageTop); } diff --git a/src/lib/annotations/doc/DocHighlightDialog.js b/src/lib/annotations/doc/DocHighlightDialog.js index 5fd319d5c..70058230b 100644 --- a/src/lib/annotations/doc/DocHighlightDialog.js +++ b/src/lib/annotations/doc/DocHighlightDialog.js @@ -6,7 +6,6 @@ import { ICON_HIGHLIGHT, ICON_HIGHLIGHT_COMMENT } from '../../icons/icons'; import * as constants from '../annotationConstants'; 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'; @@ -20,7 +19,7 @@ class DocHighlightDialog extends AnnotationDialog { // Public //-------------------------------------------------------------------------- - /** D + /** * Saves an annotation with the associated text or blank if only * highlighting. Only adds an annotation to the dialog if it contains text. * The annotation is still added to the thread on the server side. @@ -39,13 +38,57 @@ class DocHighlightDialog extends AnnotationDialog { ]); annotatorUtil.showElement(highlightLabelEl); - // Only reposition if mouse is still hovering over the dialog - this.position(); + // Only reposition if mouse is still hovering over the dialog and not mobile + if (!this.isMobile) { + this.position(); + } } super.addAnnotation(annotation); } + /** @inheritdoc */ + postAnnotation(textInput) { + const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA); + const text = textInput || annotationTextEl.value; + if (text.trim() === '') { + return; + } + + // Convert from plain highlight to comment + const headerEl = this.element.querySelector('.bp-annotation-mobile-header'); + if (headerEl) { + headerEl.classList.remove(constants.CLASS_HIDDEN); + this.element.classList.remove(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT); + } + + super.postAnnotation(textInput); + } + + /** + * Set the state of the dialog so comments are hidden, if they're currently shown. + * + * @public + * @return {void} + */ + hideCommentsDialog() { + if (!this.commentsDialogEl || !this.highlightDialogEl) { + return; + } + + // Displays comments dialog and hides highlight annotations button + const commentsDialogIsHidden = this.commentsDialogEl.classList.contains(constants.CLASS_HIDDEN); + if (commentsDialogIsHidden) { + return; + } + + annotatorUtil.hideElement(this.commentsDialogEl); + + this.element.classList.add(CLASS_HIGHLIGHT_DIALOG); + annotatorUtil.showElement(this.highlightDialogEl); + this.hasComments = false; + } + /** * Emit the message to create a highlight and render it. * @@ -76,7 +119,6 @@ class DocHighlightDialog extends AnnotationDialog { const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM; const [browserX, browserY] = this.getScaledPDFCoordinates(pageDimensions, pageHeight); - pageEl.appendChild(this.element); const highlightDialogWidth = this.getDialogWidth(); @@ -117,9 +159,14 @@ class DocHighlightDialog extends AnnotationDialog { * highlight comments dialog. Dialogs are toggled based on whether the * highlight annotation has text comments or not. * + * @override * @return {void} */ toggleHighlightDialogs() { + if (!this.commentsDialogEl || !this.highlightDialogEl) { + return; + } + const commentsDialogIsHidden = this.commentsDialogEl.classList.contains(constants.CLASS_HIDDEN); // Displays comments dialog and hides highlight annotations button @@ -130,7 +177,6 @@ class DocHighlightDialog extends AnnotationDialog { this.element.classList.add(constants.CLASS_ANNOTATION_DIALOG); annotatorUtil.showElement(this.commentsDialogEl); this.hasComments = true; - // Activate comments textarea const textAreaEl = this.dialogEl.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA); textAreaEl.classList.add(constants.CLASS_ACTIVE); @@ -146,7 +192,9 @@ class DocHighlightDialog extends AnnotationDialog { } // Reposition dialog - this.position(); + if (!this.isMobile) { + this.position(); + } } /** @@ -203,7 +251,7 @@ class DocHighlightDialog extends AnnotationDialog { // Generate HTML of highlight dialog this.highlightDialogEl = this.generateHighlightDialogEl(); - this.highlightDialogEl.classList.add(CLASS_ANNOTATION_HIGHLIGHT_DIALOG); + this.highlightDialogEl.classList.add(constants.CLASS_ANNOTATION_HIGHLIGHT_DIALOG); // Generate HTML of comments dialog this.commentsDialogEl = this.generateDialogEl(annotations.length); @@ -475,7 +523,7 @@ class DocHighlightDialog extends AnnotationDialog { const highlightDialogEl = document.createElement('div'); highlightDialogEl.innerHTML = ` - +