Chore: plain highlights

src/lib/annotations/Annotator.js

@@ -60,6 +60,7 @@ class Annotator extends EventEmitter {
         this.locale = data.locale;
         this.validationErrorDisplayed = false;
         this.isMobile = data.isMobile;
+        this.hasTouch = data.hasTouch;
         this.previewUI = data.previewUI;
         this.annotationModeHandlers = [];
     }

src/lib/annotations/CommentBox.js

@@ -65,6 +65,9 @@ class CommentBox extends EventEmitter {
      */
     parentEl;
 
+    /** Whether or not we should use touch events */
+    hasTouch;
+
     /* Events that the comment box can emit. */
     static CommentEvents = {
         cancel: 'comment_cancel',
@@ -88,6 +91,7 @@ class CommentBox extends EventEmitter {
         this.cancelText = config.cancel || this.cancelText;
         this.postText = config.post || this.postText;
         this.placeholderText = config.placeholder || this.placeholderText;
+        this.hasTouch = config.hasTouch;
 
         // Explicit scope binding for event listeners
         this.onCancel = this.onCancel.bind(this);
@@ -162,6 +166,10 @@ class CommentBox extends EventEmitter {
         this.containerEl = null;
         this.cancelEl.removeEventListener('click', this.onCancel);
         this.postEl.removeEventListener('click', this.onPost);
+        if (this.hasTouch) {
+            this.cancelEl.removeEventListener('touchstart', this.onCancel);
+            this.postEl.removeEventListener('touchstart', this.onPost);
+        }
     }
 
     //--------------------------------------------------------------------------
@@ -196,9 +204,12 @@ class CommentBox extends EventEmitter {
      * Clear the current text in the textarea element and notify listeners.
      *
      * @private
+     * @param {Event} event Event created by input event
      * @return {void}
      */
-    onCancel() {
+    onCancel(event) {
+        // stops touch propogating to a click event
+        event.preventDefault();
         this.clear();
         this.emit(CommentBox.CommentEvents.cancel);
     }
@@ -207,9 +218,12 @@ class CommentBox extends EventEmitter {
      * Notify listeners of submit event and then clear textarea element.
      *
      * @private
+     * @param {Event} event Event created by input event
      * @return {void}
      */
-    onPost() {
+    onPost(event) {
+        // stops touch propogating to a click event
+        event.preventDefault();
         this.emit(CommentBox.CommentEvents.post, this.textAreaEl.value);
         this.clear();
     }
@@ -232,6 +246,10 @@ class CommentBox extends EventEmitter {
         // Add event listeners
         this.cancelEl.addEventListener('click', this.onCancel);
         this.postEl.addEventListener('click', this.onPost);
+        if (this.hasTouch) {
+            this.cancelEl.addEventListener('touchstart', this.onCancel);
+            this.postEl.addEventListener('touchstart', this.onPost);
+        }
 
         return containerEl;
     }

src/lib/annotations/doc/CreateHighlightDialog.js

@@ -65,20 +65,30 @@ class CreateHighlightDialog extends EventEmitter {
     /** @property {boolean} - Whether or not we're on a mobile device. */
     isMobile;
 
+    /** @property {boolean} - Whether or not we support touch. */
+    hasTouch;
+
+    /** @property {boolean} - Whether or not this is visible. */
+    isVisible;
+
     /**
      * A dialog used to create plain and comment highlights.
      *
      * [constructor]
      *
      * @param {HTMLElement} parentEl - Parent element
      * @param {boolean} isMobile - Whether or not this is running on a mobile device
+     * @param {Object} [config] - For configuring the dialog.
+     * @param {boolean} [config.hasTouch] - True to add touch events.
+     * @param {boolean} [config.isMobile] - True if on a mobile device.
      * @return {CreateHighlightDialog} CreateHighlightDialog instance
      */
-    constructor(parentEl, isMobile = false) {
+    constructor(parentEl, config = {}) {
         super();
 
         this.parentEl = parentEl;
-        this.isMobile = isMobile;
+        this.isMobile = config.isMobile || false;
+        this.hasTouch = config.hasTouch || false;
 
         // Explicit scope binding for event listeners
         this.onHighlightClick = this.onHighlightClick.bind(this);
@@ -120,6 +130,7 @@ class CreateHighlightDialog extends EventEmitter {
      * @return {void}
      */
     show(newParentEl) {
+        this.isVisible = true;
         if (!this.containerEl) {
             this.containerEl = this.createElement();
         }
@@ -145,6 +156,7 @@ class CreateHighlightDialog extends EventEmitter {
      * @return {void}
      */
     hide() {
+        this.isVisible = false;
         if (!this.containerEl) {
             return;
         }
@@ -170,7 +182,6 @@ class CreateHighlightDialog extends EventEmitter {
         // Stop interacting with this element from triggering outside actions
         this.containerEl.removeEventListener('click', this.stopPropagation);
         this.containerEl.removeEventListener('mouseup', this.stopPropagation);
-        this.containerEl.removeEventListener('touchend', this.stopPropagation);
         this.containerEl.removeEventListener('dblclick', this.stopPropagation);
 
         // Event listeners
@@ -179,6 +190,12 @@ class CreateHighlightDialog extends EventEmitter {
         this.commentBox.removeListener(CommentBox.CommentEvents.post, this.onCommentPost);
         this.commentBox.removeListener(CommentBox.CommentEvents.cancel, this.onCommentCancel);
 
+        if (this.hasTouch) {
+            this.containerEl.removeEventListener('touchend', this.stopPropagation);
+            this.highlightCreateEl.removeEventListener('touchstart', this.onHighlightClick);
+            this.commentCreateEl.removeEventListener('touchstart', this.onCommentClick);
+        }
+
         this.containerEl.remove();
         this.containerEl = null;
         this.parentEl = null;
@@ -312,8 +329,6 @@ class CreateHighlightDialog extends EventEmitter {
         // Stop interacting with this element from triggering outside actions
         highlightDialogEl.addEventListener('click', this.stopPropagation);
         highlightDialogEl.addEventListener('mouseup', this.stopPropagation);
-        highlightDialogEl.addEventListener('touchend', this.stopPropagation);
-        highlightDialogEl.addEventListener('touchstart', this.stopPropagation);
         highlightDialogEl.addEventListener('dblclick', this.stopPropagation);
 
         // Event listeners
@@ -322,6 +337,14 @@ class CreateHighlightDialog extends EventEmitter {
         this.commentBox.addListener(CommentBox.CommentEvents.post, this.onCommentPost);
         this.commentBox.addListener(CommentBox.CommentEvents.cancel, this.onCommentCancel);
 
+        // touch events
+        if (this.hasTouch) {
+            this.highlightCreateEl.addEventListener('touchstart', this.stopPropagation);
+            this.commentCreateEl.addEventListener('touchstart', this.stopPropagation);
+            this.highlightCreateEl.addEventListener('touchend', this.onHighlightClick);
+            this.commentCreateEl.addEventListener('touchend', this.onCommentClick);
+        }
+
         // Hide comment box, by default
         this.commentBox.hide();
 

src/lib/annotations/doc/DocAnnotator.js

@@ -32,6 +32,9 @@ const CLASS_RANGY_HIGHLIGHT = 'rangy-highlight';
 const SELECTOR_PREVIEW_DOC = '.bp-doc';
 const CLASS_DEFAULT_CURSOR = 'bp-use-default-cursor';
 
+// Required by rangy highlighter
+const ID_ANNOTATED_ELEMENT = 'bp-rangy-annotated-element';
+
 /**
  * For filtering out and only showing the first thread in a list of threads.
  *
@@ -59,49 +62,28 @@ function isThreadInHoverState(thread) {
 
 @autobind
 class DocAnnotator extends Annotator {
-    /**
-     * For tracking the most recent event fired by mouse move event.
-     *
-     * @property {Event}
-     */
+    /** @property {Event} - For tracking the most recent event fired by mouse move event. */
     mouseMoveEvent;
 
-    /**
-     * Event callback for mouse move events with for highlight annotations.
-     *
-     * @property {Function}
-     */
+    /** @property {Function} - Event callback for mouse move events with for highlight annotations. */
     highlightMousemoveHandler;
 
-    /**
-     * Handle to RAF used to throttle highlight collision checks.
-     *
-     * @property {Function}
-     */
+    /** @property {Function} - Handle to RAF used to throttle highlight collision checks. */
     highlightThrottleHandle;
 
-    /**
-     * Timer used to throttle highlight event process.
-     *
-     * @property {number}
-     */
+    /** @property {number} - Timer used to throttle highlight event process. */
     throttleTimer = 0;
 
-    /**
-     * UI used to create new highlight annotations.
-     *
-     * @property {CreateHighlightDialog}
-     */
+    /** @property {CreateHighlightDialog} - UI used to create new highlight annotations. */
     createHighlightDialog;
 
-    /**
-     * For delaying creation of highlight quad points and dialog. Tracks the
-     * current selection event, made in a previous event.
-     *
-     * @property {Event}
-     */
+    /** @property {Event} - For delaying creation of highlight quad points and dialog. Tracks the
+     * current selection event, made in a previous event. */
     lastHighlightEvent;
 
+    /** @property {Selection} - For tracking diffs in text selection, for mobile highlights creation. */
+    lastSelection;
+
     /**
      * Creates and mananges plain highlight and comment highlight and point annotations
      * on document files.
@@ -119,9 +101,11 @@ class DocAnnotator extends Annotator {
         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.container, this.isMobile);
+        this.createHighlightDialog = new CreateHighlightDialog(this.container, {
+            isMobile: this.isMobile,
+            hasTouch: this.hasTouch
+        });
         this.createHighlightDialog.addListener(CreateEvents.plain, this.createPlainHighlight);
 
         this.createHighlightDialog.addListener(CreateEvents.comment, this.highlightCurrentSelection);
@@ -146,6 +130,13 @@ class DocAnnotator extends Annotator {
         this.createHighlightDialog = null;
     }
 
+    /** @inheritdoc */
+    init(initialScale) {
+        super.init(initialScale);
+        // Allow rangy to highlight this
+        this.annotatedElement.id = ID_ANNOTATED_ELEMENT;
+    }
+
     //--------------------------------------------------------------------------
     // Abstract Implementations
     //--------------------------------------------------------------------------
@@ -347,6 +338,7 @@ class DocAnnotator extends Annotator {
         const annotations = [];
         const thread = this.createAnnotationThread(annotations, location, TYPES.highlight);
         this.lastHighlightEvent = null;
+        this.lastSelection = null;
 
         if (!thread) {
             return null;
@@ -424,9 +416,8 @@ 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);
+            if (this.hasTouch && this.isMobile) {
+                document.addEventListener('selectionchange', this.onSelectionChange);
             }
         }
     }
@@ -446,9 +437,8 @@ class DocAnnotator extends Annotator {
             this.annotatedElement.removeEventListener('mousedown', this.highlightMousedownHandler);
             this.annotatedElement.removeEventListener('contextmenu', this.highlightMousedownHandler);
             this.annotatedElement.removeEventListener('mousemove', this.getHighlightMouseMoveHandler());
-            if (this.isMobile) {
-                document.removeEventListener('selectionchange', this.highlightCreateHandler);
-                document.removeEventListener('touchstart', this.onTouchStart);
+            if (this.hasTouch && this.isMobile) {
+                document.removeEventListener('selectionchange', this.onSelectionChange);
             }
         }
 
@@ -500,6 +490,30 @@ class DocAnnotator extends Annotator {
     // Private
     //--------------------------------------------------------------------------
 
+    /**
+     * Handles changes in text selection. Used for mobile highlight creation.
+     *
+     * @return {void}
+     */
+    onSelectionChange(event) {
+        // Do nothing if the selection is empty
+        const selection = window.getSelection().toString();
+        // Bail if mid highlight and tapping on the screen
+        if (!selection || this.lastHighlightEvent) {
+            this.lastSelection = null;
+            this.lastHighlightEvent = null;
+            this.createHighlightDialog.hide();
+            return;
+        }
+
+        if (!this.createHighlightDialog.isVisble) {
+            this.createHighlightDialog.show(this.container);
+        }
+
+        this.lastSelection = selection;
+        this.lastHighlightEvent = event;
+    }
+
     /**
      * Highlight the current range of text that has been selected.
      *
@@ -707,19 +721,6 @@ class DocAnnotator extends Annotator {
         }
     }
 
-    /**
-     * 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.

src/lib/viewers/BaseViewer.js

@@ -662,6 +662,7 @@ class BaseViewer extends EventEmitter {
             },
             fileVersionId,
             isMobile: this.isMobile,
+            hasTouch: this.hasTouch,
             locale: location.locale,
             previewUI: this.previewUI
         });