From 0e5714a8ba3a8c57c93ede071d40b7b7addefeea Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Mon, 26 Jun 2017 11:45:07 -0700 Subject: [PATCH 1/9] Chore: Separate create highlight dialog and quad point creation --- src/lib/annotations/AnnotationDialog.js | 34 +- src/lib/annotations/Annotator.scss | 4 +- src/lib/annotations/CommentBox.js | 239 +++++++++++++ src/lib/annotations/MobileAnnotator.scss | 3 +- .../annotations/doc/CreateHighlightDialog.js | 338 ++++++++++++++++++ src/lib/annotations/doc/DocAnnotator.js | 177 ++++++++- .../annotations/doc/DocHighlightAnnotator.js | 118 ++++++ src/lib/annotations/doc/DocHighlightDialog.js | 16 +- src/lib/annotations/doc/DocPointAnnotator.js | 57 +++ .../doc/__tests__/DocAnnotator-test.js | 109 ++++-- src/lib/annotations/doc/docAnnotatorUtil.js | 16 +- src/lib/annotations/doc/documentConstants.js | 2 + 12 files changed, 1046 insertions(+), 67 deletions(-) create mode 100644 src/lib/annotations/CommentBox.js create mode 100644 src/lib/annotations/doc/CreateHighlightDialog.js create mode 100644 src/lib/annotations/doc/DocHighlightAnnotator.js create mode 100644 src/lib/annotations/doc/DocPointAnnotator.js create mode 100644 src/lib/annotations/doc/documentConstants.js diff --git a/src/lib/annotations/AnnotationDialog.js b/src/lib/annotations/AnnotationDialog.js index 39251c01e..73a4adf63 100644 --- a/src/lib/annotations/AnnotationDialog.js +++ b/src/lib/annotations/AnnotationDialog.js @@ -206,6 +206,23 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog'; } } + /** + * Posts an annotation in the dialog. + * + * @public + * @return {void} + */ + postAnnotation(textInput) { + const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA); + const text = textInput || annotationTextEl.value; + if (text.trim() === '') { + return; + } + + this.emit('annotationcreate', { text }); + annotationTextEl.value = ''; + } + //-------------------------------------------------------------------------- // Abstract //-------------------------------------------------------------------------- @@ -479,23 +496,6 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog'; annotationContainerEl.appendChild(annotationEl); } - /** - * Posts an annotation in the dialog. - * - * @private - * @return {void} - */ - postAnnotation() { - const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA); - const text = annotationTextEl.value; - if (text.trim() === '') { - return; - } - - this.emit('annotationcreate', { text }); - annotationTextEl.value = ''; - } - /** * Cancels posting an annotation. * diff --git a/src/lib/annotations/Annotator.scss b/src/lib/annotations/Annotator.scss index a5e756814..7fdbf6d92 100644 --- a/src/lib/annotations/Annotator.scss +++ b/src/lib/annotations/Annotator.scss @@ -103,7 +103,8 @@ $avatar-color-9: #f22c44; // CSS for points and dialogs //------------------------------------------------------------------------------ -.bp-annotation-dialog { +.bp-annotation-dialog, +.bp-create-annotation-dialog { border-top: 20px solid transparent; // Transparent border for hover detection cursor: default; // Overrides cursor: none from annotation mode position: absolute; @@ -376,6 +377,7 @@ $avatar-color-9: #f22c44; // Quad point positioning - the helper divs are positioned relative to the Rangy-created element .bp-doc .rangy-highlight { position: relative; + background-color: #ffff00; } // These helper divs allow us to calculate the quad points of an element diff --git a/src/lib/annotations/CommentBox.js b/src/lib/annotations/CommentBox.js new file mode 100644 index 000000000..c7a36126b --- /dev/null +++ b/src/lib/annotations/CommentBox.js @@ -0,0 +1,239 @@ +import EventEmitter from 'events'; +import { CLASS_HIDDEN, CLASS_ACTIVE } from '../constants'; + +// Display Text +const TEXT_ANNOTATION_CANCEL = __('annotation_cancel'); +const TEXT_ANNOTATION_POST = __('annotation_post'); +const TEXT_ADD_COMMENT_PLACEHOLDER = __('annotation_add_comment_placeholder'); + +// Styling + +class CommentBox extends EventEmitter { + /** + * Text displayed in the Cancel button element. + * + * @property {string} + */ + cancelText = TEXT_ANNOTATION_CANCEL; + + /** + * Text displayed in the Post button element. + * + * @property {string} + */ + postText = TEXT_ANNOTATION_POST; + + /** + * Placeholder text displayed in the text area element. + * + * @property {string} + */ + placeholderText = TEXT_ADD_COMMENT_PLACEHOLDER; + + /** + * Reference to the comment box element. Contains buttons and text area. + * + * @property {HTMLElement} + */ + el; + + /** + * Reference to the cancel button element in the comment box. + * + * @property {HTMLElement} + */ + cancelEl; + + /** + * Reference to the post button element in the comment box. + * + * @property {HTMLElement} + */ + postEl; + + /** + * Reference to the text area element in the comment box. + * + * @property {HTMLElement} + */ + textAreaEl; + + /** + * Reference to parent element that the comment box should be nested inside. + * + * @property {HTMLElement} + */ + parentEl; + + /* Events that the comment box can emit. */ + static CommentEvents = { + cancel: 'comment_cancel', + post: 'comment_post' + }; + + /** + * Creates an element for text entry, submission and cancellation. + * + * @param {Object} [config] - Object containing text values to be displayed to the user. + * config.cancel - Text displayed in the "Cancel" button + * config.post - Text displayed in the "Post" button + * config.placeholder - Placeholder text displayed in the text area + */ + constructor(parentEl, config = {}) { + super(); + + this.parentEl = parentEl; + + this.cancelText = config.cancel || this.cancelText; + this.postText = config.post || this.postText; + this.placeholderText = config.placeholder || this.placeholderText; + + // Explicit scope binding for event listeners + this.onCancel = this.onCancel.bind(this); + this.onPost = this.onPost.bind(this); + } + + /** + * Focus on the text box. + * + * @public + * @return {void} + */ + focus() { + this.textAreaEl.focus(); + } + + /** + * Clear out the text box. + * + * @public + * @return {void} + */ + clear() { + if (!this.el) { + return; + } + + this.textAreaEl.value = ''; + } + + /** + * Hide the element. + * + * @public + * @return {void} + */ + hide() { + if (!this.el) { + return; + } + + this.el.classList.add(CLASS_HIDDEN); + } + + /** + * Show the element. + * + * @public + * @return {void} + */ + show() { + if (!this.el) { + this.el = this.createCommentBox(); + this.parentEl.appendChild(this.el); + } + + this.el.classList.remove(CLASS_HIDDEN); + } + + /** + * Destructor + * + * @public + */ + destroy() { + if (!this.el) { + return; + } + + this.el.remove(); + this.parentEl = null; + this.el = null; + this.cancelEl.removeEventListener('click', this.onCancel); + this.postEl.removeEventListener('click', this.onPost); + } + + //-------------------------------------------------------------------------- + // Private + //-------------------------------------------------------------------------- + + /** + * Create HTML containing the UI for the comment box. + * + * @private + * @return {HTMLElement} HTML containing UI for the comment box. + */ + createHTML() { + const el = document.createElement('section'); + el.classList.add('bp-create-highlight-comment'); + el.innerHTML = ` + +
+ + +
`.trim(); + + return el; + } + + /** + * Clear the current text in the textarea element and notify listeners. + * + * @private + * @return {void} + */ + onCancel() { + this.clear(); + this.emit(CommentBox.CommentEvents.cancel); + } + + /** + * Notify listeners of submit event and then clear textarea element. + * + * @private + * @return {void} + */ + onPost() { + this.emit(CommentBox.CommentEvents.post, this.textAreaEl.value); + this.clear(); + } + + /** + * Create HTML for the comment box. Assigns references to elements, attach event listeners. + * ie) Post button, cancel button + * + * @private + * @return {HTMLElement} The HTML to append to this.parentElement + */ + createCommentBox() { + const el = this.createHTML(); + + // Reference HTML + this.textAreaEl = el.querySelector('.annotation-textarea'); + this.cancelEl = el.querySelector('.cancel-annotation-btn'); + this.postEl = el.querySelector('.post-annotation-btn'); + + // Add event listeners + this.cancelEl.addEventListener('click', this.onCancel); + this.postEl.addEventListener('click', this.onPost); + + return el; + } +} + +export default CommentBox; diff --git a/src/lib/annotations/MobileAnnotator.scss b/src/lib/annotations/MobileAnnotator.scss index 69dd68e01..f904abed8 100644 --- a/src/lib/annotations/MobileAnnotator.scss +++ b/src/lib/annotations/MobileAnnotator.scss @@ -61,7 +61,8 @@ $tablet: "(min-width: 768px)"; } } -.bp-mobile-annotation-dialog.bp-annotation-dialog { +.bp-mobile-annotation-dialog.bp-annotation-dialog, +.bp-mobile-annotation-dialog.bp-temp-annotation-dialog { .annotation-container { background-color: $white; border: 0; diff --git a/src/lib/annotations/doc/CreateHighlightDialog.js b/src/lib/annotations/doc/CreateHighlightDialog.js new file mode 100644 index 000000000..51f77e504 --- /dev/null +++ b/src/lib/annotations/doc/CreateHighlightDialog.js @@ -0,0 +1,338 @@ +import EventEmitter from 'events'; +import { ICON_HIGHLIGHT, ICON_HIGHLIGHT_COMMENT } from '../../icons/icons'; +import CommentBox from '../CommentBox'; +import { CLASS_HIDDEN } from '../../constants'; + +const CLASS_CREATE_DIALOG = 'bp-create-annotation-dialog'; +const TITLE_HIGHLIGHT_TOGGLE = __('annotation_highlight_toggle'); +const TITLE_HIGHLIGHT_COMMENT = __('annotation_highlight_comment'); +const CREATE_HIGHLIGHT_DIALOG_TEMPLATE = ` +
+
+
+ + + + +
+
`.trim(); + +class CreateHighlightDialog extends EventEmitter { + /** + * Container element for the dialog. + * + * @property {HTMLElement} + */ + el; + + /** + * The clickable element for creating basic highlights. + * + * @property {HTMLElement} + */ + highlightCreateEl; + + /** + * The clickable element got creating comment highlights. + * + * @property {HTMLElement} + */ + commentCreateEl; + + /** + * The parent container to nest the dialog element in. + * + * @property {HTMLElement} + */ + parentEl; + + /** + * The element containing the buttons that can creaet highlights. + * + * @property {HTMLElement} + */ + buttonsEl; + + /** + * The comment box instance. Contains area for text input and post/cancel buttons. + * + * @property {CommentBox} + */ + commentBox; + + /** + * Position, on the DOM, to align the dialog to the end of a highlight. + * + * @property {Object} + */ + position = { + x: 0, + y: 0 + }; + + /** + * Events emitted by this component. + */ + static CreateEvents = { + basic: 'highlight_basic_create', + comment: 'highlight_comment_edit', + commentPost: 'highlight_comment_post' + }; + + /** + * A dialog used to create basic and comment highlights. + * + * [constructor] + */ + constructor(parentEl) { + super(); + + this.parentEl = parentEl; + + // Explicit scope binding for event listeners + this.onHighlightClick = this.onHighlightClick.bind(this); + this.onCommentClick = this.onCommentClick.bind(this); + this.onCommentPost = this.onCommentPost.bind(this); + this.onCommentCancel = this.onCommentCancel.bind(this); + } + + /** + * Set the parent container to next this dialog in. + * + * @public + * @param {HTMLElement} newParentEl - The element that will contain this. + * @return {void} + */ + setParentEl(newParentEl) { + if (newParentEl === this.parentEl) { + return; + } + + this.parentEl = newParentEl; + } + + /** + * Set the coordinates to position the dialog at, and force an update. + * + * @public + * @param {number} x - The x coordinate to position the dialog at + * @param {number} y - The y coordinate to position the dialog at + * @return {void} + */ + setPosition(x, y) { + this.position.x = x; + this.position.y = y; + this.updatePosition(); + } + + /** + * Show the dialog. Adds to the parent container if it isn't already there. + * + * @public + * @param {HTMLElement} [newParentEl] - The new parent container to nest this in. + * @return {void} + */ + show(newParentEl) { + if (!this.el) { + this.el = this.createElement(); + } + + // Move to the correct parent element + if (newParentEl) { + this.setParentEl(newParentEl); + } + + // Add to parent if it hasn't been added already + if (!this.parentEl.querySelector(`.${CLASS_CREATE_DIALOG}`)) { + this.parentEl.appendChild(this.el); + } + + this.setButtonVisibility(true); + this.el.classList.remove(CLASS_HIDDEN); + } + + /** + * Hide the dialog, and clear out the comment box text entry. + * + * @return {void} + */ + hide() { + if (!this.el) { + return; + } + + this.el.classList.add(CLASS_HIDDEN); + this.commentBox.hide(); + this.commentBox.clear(); + } + + /** + * Destructor + * + * @public + */ + destroy() { + if (!this.el) { + return; + } + + this.hide(); + + // Stop interacting with this element from triggering outside actions + this.el.removeEventListener('click', this.stopPropagation); + this.el.removeEventListener('mouseup', this.stopPropagation); + this.el.removeEventListener('touchend', this.stopPropagation); + this.el.removeEventListener('dblclick', this.stopPropagation); + + // Event listeners + this.highlightCreateEl.removeEventListener('click', this.onHighlightClick); + this.commentCreateEl.removeEventListener('click', this.onCommentClick); + this.commentBox.removeListener(CommentBox.CommentEvents.post, this.onCommentPost); + this.commentBox.removeListener(CommentBox.CommentEvents.cancel, this.onCommentCancel); + + this.el.remove(); + this.el = null; + this.parentEl = null; + + this.commentBox.destroy(); + this.commentBox = null; + } + + //-------------------------------------------------------------------------- + // Private + //-------------------------------------------------------------------------- + + /** + * Update the position styling for the dialog so that the chevron points to + * the desired location. + * + * @return {void} + */ + updatePosition() { + // Plus 5 pixels for caret + this.el.style.top = `${this.position.x + 5}px`; + // Plus 1 pixel for caret + this.el.style.left = `${this.position.y - 1 - this.el.clientWidth / 2}px`; + } + + /** + * Fire an event notifying that the basic highlight button has been clicked. + * + * @return {void} + */ + onHighlightClick() { + this.emit(CreateHighlightDialog.CreateEvents.basic); + } + + /** + * 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. + * + * @return {void} + */ + onCommentClick() { + this.emit(CreateHighlightDialog.CreateEvents.comment); + + this.commentBox.show(); + this.commentBox.focus(); + this.setButtonVisibility(false); + this.updatePosition(); + } + + /** + * Fire an event notifying that the post button has been pressed. Clears + * out the comment box. + * + * @param {string} text - Text entered into the comment box + * @return {void} + */ + onCommentPost(text) { + this.emit(CreateHighlightDialog.CreateEvents.commentPost, text); + this.commentBox.clear(); + } + + /** + * The cancel button has been pressed. Close the comment box, and return to + * default state. + * + * @return {void} + */ + onCommentCancel() { + this.commentBox.hide(); + this.setButtonVisibility(true); + this.updatePosition(); + } + + /** + * Hide or show the basic and comment buttons, in the dialog. + * + * @param {boolean} visible - If true, shows the basic and comment buttons + */ + setButtonVisibility(visible) { + if (visible) { + this.buttonsEl.classList.remove(CLASS_HIDDEN); + } else { + this.buttonsEl.classList.add(CLASS_HIDDEN); + } + } + + /** + * Stop the dialog from propagating events to parent container. Pairs with + * giving focus to the text area in the comment box and clicking "Post". + * + * @param {Event} event - The DOM event coming from interacting with the element. + * @return {void} + */ + stopPropagation(event) { + event.stopPropagation(); + } + + /** + * Create the element containing highlight create and comment buttons, and comment box. + * + * @private + * @return {HTMLElement} The element containing Highlight creation UI + */ + createElement() { + const highlightDialogEl = document.createElement('div'); + highlightDialogEl.classList.add(CLASS_CREATE_DIALOG); + highlightDialogEl.innerHTML = CREATE_HIGHLIGHT_DIALOG_TEMPLATE; + + const containerEl = highlightDialogEl.querySelector('.bp-annotation-highlight-dialog'); + + // Reference HTML + this.highlightCreateEl = containerEl.querySelector('.bp-add-highlight-btn'); + this.commentCreateEl = containerEl.querySelector('.bp-highlight-comment-btn'); + this.buttonsEl = containerEl.querySelector('.bp-annotations-highlight-btns'); + + // Create comment box + this.commentBox = new CommentBox(containerEl); + + // 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 + this.highlightCreateEl.addEventListener('click', this.onHighlightClick); + this.commentCreateEl.addEventListener('click', this.onCommentClick); + this.commentBox.addListener(CommentBox.CommentEvents.post, this.onCommentPost); + this.commentBox.addListener(CommentBox.CommentEvents.cancel, this.onCommentCancel); + + // Hide comment box, by default + this.commentBox.hide(); + + return highlightDialogEl; + } +} + +export default CreateHighlightDialog; diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index b4c3d26ae..59206da9f 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -9,6 +9,7 @@ import autobind from 'autobind-decorator'; import Annotator from '../Annotator'; import DocHighlightThread from './DocHighlightThread'; import DocPointThread from './DocPointThread'; +import CreateHighlightDialog from './CreateHighlightDialog'; import * as annotatorUtil from '../annotatorUtil'; import * as constants from '../annotationConstants'; import * as docAnnotatorUtil from './docAnnotatorUtil'; @@ -73,6 +74,74 @@ function isThreadInHoverState(thread) { */ throttleTimer = 0; + /** + * UI used to create new highlight annotations. + * + * @property {CreateHighlightDialog} + */ + createHighlightDialog; + + /** + * For delaying creation of highlight quad points and dialog. Tracks the + * current selection event, made in a previous event. + * + * @property {Event} + */ + lastHighlightEvent; + + /** + * Creates and mananges basic highlight and comment highlight and point annotations + * on document files. + * + * [constructor] + * @inheritdoc + * @return {DocAnnotator} + */ + constructor(data) { + super(data); + + // Explicit scoping + this.highlightCurrentSelection = this.highlightCurrentSelection.bind(this); + this.createHighlightAnnotation = this.createHighlightAnnotation.bind(this); + this.createBasicHighlight = this.createBasicHighlight.bind(this); + + this.createHighlightDialog = new CreateHighlightDialog(); + this.createHighlightDialog.addListener(CreateHighlightDialog.CreateEvents.basic, this.createBasicHighlight); + + this.createHighlightDialog.addListener( + CreateHighlightDialog.CreateEvents.comment, + this.highlightCurrentSelection + ); + + this.createHighlightDialog.addListener( + CreateHighlightDialog.CreateEvents.commentPost, + this.createHighlightAnnotation + ); + } + + /** + * Destructor + * + * @public + */ + destroy() { + super.destroy(); + + this.createHighlightDialog.removeListener(CreateHighlightDialog.CreateEvents.basic, this.createBasicHighlight); + + this.createHighlightDialog.removeListener( + CreateHighlightDialog.CreateEvents.comment, + this.highlightCurrentSelection + ); + + this.createHighlightDialog.removeListener( + CreateHighlightDialog.CreateEvents.commentPost, + this.createHighlightAnnotation + ); + this.createHighlightDialog.destroy(); + this.createHighlightDialog = null; + } + //-------------------------------------------------------------------------- // Abstract Implementations //-------------------------------------------------------------------------- @@ -148,7 +217,7 @@ function isThreadInHoverState(thread) { location = { x, y, page, dimensions }; } else if (annotatorUtil.isHighlightAnnotation(annotationType)) { - if (!docAnnotatorUtil.isSelectionPresent()) { + if (!this.highlighter || !this.highlighter.highlights.length) { return location; } @@ -241,6 +310,52 @@ function isThreadInHoverState(thread) { return thread; } + /** + * Creates a basic highlight annotation. + * + * @private + * @return {void} + */ + createBasicHighlight() { + this.highlightCurrentSelection(); + this.createHighlightAnnotation(); + } + + /** + * Creates an highlight annotation thread, adds it to in-memory map, and returns it. + * + * @override + * @param {Annotation[]} annotations - Annotations in thread + * @param {Object} location - Location object + * @return {DocHighlightThread} Created doc highlight annotation thread + */ + createHighlightAnnotation(commentText) { + // Empty string will be passed in if no text submitted in comment + if (commentText === '' || !this.lastHighlightEvent) { + return null; + } + this.createHighlightDialog.hide(); + + const annotations = []; + const location = this.getLocationFromEvent(this.lastHighlightEvent, constants.ANNOTATION_TYPE_HIGHLIGHT); + const thread = this.createAnnotationThread(annotations, location, constants.ANNOTATION_TYPE_HIGHLIGHT); + this.lastHighlightEvent = null; + + if (!commentText) { + thread.dialog.drawAnnotation(); + } else { + thread.dialog.hasComments = true; + } + + thread.show(); + thread.dialog.postAnnotation(commentText); + + this.bindCustomListenersOnThread(thread); + this.renderAnnotationsOnPage(location.page); + + return thread; + } + /** * Renders annotations from memory for a specified page. * @@ -366,6 +481,22 @@ function isThreadInHoverState(thread) { // Private //-------------------------------------------------------------------------- + /** + * Highlight the current range of text that has been selected. + * + * @private + * @return {void} + */ + highlightCurrentSelection() { + if (!this.highlighter) { + return; + } + + this.highlighter.highlightSelection('rangy-highlight', { + containerElementId: this.annotatedElement.id + }); + } + /** * Gets threads on page * @@ -524,11 +655,15 @@ function isThreadInHoverState(thread) { * @param {Event} event - DOM event */ highlightMouseupHandler(event) { + if (this.highlighter) { + this.highlighter.removeAllHighlights(); + } + this.createHighlightDialog.hide(); // Creating highlights is disabled on mobile for now since the // event we would listen to, selectionchange, fires continuously and // is unreliable. If the mouse moved or we double clicked text, // we trigger the create handler instead of the click handler - if (!this.isMobile && (this.didMouseMove || event.type === 'dblclick')) { + if (this.didMouseMove || event.type === 'dblclick') { this.highlightCreateHandler(event); } else { this.highlightClickHandler(event); @@ -548,15 +683,19 @@ function isThreadInHoverState(thread) { highlightCreateHandler(event) { event.stopPropagation(); - // Determine if any highlight threads are pending and ignore the - // creation of any new highlights - if (docAnnotatorUtil.hasActiveDialog(this.annotatedElement)) { + const selection = window.getSelection(); + if (selection.rangeCount <= 0 || selection.isCollapsed) { return; } // Only filter through highlight threads on the current page // Reset active highlight threads before creating new highlight - const page = annotatorUtil.getPageInfo(event.target).page; + const { pageEl, page } = annotatorUtil.getPageInfo(event.target); + + if (!pageEl) { + return; + } + const activeThreads = this.getHighlightThreadsOnPage(page).filter( (thread) => constants.ACTIVE_STATES.indexOf(thread.state) > -1 ); @@ -564,21 +703,25 @@ function isThreadInHoverState(thread) { thread.reset(); }); - // Get annotation location from mouseup event, ignore if location is invalid - const location = this.getLocationFromEvent(event, constants.ANNOTATION_TYPE_HIGHLIGHT); - if (!location) { + const lastRange = selection.getRangeAt(selection.rangeCount - 1); + const rects = lastRange.getClientRects(); + + if (rects.length === 0) { return; } - // Create and show pending annotation thread - const thread = this.createAnnotationThread([], location, constants.ANNOTATION_TYPE_HIGHLIGHT); + const { right, bottom } = rects[rects.length - 1]; - if (thread) { - thread.show(); + const pageDimensions = pageEl.getBoundingClientRect(); + const pageLeft = pageDimensions.left; + const pageTop = pageDimensions.top + PAGE_PADDING_TOP; - // Bind events on thread - this.bindCustomListenersOnThread(thread); + this.createHighlightDialog.show(pageEl); + if (!this.isMobile) { + this.createHighlightDialog.setPosition(bottom - pageTop, right - pageLeft); } + + this.lastHighlightEvent = event; } /** @@ -687,8 +830,6 @@ function isThreadInHoverState(thread) { * @return {void} */ showHighlightsOnPage(page) { - // let time = new Date().getTime(); - // Clear context if needed const pageEl = this.annotatedElement.querySelector(`[data-page-number="${page}"]`); const annotationLayerEl = pageEl.querySelector('.bp-annotation-layer'); @@ -700,8 +841,6 @@ function isThreadInHoverState(thread) { this.getHighlightThreadsOnPage(page).forEach((thread) => { thread.show(); }); - - // console.log(`Drawing annotations for page ${page} took ${new Date().getTime() - time}ms`); } /** diff --git a/src/lib/annotations/doc/DocHighlightAnnotator.js b/src/lib/annotations/doc/DocHighlightAnnotator.js new file mode 100644 index 000000000..79bbfef7b --- /dev/null +++ b/src/lib/annotations/doc/DocHighlightAnnotator.js @@ -0,0 +1,118 @@ +import rangy from 'rangy'; +/* eslint-disable no-unused-vars */ +// Workaround for rangy npm issue: https://github.com/timdown/rangy/issues/342 +import rangyClassApplier from 'rangy/lib/rangy-classapplier'; +import rangyHighlight from 'rangy/lib/rangy-highlighter'; +import rangySaveRestore from 'rangy/lib/rangy-selectionsaverestore'; +/* eslint-enable no-unused-vars */ +import * as docAnnotatorUtil from './docAnnotatorUtil'; +import * as annotatorUtil from '../annotatorUtil'; +import { ACTIVE_STATES, ANNOTATION_TYPE_HIGHLIGHT } from '../annotationConstants'; +import { PAGE_PADDING_BOTTOM, PAGE_PADDING_TOP } from './documentConstants'; + +export default class DocHighlightAnnotator { + highlighter; + + constructor() { + // Init rangy and rangy highlight + this.highlighter = rangy.createHighlighter(); + this.highlighter.addClassApplier( + rangy.createClassApplier('rangy-highlight', { + ignoreWhiteSpace: true, + tagNames: ['span', 'a'] + }) + ); + } + + /** + * Returns an annotation location on a document from the DOM event or null + * if no correct annotation location can be inferred from the event. The PDF + * quad points as defined by the PDF spec and page the highlight is on. + * + * @override + * @param {Event} event - DOM event + * @param {number} zoomScale - The scale of the zoomed document + * @return {Object|null} Location object + */ + getLocationFromEvent(event, zoomScale) { + if (!docAnnotatorUtil.isSelectionPresent()) { + return null; + } + + // Get correct page + let { pageEl, page } = annotatorUtil.getPageInfo(event.target); + if (page === -1) { + // The ( .. ) around assignment is required syntax + ({ pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode)); + } + + // We save the dimensions of the annotated element scaled to 100% + // so we can compare to the annotated element during render time + // and scale if needed (in case the representation changes size) + const pageDimensions = pageEl.getBoundingClientRect(); + const pageWidth = pageDimensions.width; + const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM; + const dimensions = { + x: pageWidth / zoomScale, + y: pageHeight / zoomScale + }; + + return { page, dimensions }; + } + + generateQuadPoints(event, zoomScale) { + // Get correct page + let { pageEl, page } = annotatorUtil.getPageInfo(event.target); + if (page === -1) { + // The ( .. ) around assignment is required syntax + ({ pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode)); + } + + // Use Rangy to save the current selection because using the + // highlight module can mess with the selection. We restore this + // selection after we clean up the highlight + const savedSelection = rangy.saveSelection(); + + // Use highlight module to calculate quad points + const { highlight, highlightEls } = docAnnotatorUtil.getHighlightAndHighlightEls(this.highlighter, pageEl); + + // Do not create highlight annotation if no highlights are detected + if (highlightEls.length === 0) { + return null; + } + + const quadPoints = []; + highlightEls.forEach((element) => { + quadPoints.push(docAnnotatorUtil.getQuadPoints(element, pageEl, zoomScale)); + }); + + // Remove rangy highlight and restore selection + this.removeRangyHighlight(highlight); + rangy.restoreSelection(savedSelection); + + return quadPoints; + } + + /** + * Helper to remove a Rangy highlight by deleting the highlight in the + * internal highlighter list that has a matching ID. We can't directly use + * the highlighter's removeHighlights since the highlight could possibly + * not be a true Rangy highlight object. + * + * @private + * @param {Object} highlight - Highlight to delete. + * @return {void} + */ + removeRangyHighlight(highlight) { + const highlights = this.highlighter.highlights; + if (!Array.isArray(highlights)) { + return; + } + + const matchingHighlights = highlights.filter((internalHighlight) => { + return internalHighlight.id === highlight.id; + }); + + this.highlighter.removeHighlights(matchingHighlights); + } +} diff --git a/src/lib/annotations/doc/DocHighlightDialog.js b/src/lib/annotations/doc/DocHighlightDialog.js index 4232859ef..57f891156 100644 --- a/src/lib/annotations/doc/DocHighlightDialog.js +++ b/src/lib/annotations/doc/DocHighlightDialog.js @@ -42,6 +42,17 @@ const PAGE_PADDING_TOP = 15; super.addAnnotation(annotation); } + /** + * Emit the message to create a highlight and render it. + * + * @public + * @return {void} + */ + drawAnnotation() { + this.emit('annotationdraw'); + this.toggleHighlight(); + } + //-------------------------------------------------------------------------- // Abstract Implementations //-------------------------------------------------------------------------- @@ -306,8 +317,7 @@ const PAGE_PADDING_TOP = 15; switch (dataType) { // Clicking 'Highlight' button to create or remove a highlight case 'highlight-btn': - this.emit('annotationdraw'); - this.toggleHighlight(); + this.drawAnnotation(); break; // Clicking 'Highlight' button to create a highlight case 'add-highlight-comment-btn': @@ -455,7 +465,7 @@ const PAGE_PADDING_TOP = 15; const highlightDialogEl = document.createElement('div'); highlightDialogEl.innerHTML = ` - + `.trim(); - return el; + return containerEl; } /** @@ -225,18 +223,18 @@ class CommentBox extends EventEmitter { * @return {HTMLElement} The HTML to append to this.parentElement */ createCommentBox() { - const el = this.createHTML(); + const containerEl = this.createHTML(); // Reference HTML - this.textAreaEl = el.querySelector('.annotation-textarea'); - this.cancelEl = el.querySelector('.cancel-annotation-btn'); - this.postEl = el.querySelector('.post-annotation-btn'); + this.textAreaEl = containerEl.querySelector('.annotation-textarea'); + this.cancelEl = containerEl.querySelector('.cancel-annotation-btn'); + this.postEl = containerEl.querySelector('.post-annotation-btn'); // Add event listeners this.cancelEl.addEventListener('click', this.onCancel); this.postEl.addEventListener('click', this.onPost); - return el; + return containerEl; } } diff --git a/src/lib/annotations/__tests__/CommentBox-test.js b/src/lib/annotations/__tests__/CommentBox-test.js index eff7e6009..59419f299 100644 --- a/src/lib/annotations/__tests__/CommentBox-test.js +++ b/src/lib/annotations/__tests__/CommentBox-test.js @@ -59,8 +59,8 @@ describe('lib/annotations/CommentBox', () => { }); it('should do nothing if the comment box HTML doesn\'t exist', () => { - commentBox.el.remove(); - commentBox.el = null; + commentBox.containerEl.remove(); + commentBox.containerEl = null; const focus = sandbox.stub(commentBox.textAreaEl, 'focus'); @@ -83,8 +83,8 @@ describe('lib/annotations/CommentBox', () => { }); it('should do nothing if the comment box HTML doesn\'t exist', () => { - commentBox.el.remove(); - commentBox.el = null; + commentBox.containerEl.remove(); + commentBox.containerEl = null; const text = 'yay'; commentBox.textAreaEl.value = text; @@ -107,8 +107,8 @@ describe('lib/annotations/CommentBox', () => { }); it('should do nothing if the comment box HTML doesn\'t exist', () => { - const addClass = sandbox.stub(commentBox.el.classList, 'add'); - commentBox.el = null; + const addClass = sandbox.stub(commentBox.containerEl.classList, 'add'); + commentBox.containerEl = null; commentBox.hide(); expect(addClass).to.not.be.called; @@ -116,31 +116,31 @@ describe('lib/annotations/CommentBox', () => { it('should add the hidden class to the comment box element', () => { commentBox.hide(); - expect(commentBox.el.classList.contains(CLASS_HIDDEN)).to.be.true; + expect(commentBox.containerEl.classList.contains(CLASS_HIDDEN)).to.be.true; }); }); describe('show()', () => { it('should invoke createComment box, if UI has not been created', () => { - const el = document.createElement('div'); - const create = sandbox.stub(commentBox, 'createCommentBox').returns(el); + const containerEl = document.createElement('div'); + const create = sandbox.stub(commentBox, 'createCommentBox').returns(containerEl); commentBox.show(); expect(create).to.be.called; // Nullify to prevent fail during destroy - commentBox.el = null; + commentBox.containerEl = null; }); it('should add the container element to the parent, if the UI has not been created', () => { const append = sandbox.stub(parentEl, 'appendChild'); commentBox.show(); - expect(append).to.be.calledWith(commentBox.el); + expect(append).to.be.calledWith(commentBox.containerEl); }); it('should remove the hidden class from the container', () => { commentBox.show(); - expect(commentBox.el.classList.contains(CLASS_HIDDEN)).to.be.false; + expect(commentBox.containerEl.classList.contains(CLASS_HIDDEN)).to.be.false; }); }); @@ -154,7 +154,7 @@ describe('lib/annotations/CommentBox', () => { it('should remove the UI container from the parent element', () => { commentBox.show(); - const remove = sandbox.stub(commentBox.el, 'remove'); + const remove = sandbox.stub(commentBox.containerEl, 'remove'); commentBox.destroy(); expect(remove).to.be.called; }); diff --git a/src/lib/annotations/annotationConstants.js b/src/lib/annotations/annotationConstants.js index 68dde7fb5..e71e39ca3 100644 --- a/src/lib/annotations/annotationConstants.js +++ b/src/lib/annotations/annotationConstants.js @@ -43,3 +43,6 @@ export const HOVER_STATES = [ANNOTATION_STATE_HOVER, ANNOTATION_STATE_ACTIVE_HOV export const HIGHLIGHT_NORMAL_FILL_STYLE = 'rgba(254, 217, 78, 0.5)'; export const HIGHLIGHT_ACTIVE_FILL_STYLE = 'rgba(255, 201, 0, 0.5)'; export const HIGHLIGHT_ERASE_FILL_STYLE = 'rgba(255, 245, 132, 1)'; + +export const PAGE_PADDING_TOP = 15; +export const PAGE_PADDING_BOTTOM = 15; diff --git a/src/lib/annotations/doc/CreateHighlightDialog.js b/src/lib/annotations/doc/CreateHighlightDialog.js index 63faf9752..d9422b90e 100644 --- a/src/lib/annotations/doc/CreateHighlightDialog.js +++ b/src/lib/annotations/doc/CreateHighlightDialog.js @@ -25,16 +25,25 @@ const CREATE_HIGHLIGHT_DIALOG_TEMPLATE = ` `.trim(); +/** + * Events emitted by this component. + */ +export const CreateEvents = { + plain: 'plain_highlight_create', + comment: 'comment_highlight_edit', + commentPost: 'comment_highlight_post' +}; + class CreateHighlightDialog extends EventEmitter { /** * Container element for the dialog. * * @property {HTMLElement} */ - el; + containerEl; /** - * The clickable element for creating basic highlights. + * The clickable element for creating plain highlights. * * @property {HTMLElement} */ @@ -79,16 +88,7 @@ class CreateHighlightDialog extends EventEmitter { }; /** - * Events emitted by this component. - */ - static CreateEvents = { - basic: 'highlight_basic_create', - comment: 'highlight_comment_edit', - commentPost: 'highlight_comment_post' - }; - - /** - * A dialog used to create basic and comment highlights. + * A dialog used to create plain and comment highlights. * * [constructor] */ @@ -137,8 +137,8 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ show(newParentEl) { - if (!this.el) { - this.el = this.createElement(); + if (!this.containerEl) { + this.containerEl = this.createElement(); } // Move to the correct parent element @@ -148,11 +148,11 @@ class CreateHighlightDialog extends EventEmitter { // Add to parent if it hasn't been added already if (!this.parentEl.querySelector(`.${CLASS_CREATE_DIALOG}`)) { - this.parentEl.appendChild(this.el); + this.parentEl.appendChild(this.containerEl); } this.setButtonVisibility(true); - this.el.classList.remove(CLASS_HIDDEN); + this.containerEl.classList.remove(CLASS_HIDDEN); } /** @@ -161,11 +161,11 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ hide() { - if (!this.el) { + if (!this.containerEl) { return; } - this.el.classList.add(CLASS_HIDDEN); + this.containerEl.classList.add(CLASS_HIDDEN); this.commentBox.hide(); this.commentBox.clear(); } @@ -176,17 +176,17 @@ class CreateHighlightDialog extends EventEmitter { * @public */ destroy() { - if (!this.el) { + if (!this.containerEl) { return; } this.hide(); // Stop interacting with this element from triggering outside actions - this.el.removeEventListener('click', this.stopPropagation); - this.el.removeEventListener('mouseup', this.stopPropagation); - this.el.removeEventListener('touchend', this.stopPropagation); - this.el.removeEventListener('dblclick', this.stopPropagation); + 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 this.highlightCreateEl.removeEventListener('click', this.onHighlightClick); @@ -194,8 +194,8 @@ class CreateHighlightDialog extends EventEmitter { this.commentBox.removeListener(CommentBox.CommentEvents.post, this.onCommentPost); this.commentBox.removeListener(CommentBox.CommentEvents.cancel, this.onCommentCancel); - this.el.remove(); - this.el = null; + this.containerEl.remove(); + this.containerEl = null; this.parentEl = null; this.commentBox.destroy(); @@ -214,18 +214,18 @@ class CreateHighlightDialog extends EventEmitter { */ updatePosition() { // Plus 1 pixel for caret - this.el.style.left = `${this.position.x - 1 - this.el.clientWidth / 2}px`; + this.containerEl.style.left = `${this.position.x - 1 - this.containerEl.clientWidth / 2}px`; // Plus 5 pixels for caret - this.el.style.top = `${this.position.y + 5}px`; + this.containerEl.style.top = `${this.position.y + 5}px`; } /** - * Fire an event notifying that the basic highlight button has been clicked. + * Fire an event notifying that the plain highlight button has been clicked. * * @return {void} */ onHighlightClick() { - this.emit(CreateHighlightDialog.CreateEvents.basic); + this.emit(CreateEvents.plain); } /** @@ -235,7 +235,7 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ onCommentClick() { - this.emit(CreateHighlightDialog.CreateEvents.comment); + this.emit(CreateEvents.comment); this.commentBox.show(); this.commentBox.focus(); @@ -251,7 +251,7 @@ class CreateHighlightDialog extends EventEmitter { * @return {void} */ onCommentPost(text) { - this.emit(CreateHighlightDialog.CreateEvents.commentPost, text); + this.emit(CreateEvents.commentPost, text); this.commentBox.clear(); } @@ -268,9 +268,9 @@ class CreateHighlightDialog extends EventEmitter { } /** - * Hide or show the basic and comment buttons, in the dialog. + * Hide or show the plain and comment buttons, in the dialog. * - * @param {boolean} visible - If true, shows the basic and comment buttons + * @param {boolean} visible - If true, shows the plain and comment buttons */ setButtonVisibility(visible) { if (visible) { diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index b17be4237..dd8f432b8 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -9,14 +9,12 @@ import autobind from 'autobind-decorator'; import Annotator from '../Annotator'; import DocHighlightThread from './DocHighlightThread'; import DocPointThread from './DocPointThread'; -import CreateHighlightDialog from './CreateHighlightDialog'; +import CreateHighlightDialog, { CreateEvents } from './CreateHighlightDialog'; import * as annotatorUtil from '../annotatorUtil'; import * as constants from '../annotationConstants'; import * as docAnnotatorUtil from './docAnnotatorUtil'; const MOUSEMOVE_THROTTLE_MS = 50; -const PAGE_PADDING_BOTTOM = 15; -const PAGE_PADDING_TOP = 15; const HOVER_TIMEOUT_MS = 75; const MOUSE_MOVE_MIN_DISTANCE = 5; @@ -90,7 +88,7 @@ function isThreadInHoverState(thread) { lastHighlightEvent; /** - * Creates and mananges basic highlight and comment highlight and point annotations + * Creates and mananges plain highlight and comment highlight and point annotations * on document files. * * [constructor] @@ -103,20 +101,14 @@ function isThreadInHoverState(thread) { // Explicit scoping this.highlightCurrentSelection = this.highlightCurrentSelection.bind(this); this.createHighlightAnnotation = this.createHighlightAnnotation.bind(this); - this.createBasicHighlight = this.createBasicHighlight.bind(this); + this.createPlainHighlight = this.createPlainHighlight.bind(this); this.createHighlightDialog = new CreateHighlightDialog(); - this.createHighlightDialog.addListener(CreateHighlightDialog.CreateEvents.basic, this.createBasicHighlight); + this.createHighlightDialog.addListener(CreateEvents.plain, this.createPlainHighlight); - this.createHighlightDialog.addListener( - CreateHighlightDialog.CreateEvents.comment, - this.highlightCurrentSelection - ); + this.createHighlightDialog.addListener(CreateEvents.comment, this.highlightCurrentSelection); - this.createHighlightDialog.addListener( - CreateHighlightDialog.CreateEvents.commentPost, - this.createHighlightAnnotation - ); + this.createHighlightDialog.addListener(CreateEvents.commentPost, this.createHighlightAnnotation); } /** @@ -127,17 +119,11 @@ function isThreadInHoverState(thread) { destroy() { super.destroy(); - this.createHighlightDialog.removeListener(CreateHighlightDialog.CreateEvents.basic, this.createBasicHighlight); + this.createHighlightDialog.removeListener(CreateEvents.plain, this.createPlainHighlight); - this.createHighlightDialog.removeListener( - CreateHighlightDialog.CreateEvents.comment, - this.highlightCurrentSelection - ); + this.createHighlightDialog.removeListener(CreateEvents.comment, this.highlightCurrentSelection); - this.createHighlightDialog.removeListener( - CreateHighlightDialog.CreateEvents.commentPost, - this.createHighlightAnnotation - ); + this.createHighlightDialog.removeListener(CreateEvents.commentPost, this.createHighlightAnnotation); this.createHighlightDialog.destroy(); this.createHighlightDialog = null; } @@ -195,10 +181,10 @@ function isThreadInHoverState(thread) { // Store coordinates at 100% scale in PDF space in PDF units const pageDimensions = pageEl.getBoundingClientRect(); const pageWidth = pageDimensions.width; - const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM; + const pageHeight = pageDimensions.height - constants.PAGE_PADDING_TOP - constants.PAGE_PADDING_BOTTOM; const browserCoordinates = [ event.clientX - pageDimensions.left, - event.clientY - pageDimensions.top - PAGE_PADDING_TOP + event.clientY - pageDimensions.top - constants.PAGE_PADDING_TOP ]; const pdfCoordinates = docAnnotatorUtil.convertDOMSpaceToPDFSpace( browserCoordinates, @@ -255,7 +241,7 @@ function isThreadInHoverState(thread) { // and scale if needed (in case the representation changes size) const pageDimensions = pageEl.getBoundingClientRect(); const pageWidth = pageDimensions.width; - const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM; + const pageHeight = pageDimensions.height - constants.PAGE_PADDING_TOP - constants.PAGE_PADDING_BOTTOM; const dimensions = { x: pageWidth / zoomScale, y: pageHeight / zoomScale @@ -311,12 +297,12 @@ function isThreadInHoverState(thread) { } /** - * Creates a basic highlight annotation. + * Creates a plain highlight annotation. * * @private * @return {void} */ - createBasicHighlight() { + createPlainHighlight() { this.highlightCurrentSelection(); this.createHighlightAnnotation(); } @@ -337,7 +323,6 @@ function isThreadInHoverState(thread) { this.createHighlightDialog.hide(); const location = this.getLocationFromEvent(this.lastHighlightEvent, constants.ANNOTATION_TYPE_HIGHLIGHT); - console.error(location); if (!location) { return null; } @@ -719,7 +704,7 @@ function isThreadInHoverState(thread) { const pageDimensions = pageEl.getBoundingClientRect(); const pageLeft = pageDimensions.left; - const pageTop = pageDimensions.top + PAGE_PADDING_TOP; + const pageTop = pageDimensions.top + constants.PAGE_PADDING_TOP; this.createHighlightDialog.show(pageEl); if (!this.isMobile) { diff --git a/src/lib/annotations/doc/DocPointAnnotator.js b/src/lib/annotations/doc/DocPointAnnotator.js index 9ad31c285..2cc8a4b58 100644 --- a/src/lib/annotations/doc/DocPointAnnotator.js +++ b/src/lib/annotations/doc/DocPointAnnotator.js @@ -1,6 +1,6 @@ import * as docAnnotatorUtil from './docAnnotatorUtil'; import * as annotatorUtil from '../annotatorUtil'; -import { PAGE_PADDING_BOTTOM, PAGE_PADDING_TOP } from './documentConstants'; +import { PAGE_PADDING_BOTTOM, PAGE_PADDING_TOP } from '../annotationConstants'; export default class PointHighlightAnnotator { /** diff --git a/src/lib/annotations/doc/__tests__/CreateHighlightDialog-test.js b/src/lib/annotations/doc/__tests__/CreateHighlightDialog-test.js index a46e9cd86..db87172e9 100644 --- a/src/lib/annotations/doc/__tests__/CreateHighlightDialog-test.js +++ b/src/lib/annotations/doc/__tests__/CreateHighlightDialog-test.js @@ -1,5 +1,5 @@ /* eslint-disable no-unused-expressions */ -import CreateHighlightDialog from '../CreateHighlightDialog'; +import CreateHighlightDialog, { CreateEvents } from '../CreateHighlightDialog'; import { CLASS_HIDDEN } from '../../../constants'; import CommentBox from '../../CommentBox'; @@ -67,7 +67,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { it('should move the UI element to the parent element if it does not already contain it', () => { const append = sandbox.stub(parentEl, 'appendChild'); dialog.show(); - expect(append).to.be.calledWith(dialog.el); + expect(append).to.be.calledWith(dialog.containerEl); }); it('should invoke setButtonVisibility() to show the highlight buttons', () => { @@ -78,7 +78,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { it('should remove the hidden class from the UI element', () => { dialog.show(); - expect(dialog.el.classList.contains(CLASS_HIDDEN)).to.be.false; + expect(dialog.containerEl.classList.contains(CLASS_HIDDEN)).to.be.false; }); }); @@ -87,7 +87,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { dialog.show(); }); it('should do nothing if there is no UI element', () => { - dialog.el = null; + dialog.containerEl = null; const hideComment = sandbox.stub(dialog.commentBox, 'hide'); dialog.hide(); expect(hideComment).to.not.be.called; @@ -95,7 +95,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { it('should add the hidden class to the ui element', () => { dialog.hide(); - expect(dialog.el.classList.contains(CLASS_HIDDEN)).to.be.true; + expect(dialog.containerEl.classList.contains(CLASS_HIDDEN)).to.be.true; }); it('should hide the comment box', () => { @@ -117,14 +117,14 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { }); it('should do nothing if no ui has been created', () => { - dialog.el = null; + dialog.containerEl = null; const hide = sandbox.stub(dialog, 'hide'); dialog.destroy(); expect(hide).to.not.be.called; }); it('should remove events that stopPropagation() from occurring outside the dialog', () => { - const remove = sandbox.stub(dialog.el, 'removeEventListener'); + const remove = sandbox.stub(dialog.containerEl, 'removeEventListener'); dialog.destroy(); expect(remove).to.be.calledWith('click'); expect(remove).to.be.calledWith('mouseup'); @@ -157,7 +157,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { }); it('should remove the ui element from the dom', () => { - const remove = sandbox.stub(dialog.el, 'remove'); + const remove = sandbox.stub(dialog.containerEl, 'remove'); dialog.destroy(); expect(remove).to.be.called; }); @@ -178,23 +178,23 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { const y = 50; dialog.position.y = y; dialog.updatePosition(); - expect(dialog.el.style.top).to.equal(`${y + 5}px`); + expect(dialog.containerEl.style.top).to.equal(`${y + 5}px`); }); it('should update the left of the ui element, to center it', () => { - const width = dialog.el.clientWidth; + const width = dialog.containerEl.clientWidth; const x = 50; dialog.position.x = x; dialog.updatePosition(); - expect(dialog.el.style.left).to.equal(`${x - 1 - width / 2}px`); + expect(dialog.containerEl.style.left).to.equal(`${x - 1 - width / 2}px`); }); }); describe('onHighlightClick()', () => { - it('should invoke the "basic" highlight event', () => { + it('should invoke the "plain" highlight event', () => { const emit = sandbox.stub(dialog, 'emit'); dialog.onHighlightClick(); - expect(emit).to.be.calledWith(CreateHighlightDialog.CreateEvents.basic); + expect(emit).to.be.calledWith(CreateEvents.plain); }); }); @@ -206,7 +206,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { it('should invoke the "comment" highlight event', () => { const emit = sandbox.stub(dialog, 'emit'); dialog.onCommentClick(); - expect(emit).to.be.calledWith(CreateHighlightDialog.CreateEvents.comment); + expect(emit).to.be.calledWith(CreateEvents.comment); }); it('should show the comment box', () => { @@ -243,7 +243,7 @@ describe('lib/annotations/doc/CreateHighlightDialog', () => { const emit = sandbox.stub(dialog, 'emit'); const text = 'some text'; dialog.onCommentPost(text); - expect(emit).to.be.calledWith(CreateHighlightDialog.CreateEvents.commentPost, text); + expect(emit).to.be.calledWith(CreateEvents.commentPost, text); }); it('should clear the comment box', () => { diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index 4eda57665..d089f4b5d 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -274,11 +274,11 @@ describe('lib/annotations/doc/DocAnnotator', () => { }); }); - describe('createBasicHighlight()', () => { + describe('createPlainHighlight()', () => { beforeEach(() => { sandbox.stub(annotator, 'highlightCurrentSelection'); sandbox.stub(annotator, 'createHighlightAnnotation'); - annotator.createBasicHighlight(); + annotator.createPlainHighlight(); }); it('should invoke highlightCurrentSelection()', () => { diff --git a/src/lib/annotations/doc/docAnnotatorUtil.js b/src/lib/annotations/doc/docAnnotatorUtil.js index c5433c29c..3541eb38c 100644 --- a/src/lib/annotations/doc/docAnnotatorUtil.js +++ b/src/lib/annotations/doc/docAnnotatorUtil.js @@ -1,8 +1,7 @@ import * as annotatorUtil from '../annotatorUtil'; +import { PAGE_PADDING_TOP, PAGE_PADDING_BOTTOM } from '../annotationConstants'; const PREVIEW_PRESENTATION_CLASS = 'bp-doc-presentation'; -const PAGE_PADDING_BOTTOM = 15; -const PAGE_PADDING_TOP = 15; const HEIGHT_PADDING = 30; // PDF unit = 1/72 inch, CSS pixel = 1/92 inch const PDF_UNIT_TO_CSS_PIXEL = 4 / 3; @@ -114,17 +113,6 @@ export function isPointInPolyOpt(poly, x, y) { */ /* istanbul ignore next */ export function getHighlightAndHighlightEls(highlighter, pageEl) { - // We use Rangy to turn the selection into a highlight, which creates - // spans around the selection that we can then turn into quadpoints - // const highlight = highlighter.highlightSelection('rangy-highlight', { - // containerElementId: pageEl.id - // })[0]; - - // // Only grab highlights on the text layer - // const textLayer = pageEl.querySelector('.textLayer'); - // const highlightEls = [].slice.call(textLayer.querySelectorAll('.rangy-highlight'), 0).filter((element) => { - // return element.tagName && element.tagName === 'SPAN' && element.textContent.trim() !== ''; - // }); const highlight = highlighter.highlights[0]; // // Only grab highlights on the text layer const textLayer = pageEl.querySelector('.textLayer'); diff --git a/src/lib/annotations/doc/documentConstants.js b/src/lib/annotations/doc/documentConstants.js deleted file mode 100644 index e708b1c17..000000000 --- a/src/lib/annotations/doc/documentConstants.js +++ /dev/null @@ -1,2 +0,0 @@ -export const PAGE_PADDING_TOP = 15; -export const PAGE_PADDING_BOTTOM = 15; From fe74d48235d465945afd33c4e16381e5ee289404 Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Tue, 27 Jun 2017 15:25:42 -0700 Subject: [PATCH 5/9] Chore: More PR feedback --- src/lib/annotations/CommentBox.js | 7 ++++--- src/lib/annotations/doc/CreateHighlightDialog.js | 10 +++++----- src/lib/annotations/doc/DocAnnotator.js | 14 +++++++++----- .../annotations/doc/__tests__/DocAnnotator-test.js | 10 ++++++++++ 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/lib/annotations/CommentBox.js b/src/lib/annotations/CommentBox.js index 6ee11e290..6d3deb5fc 100644 --- a/src/lib/annotations/CommentBox.js +++ b/src/lib/annotations/CommentBox.js @@ -1,5 +1,6 @@ import EventEmitter from 'events'; -import { CLASS_HIDDEN, CLASS_ACTIVE } from '../constants'; +import { CLASS_ACTIVE } from '../constants'; +import { hideElement, showElement } from './annotatorUtil'; // Display Text const TEXT_ANNOTATION_CANCEL = __('annotation_cancel'); @@ -130,7 +131,7 @@ class CommentBox extends EventEmitter { return; } - this.containerEl.classList.add(CLASS_HIDDEN); + hideElement(this.containerEl); } /** @@ -145,7 +146,7 @@ class CommentBox extends EventEmitter { this.parentEl.appendChild(this.containerEl); } - this.containerEl.classList.remove(CLASS_HIDDEN); + showElement(this.containerEl); } /** diff --git a/src/lib/annotations/doc/CreateHighlightDialog.js b/src/lib/annotations/doc/CreateHighlightDialog.js index d9422b90e..3b695b797 100644 --- a/src/lib/annotations/doc/CreateHighlightDialog.js +++ b/src/lib/annotations/doc/CreateHighlightDialog.js @@ -1,7 +1,7 @@ import EventEmitter from 'events'; import { ICON_HIGHLIGHT, ICON_HIGHLIGHT_COMMENT } from '../../icons/icons'; import CommentBox from '../CommentBox'; -import { CLASS_HIDDEN } from '../../constants'; +import { hideElement, showElement } from '../annotatorUtil'; const CLASS_CREATE_DIALOG = 'bp-create-annotation-dialog'; const TITLE_HIGHLIGHT_TOGGLE = __('annotation_highlight_toggle'); @@ -152,7 +152,7 @@ class CreateHighlightDialog extends EventEmitter { } this.setButtonVisibility(true); - this.containerEl.classList.remove(CLASS_HIDDEN); + showElement(this.containerEl); } /** @@ -165,7 +165,7 @@ class CreateHighlightDialog extends EventEmitter { return; } - this.containerEl.classList.add(CLASS_HIDDEN); + hideElement(this.containerEl); this.commentBox.hide(); this.commentBox.clear(); } @@ -274,9 +274,9 @@ class CreateHighlightDialog extends EventEmitter { */ setButtonVisibility(visible) { if (visible) { - this.buttonsEl.classList.remove(CLASS_HIDDEN); + showElement(this.buttonsEl); } else { - this.buttonsEl.classList.add(CLASS_HIDDEN); + hideElement(this.buttonsEl); } } diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index dd8f432b8..1ac5c29f0 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -100,7 +100,7 @@ function isThreadInHoverState(thread) { // Explicit scoping this.highlightCurrentSelection = this.highlightCurrentSelection.bind(this); - this.createHighlightAnnotation = this.createHighlightAnnotation.bind(this); + this.createHighlightThread = this.createHighlightThread.bind(this); this.createPlainHighlight = this.createPlainHighlight.bind(this); this.createHighlightDialog = new CreateHighlightDialog(); @@ -108,7 +108,7 @@ function isThreadInHoverState(thread) { this.createHighlightDialog.addListener(CreateEvents.comment, this.highlightCurrentSelection); - this.createHighlightDialog.addListener(CreateEvents.commentPost, this.createHighlightAnnotation); + this.createHighlightDialog.addListener(CreateEvents.commentPost, this.createHighlightThread); } /** @@ -123,7 +123,7 @@ function isThreadInHoverState(thread) { this.createHighlightDialog.removeListener(CreateEvents.comment, this.highlightCurrentSelection); - this.createHighlightDialog.removeListener(CreateEvents.commentPost, this.createHighlightAnnotation); + this.createHighlightDialog.removeListener(CreateEvents.commentPost, this.createHighlightThread); this.createHighlightDialog.destroy(); this.createHighlightDialog = null; } @@ -304,7 +304,7 @@ function isThreadInHoverState(thread) { */ createPlainHighlight() { this.highlightCurrentSelection(); - this.createHighlightAnnotation(); + this.createHighlightThread(); } /** @@ -315,7 +315,7 @@ function isThreadInHoverState(thread) { * being the text as the first comment in the thread. * @return {DocHighlightThread} Created doc highlight annotation thread */ - createHighlightAnnotation(commentText) { + createHighlightThread(commentText) { // Empty string will be passed in if no text submitted in comment if (commentText === '' || !this.lastHighlightEvent) { return null; @@ -331,6 +331,10 @@ function isThreadInHoverState(thread) { const thread = this.createAnnotationThread(annotations, location, constants.ANNOTATION_TYPE_HIGHLIGHT); this.lastHighlightEvent = null; + if (!thread) { + return null; + } + if (!commentText) { thread.dialog.drawAnnotation(); } else { diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index d089f4b5d..d8a8f5b25 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -344,6 +344,16 @@ describe('lib/annotations/doc/DocAnnotator', () => { expect(stubs.createAnnotationThread).to.be.calledWith([], location, constants.ANNOTATION_TYPE_HIGHLIGHT); }); + it('should bail out of making an annotation if thread is null', () => { + annotator.lastHighlightEvent = {}; + const location = { page: 1 }; + stubs.getLocationFromEvent.returns(location); + stubs.createAnnotationThread.returns(null); + + annotator.createHighlightAnnotation('some text'); + expect(stubs.bindCustomListenersOnThread).to.not.be.called; + }); + it('should render the annotation thread dialog if it is a basic annotation type', () => { annotator.lastHighlightEvent = {}; const location = { page: 1 }; From 6d75f03c830a5d3a04d3e535ac3bd91a33a9b3db Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Tue, 27 Jun 2017 15:49:56 -0700 Subject: [PATCH 6/9] Chore: Test updates --- .../doc/__tests__/DocAnnotator-test.js | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index d8a8f5b25..0a6b52504 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -277,7 +277,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { describe('createPlainHighlight()', () => { beforeEach(() => { sandbox.stub(annotator, 'highlightCurrentSelection'); - sandbox.stub(annotator, 'createHighlightAnnotation'); + sandbox.stub(annotator, 'createHighlightThread'); annotator.createPlainHighlight(); }); @@ -285,12 +285,12 @@ describe('lib/annotations/doc/DocAnnotator', () => { expect(annotator.highlightCurrentSelection).to.be.called; }); - it('should invoke createHighlightAnnotation()', () => { - expect(annotator.createHighlightAnnotation).to.be.called; + it('should invoke createHighlightThread()', () => { + expect(annotator.createHighlightThread).to.be.called; }); }); - describe('createHighlightAnnotation()', () => { + describe('createHighlightThread()', () => { let thread; let dialog; beforeEach(() => { @@ -315,14 +315,14 @@ describe('lib/annotations/doc/DocAnnotator', () => { it('should do nothing and return null if empty string passed in', () => { annotator.lastHighlightEvent = {}; - annotator.createHighlightAnnotation(''); + annotator.createHighlightThread(''); expect(stubs.hideDialog).to.not.be.called; }); it('should do nothing and return null if there was no highlight event on the previous action', () => { annotator.lastHighlightEvent = null; - annotator.createHighlightAnnotation('some text'); + annotator.createHighlightThread('some text'); expect(stubs.hideDialog).to.not.be.called; }); @@ -330,7 +330,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { annotator.lastHighlightEvent = {}; stubs.getLocationFromEvent.returns(null); - annotator.createHighlightAnnotation('some text'); + annotator.createHighlightThread('some text'); expect(stubs.createAnnotationThread).to.not.be.called; }); @@ -340,7 +340,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - annotator.createHighlightAnnotation('some text with severe passive agression'); + annotator.createHighlightThread('some text with severe passive agression'); expect(stubs.createAnnotationThread).to.be.calledWith([], location, constants.ANNOTATION_TYPE_HIGHLIGHT); }); @@ -350,7 +350,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(null); - annotator.createHighlightAnnotation('some text'); + annotator.createHighlightThread('some text'); expect(stubs.bindCustomListenersOnThread).to.not.be.called; }); @@ -360,7 +360,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - annotator.createHighlightAnnotation(); + annotator.createHighlightThread(); expect(dialog.drawAnnotation).to.be.called; }); @@ -370,7 +370,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - annotator.createHighlightAnnotation('I think this document should be more better'); + annotator.createHighlightThread('I think this document should be more better'); expect(dialog.hasComments).to.be.true; }); @@ -380,7 +380,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - annotator.createHighlightAnnotation(); + annotator.createHighlightThread(); expect(thread.show).to.be.called; }); @@ -391,7 +391,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.createAnnotationThread.returns(thread); const text = 'This is an annotation pointing out a mistake in the document!'; - annotator.createHighlightAnnotation(text); + annotator.createHighlightThread(text); expect(dialog.postAnnotation).to.be.calledWith(text); }); @@ -401,7 +401,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - annotator.createHighlightAnnotation(); + annotator.createHighlightThread(); expect(stubs.bindCustomListenersOnThread).to.be.calledWith(thread); }); @@ -412,7 +412,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - annotator.createHighlightAnnotation(); + annotator.createHighlightThread(); expect(stubs.renderAnnotationsOnPage).to.be.calledWith(page); }); @@ -423,7 +423,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { stubs.getLocationFromEvent.returns(location); stubs.createAnnotationThread.returns(thread); - expect(annotator.createHighlightAnnotation()).to.deep.equal(thread); + expect(annotator.createHighlightThread()).to.deep.equal(thread); }); }); From 59ca760aaf528f77b66bb5d42f778c7e5bf6d4ed Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Wed, 28 Jun 2017 11:41:34 -0700 Subject: [PATCH 7/9] Chore: Fixed last char not un-highlighting when removing plain --- src/lib/annotations/doc/DocAnnotator.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 1ac5c29f0..587e37d00 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -214,13 +214,8 @@ function isThreadInHoverState(thread) { ({ pageEl, page } = annotatorUtil.getPageInfo(window.getSelection().anchorNode)); } - // Use Rangy to save the current selection because using the - // highlight module can mess with the selection. We restore this - // selection after we clean up the highlight - const savedSelection = rangy.saveSelection(); - // Use highlight module to calculate quad points - const { highlight, highlightEls } = docAnnotatorUtil.getHighlightAndHighlightEls(this.highlighter, pageEl); + const { highlightEls } = docAnnotatorUtil.getHighlightAndHighlightEls(this.highlighter, pageEl); // Do not create highlight annotation if no highlights are detected if (highlightEls.length === 0) { @@ -232,10 +227,6 @@ function isThreadInHoverState(thread) { quadPoints.push(docAnnotatorUtil.getQuadPoints(element, pageEl, zoomScale)); }); - // Remove rangy highlight and restore selection - this.removeRangyHighlight(highlight); - rangy.restoreSelection(savedSelection); - // We save the dimensions of the annotated element scaled to 100% // so we can compare to the annotated element during render time // and scale if needed (in case the representation changes size) From c9f025f1b8e6d950f3982e5e7bccc4f62d976181 Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Wed, 28 Jun 2017 13:48:12 -0700 Subject: [PATCH 8/9] Chore: Set state to render highlights --- src/lib/annotations/doc/DocAnnotator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 587e37d00..9d20f13e3 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -332,11 +332,11 @@ function isThreadInHoverState(thread) { thread.dialog.hasComments = true; } + thread.state = constants.ANNOTATION_STATE_HOVER; thread.show(); thread.dialog.postAnnotation(commentText); this.bindCustomListenersOnThread(thread); - this.renderAnnotationsOnPage(location.page); return thread; } From fe0bdd1141a9e57243f5bc25904762ec4da34f84 Mon Sep 17 00:00:00 2001 From: Justin Holdstock Date: Wed, 28 Jun 2017 15:30:05 -0700 Subject: [PATCH 9/9] Chore: Remove old test --- .../annotations/doc/__tests__/DocAnnotator-test.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index 0a6b52504..e4fee914b 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -405,17 +405,6 @@ describe('lib/annotations/doc/DocAnnotator', () => { expect(stubs.bindCustomListenersOnThread).to.be.calledWith(thread); }); - it('should cause a re-render of annotations on the current page', () => { - annotator.lastHighlightEvent = {}; - const page = 999999999; - const location = { page }; - stubs.getLocationFromEvent.returns(location); - stubs.createAnnotationThread.returns(thread); - - annotator.createHighlightThread(); - expect(stubs.renderAnnotationsOnPage).to.be.calledWith(page); - }); - it('should return an annotation thread', () => { annotator.lastHighlightEvent = {}; const page = 999999999;