From 6d9d8c4156d0c94b6dc209128c579921aa17e293 Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Wed, 19 Jul 2017 14:44:28 -0700 Subject: [PATCH] New: Mobile text highlight annotations --- src/lib/annotations/AnnotationDialog.js | 27 ++-- src/lib/annotations/Annotator.js | 2 + src/lib/annotations/MobileAnnotator.scss | 52 ++++++-- .../__tests__/AnnotationDialog-test.js | 11 +- .../__tests__/AnnotationThread-test.js | 2 +- src/lib/annotations/annotationConstants.js | 4 + src/lib/annotations/annotatorUtil.js | 16 +-- .../annotations/doc/CreateHighlightDialog.js | 24 +++- src/lib/annotations/doc/DocAnnotator.js | 59 +++++++-- src/lib/annotations/doc/DocHighlightDialog.js | 66 ++++++++-- src/lib/annotations/doc/DocHighlightThread.js | 19 +-- .../doc/__tests__/DocAnnotator-test.js | 40 ++++++ .../doc/__tests__/DocHighlightDialog-test.js | 116 ++++++++++++++++++ .../doc/__tests__/DocHighlightThread-test.js | 2 +- .../doc/__tests__/docAnnotatorUtil-test.js | 41 ++++++- src/lib/annotations/doc/docAnnotatorUtil.js | 19 ++- 16 files changed, 429 insertions(+), 71 deletions(-) diff --git a/src/lib/annotations/AnnotationDialog.js b/src/lib/annotations/AnnotationDialog.js index 9710b76ab..a44bfce6b 100644 --- a/src/lib/annotations/AnnotationDialog.js +++ b/src/lib/annotations/AnnotationDialog.js @@ -6,14 +6,13 @@ import { CLASS_ACTIVE, CLASS_HIDDEN } from '../constants'; import { decodeKeydown } from '../util'; 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'; @@ -34,6 +33,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 @@ -92,8 +92,8 @@ 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); + if (this.highlightDialogEl && !this.element.querySelectorAll(`.${CLASS_COMMENT}`).length) { + this.element.classList.add(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT); const headerEl = this.element.querySelector(constants.SELECTOR_MOBILE_DIALOG_HEADER); headerEl.classList.add(CLASS_HIDDEN); @@ -102,7 +102,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(); } @@ -157,14 +157,14 @@ class AnnotationDialog extends EventEmitter { return; } - this.element.classList.remove(CLASS_ANIMATE_DIALOG); + 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); @@ -173,9 +173,7 @@ class AnnotationDialog extends EventEmitter { this.unbindDOMListeners(); // Cancel any unsaved annotations - if (!this.hasAnnotations) { - this.cancelAnnotation(); - } + this.cancelAnnotation(); } /** @@ -184,6 +182,10 @@ class AnnotationDialog extends EventEmitter { * @return {void} */ hide() { + if (this.element && this.element.classList.contains(CLASS_HIDDEN)) { + return; + } + if (this.isMobile) { this.hideMobileDialog(); } @@ -415,8 +417,7 @@ class AnnotationDialog extends EventEmitter { // Clicking 'Cancel' button to cancel the annotation case 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(); @@ -489,7 +490,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/Annotator.js b/src/lib/annotations/Annotator.js index 817363235..fb729d5fb 100644 --- a/src/lib/annotations/Annotator.js +++ b/src/lib/annotations/Annotator.js @@ -12,6 +12,7 @@ import { CLASS_ANNOTATION_DIALOG, CLASS_MOBILE_DIALOG_HEADER, CLASS_DIALOG_CLOSE, + ID_MOBILE_ANNOTATION_DIALOG, TYPES } from './annotationConstants'; @@ -112,6 +113,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/MobileAnnotator.scss b/src/lib/annotations/MobileAnnotator.scss index f904abed8..3f56e5e13 100644 --- a/src/lib/annotations/MobileAnnotator.scss +++ b/src/lib/annotations/MobileAnnotator.scss @@ -1,4 +1,11 @@ $tablet: "(min-width: 768px)"; +$three-c: #ccc; + +@mixin border-top-bottom { + border-color: $three-c; + border-style: solid; + border-width: 1px 0; +} .bp-mobile-annotation-dialog { background: white; @@ -9,6 +16,8 @@ $tablet: "(min-width: 768px)"; width: 100%; &.bp-animate-show-dialog { + // padding: 0; + &:not(.bp-plain-highlight) { animation: show-dialog-small; animation-duration: .2s; @@ -18,7 +27,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 $three-c; width: 450px; } } @@ -31,6 +40,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 +74,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 +86,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 +94,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 +170,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 +200,13 @@ $tablet: "(min-width: 768px)"; text-align: center; z-index: 9999; - .bp-annotations-highlight-btns button { + .bp-annotation-highlight-btns button { + float: left; width: 50%; + + &:first-child { + border-right: 1px solid $three-c; + } } &.cannot-annotate { diff --git a/src/lib/annotations/__tests__/AnnotationDialog-test.js b/src/lib/annotations/__tests__/AnnotationDialog-test.js index e032bf93a..30924d64a 100644 --- a/src/lib/annotations/__tests__/AnnotationDialog-test.js +++ b/src/lib/annotations/__tests__/AnnotationDialog-test.js @@ -5,7 +5,6 @@ import * as annotatorUtil from '../annotatorUtil'; import * as constants from '../annotationConstants'; import { CLASS_ACTIVE, CLASS_HIDDEN } from '../../constants'; -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'; @@ -166,15 +165,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; }); }); @@ -196,7 +195,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 560167978..49bdebd01 100644 --- a/src/lib/annotations/__tests__/AnnotationThread-test.js +++ b/src/lib/annotations/__tests__/AnnotationThread-test.js @@ -480,7 +480,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/annotationConstants.js b/src/lib/annotations/annotationConstants.js index 7903de518..4c84208bd 100644 --- a/src/lib/annotations/annotationConstants.js +++ b/src/lib/annotations/annotationConstants.js @@ -2,12 +2,14 @@ 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_MODE = 'bp-point-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'; @@ -61,3 +63,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'; diff --git a/src/lib/annotations/annotatorUtil.js b/src/lib/annotations/annotatorUtil.js index 804912d24..f8ced24eb 100644 --- a/src/lib/annotations/annotatorUtil.js +++ b/src/lib/annotations/annotatorUtil.js @@ -34,11 +34,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; @@ -125,7 +125,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} @@ -167,8 +167,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..44d693285 100644 --- a/src/lib/annotations/doc/CreateHighlightDialog.js +++ b/src/lib/annotations/doc/CreateHighlightDialog.js @@ -90,18 +90,27 @@ class CreateHighlightDialog extends EventEmitter { y: 0 }; + /** + * Whether or not we're on a mobile device. + * + * @property {boolean} + */ + isMobile; + /** * A dialog used to create plain and comment highlights. * * [constructor] * * @param {HTMLElement} parentEl - Parent element + * @param {boolean} isMobile - Whether or not this is running on a mobile device * @return {CreateHighlightDialog} CreateHighlightDialog instance */ - constructor(parentEl) { + constructor(parentEl, isMobile = false) { super(); this.parentEl = parentEl; + this.isMobile = isMobile; // Explicit scope binding for event listeners this.onHighlightClick = this.onHighlightClick.bind(this); @@ -158,6 +167,7 @@ class CreateHighlightDialog extends EventEmitter { } this.setButtonVisibility(true); + showElement(this.containerEl); } @@ -172,6 +182,7 @@ class CreateHighlightDialog extends EventEmitter { } hideElement(this.containerEl); + this.commentBox.hide(); this.commentBox.clear(); } @@ -219,6 +230,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 @@ -308,6 +323,12 @@ class CreateHighlightDialog extends EventEmitter { const highlightDialogEl = document.createElement('div'); 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); @@ -323,6 +344,7 @@ class CreateHighlightDialog extends EventEmitter { highlightDialogEl.addEventListener('click', this.stopPropagation); highlightDialogEl.addEventListener('mouseup', this.stopPropagation); highlightDialogEl.addEventListener('touchend', this.stopPropagation); + highlightDialogEl.addEventListener('touchstart', this.stopPropagation); highlightDialogEl.addEventListener('dblclick', this.stopPropagation); // Event listeners diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index b03b8a507..9b4f1542f 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -26,6 +26,7 @@ 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'; @@ -100,6 +101,13 @@ class DocAnnotator extends Annotator { */ lastHighlightEvent; + /** + * Container element for mobile annotation dialogs. + * + * @property {HTMLElement} + */ + mobileDialogParentEl; + /** * Creates and mananges plain highlight and comment highlight and point annotations * on document files. @@ -116,8 +124,10 @@ 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.onTouchStart = this.onTouchStart.bind(this); - this.createHighlightDialog = new CreateHighlightDialog(); + this.createHighlightDialog = new CreateHighlightDialog(undefined, this.isMobile); this.createHighlightDialog.addListener(CreateEvents.plain, this.createPlainHighlight); this.createHighlightDialog.addListener(CreateEvents.comment, this.highlightCurrentSelection); @@ -222,10 +232,12 @@ class DocAnnotator extends Annotator { } // Get correct page - let { pageEl, page } = annotatorUtil.getPageInfo(event.target); + let { pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode); if (page === -1) { // The ( .. ) around assignment is required syntax - ({ pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode)); + ({ pageEl, page } = annotatorUtil.getPageInfo( + this.annotatedElement.querySelector(`.${CLASS_RANGY_HIGHLIGHT}`) + )); } // Use highlight module to calculate quad points @@ -391,7 +403,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'] }) @@ -413,6 +425,10 @@ class DocAnnotator extends Annotator { this.annotatedElement.addEventListener('mousedown', this.highlightMousedownHandler); this.annotatedElement.addEventListener('contextmenu', this.highlightMousedownHandler); this.annotatedElement.addEventListener('mousemove', this.getHighlightMouseMoveHandler()); + if (this.isMobile) { + document.addEventListener('selectionchange', this.highlightCreateHandler); + document.addEventListener('touchstart', this.onTouchStart); + } } } @@ -431,12 +447,16 @@ class DocAnnotator extends Annotator { this.annotatedElement.removeEventListener('mousedown', this.highlightMousedownHandler); this.annotatedElement.removeEventListener('contextmenu', this.highlightMousedownHandler); this.annotatedElement.removeEventListener('mousemove', this.getHighlightMouseMoveHandler()); - - if (this.highlightThrottleHandle) { - cancelAnimationFrame(this.highlightThrottleHandle); - this.highlightThrottleHandle = null; + if (this.isMobile) { + document.removeEventListener('selectionchange', this.highlightCreateHandler); + document.removeEventListener('touchstart', this.onTouchStart); } } + + if (this.highlightThrottleHandle) { + cancelAnimationFrame(this.highlightThrottleHandle); + this.highlightThrottleHandle = null; + } } /** @@ -674,6 +694,18 @@ class DocAnnotator extends Annotator { this.isCreatingHighlight = false; } + /** + * Handle touch start event. + * + * @return {void} + */ + onTouchStart() { + if (this.highlighter) { + this.highlighter.removeAllHighlights(); + } + this.createHighlightDialog.hide(); + } + /** * Handler for creating a pending highlight thread from the current * selection. Default creates highlight threads as ANNOTATION_TYPE_HIGHLIGHT. @@ -688,13 +720,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; @@ -712,8 +743,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 87b23e48e..59c319364 100644 --- a/src/lib/annotations/doc/DocHighlightDialog.js +++ b/src/lib/annotations/doc/DocHighlightDialog.js @@ -8,7 +8,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'; const DATA_TYPE_HIGHLIGHT = 'highlight-btn'; @@ -24,7 +23,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. @@ -43,13 +42,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.isMobile ? this.element.querySelector('.bp-annotation-mobile-header') : null; + if (headerEl) { + headerEl.classList.remove(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; + } + + const commentsDialogIsHidden = this.commentsDialogEl.classList.contains(CLASS_HIDDEN); + + // Displays comments dialog and hides highlight annotations button + 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. * @@ -80,7 +123,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(); @@ -121,9 +163,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(CLASS_HIDDEN); // Displays comments dialog and hides highlight annotations button @@ -134,7 +181,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(CLASS_ACTIVE); @@ -150,7 +196,9 @@ class DocHighlightDialog extends AnnotationDialog { } // Reposition dialog - this.position(); + if (!this.isMobile) { + this.position(); + } } /** @@ -207,7 +255,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); @@ -479,7 +527,7 @@ class DocHighlightDialog extends AnnotationDialog { const highlightDialogEl = document.createElement('div'); highlightDialogEl.innerHTML = ` - +