Feature: drawingSelectionUI (#362)

* Update: drawing boundary update
* Update: tests for drawing boundary on each stroke

* Update: drawing dialog

* Update: tests
* Update: PR feedback

src/i18n/en-US.properties

@@ -135,6 +135,12 @@ annotation_highlight_toggle=Highlight text
 annotation_highlight_comment=Add comment to highlighted text
 # Text for which user made the highlight annotation
 annotation_who_highlighted={1} highlighted
+# Text for which user made the drawn annotation
+annotation_who_drew={1} drew
+# Accessibilty text for button that soft commits drawing
+annotation_draw_save=Save drawing
+# Accessibilty text for button that soft deletes drawing
+annotation_draw_delete=Delete drawing
 # Text for when annotations fail to load
 annotations_load_error=We're sorry, annotations failed to load for this file.
 # Text for when the annotation can't be created

src/lib/annotations/AnnotationModeController.js

@@ -88,6 +88,13 @@ class AnnotationModeController extends EventEmitter {
         this.threads = this.threads.filter((item) => item !== thread);
     }
 
+    /**
+     * Clean up any selected annotations
+     *
+     * @return {void}
+     */
+    removeSelection() {}
+
     /**
      * Binds custom event listeners for a thread.
      *

src/lib/annotations/Annotator.js

@@ -415,8 +415,6 @@ class Annotator extends EventEmitter {
                 annotatorUtil.hideElement(postButtonEl);
                 annotatorUtil.hideElement(undoButtonEl);
                 annotatorUtil.hideElement(redoButtonEl);
-                annotatorUtil.disableElement(undoButtonEl);
-                annotatorUtil.disableElement(redoButtonEl);
             }
         }
 

src/lib/annotations/__tests__/annotatorUtil-test.js

@@ -25,7 +25,8 @@ import {
     getHeaders,
     replacePlaceholders,
     createLocation,
-    round
+    round,
+    prevDefAndStopProp
 } from '../annotatorUtil';
 import {
     STATES,
@@ -36,6 +37,8 @@ import {
 
 const DIALOG_WIDTH = 81;
 
+const sandbox = sinon.sandbox.create();
+
 describe('lib/annotations/annotatorUtil', () => {
     let childEl;
     let parentEl;
@@ -52,6 +55,7 @@ describe('lib/annotations/annotatorUtil', () => {
     });
 
     afterEach(() => {
+        sandbox.verifyAndRestore();
         fixture.cleanup();
     });
 
@@ -403,39 +407,54 @@ describe('lib/annotations/annotatorUtil', () => {
     });
 
     describe('eventToLocationHandler()', () => {
-        it('should not call the callback when the location is valid', () => {
-            const annotator = {
-                isChanged: false
-            }
-            const getLocation = ((event) => 'location');
-            const callback = ((location) => {
-                annotator.isChanged = true
-            });
-            const locationHandler = eventToLocationHandler(getLocation, callback);
+        let getLocation;
+        let annotator;
+        let callback;
+        let locationHandler;
+        let event;
+
+        beforeEach(() => {
+            getLocation = ((event) => 'location');
+            callback = sandbox.stub();
+            locationHandler = eventToLocationHandler(getLocation, callback);
+            event = {
+                preventDefault: () => {},
+                stopPropagation: () => {}
+            };
+        });
 
+        it('should not call the callback when the location is valid', () => {
             locationHandler(undefined);
-            expect(annotator.isChanged).to.be.false;
+            expect(callback).to.not.be.called;
         });
 
         it('should call the callback when the location is valid', () => {
-            const annotator = {
-                isChanged: false
-            }
-            const getLocation = ((event) => 'location');
-            const callback = ((location) => {
-                annotator.isChanged = true
-            });
-            const locationHandler = eventToLocationHandler(getLocation, callback);
-            const event = {
-                preventDefault: () => {},
-                stopPropagation: () => {}
-            };
+            locationHandler(event);
+            expect(callback).to.be.calledWith('location');
+        });
 
+        it('should do nothing when the target exists and it is not the textLayer', () => {
+            event.target = {
+                className: 'button'
+            };
             locationHandler(event);
-            expect(annotator.isChanged).to.be.true;
+            expect(callback).to.not.be.called;
         });
     });
 
+    describe('prevDefAndStopProp()', () => {
+        it('should prevent default and stop propogation on an event', () => {
+            const event = {
+                preventDefault: sandbox.stub(),
+                stopPropagation: sandbox.stub()
+            };
+
+            prevDefAndStopProp(event);
+            expect(event.preventDefault).to.be.called;
+            expect(event.stopPropagation).to.be.called;
+        })
+    });
+
     describe('createLocation()', () => {
         it('should create a location object without dimensions', () => {
             const location = createLocation(1,2, undefined);

src/lib/annotations/annotationConstants.js

@@ -1,3 +1,5 @@
+export const USER_ANONYMOUS = 'Anonymous';
+
 export const CLASS_ACTIVE = 'bp-is-active';
 export const CLASS_HIDDEN = 'bp-is-hidden';
 export const CLASS_INVISIBLE = 'bp-is-invisible';
@@ -32,6 +34,9 @@ export const CLASS_ANNOTATION_BUTTON_DRAW_REDO = 'bp-btn-annotate-draw-redo';
 export const CLASS_ANNOTATION_BUTTON_DRAW_POST = 'bp-btn-annotate-draw-post';
 export const CLASS_ANNOTATION_BUTTON_DRAW_CANCEL = 'bp-btn-annotate-draw-cancel';
 export const CLASS_ANNOTATION_BUTTON_DRAW_ENTER = 'bp-btn-annotate-draw-enter';
+export const CLASS_ANNOTATION_DRAWING_LABEL = 'bp-annotation-drawing-label';
+export const CLASS_ADD_DRAWING_BTN = 'bp-btn-annotate-draw-add';
+export const CLASS_DELETE_DRAWING_BTN = 'bp-btn-annotate-draw-delete';
 
 export const DATA_TYPE_ANNOTATION_DIALOG = 'annotation-dialog';
 export const DATA_TYPE_ANNOTATION_INDICATOR = 'annotation-indicator';

src/lib/annotations/annotatorUtil.js

@@ -439,7 +439,7 @@ export function validateThreadParams(thread) {
 }
 
 /**
- * Returns a function that passes a callback a location when given an event
+ * Returns a function that passes a callback a location when given an event on the document text layer
  *
  * @param {Function} locationFunction - The function to get a location from an event
  * @param {Function} callback - Callback to be called upon receiving an event
@@ -448,7 +448,9 @@ export function validateThreadParams(thread) {
 export function eventToLocationHandler(locationFunction, callback) {
     return (event) => {
         const evt = event || window.event;
-        if (!evt) {
+        // Do nothing when the target isn't the text layer in case the text layer receives event precedence over buttons
+        // NOTE: Currently only applicable to documents. Need to find a better way to ensure button event precedence.
+        if (!evt || (event.target && event.target.className !== 'textLayer')) {
             return;
         }
 
@@ -459,6 +461,20 @@ export function eventToLocationHandler(locationFunction, callback) {
     };
 }
 
+/**
+ * Call preventDefault and stopPropagation on an event
+ *
+ * @param {event} event - Event object to stop event bubbling
+ * @return {void}
+ */
+export function prevDefAndStopProp(event) {
+    if (!event) {
+        return;
+    }
+
+    event.preventDefault();
+    event.stopPropagation();
+}
 /**
  * Create a JSON object containing x/y coordinates and optionally dimensional information
  *

src/lib/annotations/doc/DocAnnotator.js

@@ -485,6 +485,8 @@ class DocAnnotator extends Annotator {
             this.highlightThrottleHandle = null;
         }
 
+        Object.values(this.modeControllers).forEach((controller) => controller.removeSelection());
+
         if (this.hasTouch && this.isMobile) {
             document.removeEventListener('selectionchange', this.onSelectionChange);
             this.annotatedElement.removeEventListener('touchstart', this.drawingSelectionHandler);

src/lib/annotations/doc/DocDrawingDialog.js

@@ -1,6 +1,8 @@
 import AnnotationDialog from '../AnnotationDialog';
 
 class DocDrawingDialog extends AnnotationDialog {
+    visible = false;
+
     /**
      * Empty stub to avoid unexpected behavior.
      *
@@ -36,6 +38,10 @@ class DocDrawingDialog extends AnnotationDialog {
      * @return {void}
      */
     show() {}
+
+    isVisible() {
+        return this.visible;
+    }
 }
 
 export default DocDrawingDialog;

src/lib/annotations/doc/DocDrawingThread.js

@@ -30,6 +30,7 @@ class DocDrawingThread extends DrawingThread {
         this.onPageChange = this.onPageChange.bind(this);
         this.reconstructBrowserCoordFromLocation = this.reconstructBrowserCoordFromLocation.bind(this);
     }
+
     /**
      * Handle a pointer movement
      *
@@ -84,6 +85,10 @@ class DocDrawingThread extends DrawingThread {
             this.pendingPath = new DrawingPath();
         }
 
+        if (this.dialog && this.dialog.isVisible()) {
+            this.dialog.hide();
+        }
+
         // Start drawing rendering
         this.lastAnimationRequestId = window.requestAnimationFrame(this.render);
     }
@@ -134,23 +139,18 @@ class DocDrawingThread extends DrawingThread {
      * @return {void}
      */
     saveAnnotation(type, text) {
-        this.emit('annotationevent', {
-            type: 'drawcommit'
-        });
         this.reset();
 
         // Only make save request to server if there exist paths to save
-        const { undoCount } = this.pathContainer.getNumberOfItems();
-        if (undoCount === 0) {
+        if (this.pathContainer.isUndoEmpty()) {
             return;
         }
 
         this.setBoundary();
         super.saveAnnotation(type, text);
 
         // Move the in-progress drawing to the concrete context
-        const inProgressCanvas = this.drawingContext.canvas;
-        this.drawingContext.clearRect(0, 0, inProgressCanvas.width, inProgressCanvas.height);
+        this.createDialog();
         this.show();
     }
 
@@ -167,7 +167,7 @@ class DocDrawingThread extends DrawingThread {
 
         // Get the annotation layer context to draw with
         const context = this.selectContext();
-        if (this.state === STATES.pending) {
+        if (this.dialog && this.dialog.isVisible()) {
             this.drawBoundary();
         }
 
@@ -183,6 +183,53 @@ class DocDrawingThread extends DrawingThread {
         this.draw(context, false);
     }
 
+    /**
+     * Binds custom event listeners for the dialog.
+     *
+     * @protected
+     * @return {void}
+     */
+    bindCustomListenersOnDialog() {
+        if (!this.dialog) {
+            return;
+        }
+
+        this.dialog.addListener('annotationcreate', () => {
+            this.emit('annotationevent', {
+                type: 'softcommit'
+            });
+        });
+        this.dialog.addListener('annotationdelete', () => {
+            this.emit('annotationevent', {
+                type: 'dialogdelete'
+            });
+        });
+    }
+
+    /**
+     * Creates the document drawing annotation dialog for the thread.
+     *
+     * @protected
+     * @override
+     * @return {void}
+     */
+    createDialog() {
+        if (this.dialog) {
+            this.dialog.destroy();
+        }
+
+        this.dialog = new DocDrawingDialog({
+            annotatedElement: this.annotatedElement,
+            container: this.container,
+            annotations: this.annotations,
+            locale: this.locale,
+            location: this.location,
+            canAnnotate: this.annotationService.canAnnotate
+        });
+
+        this.bindCustomListenersOnDialog();
+    }
+
     /**
      * Prepare the pending drawing canvas if the scale factor has changed since the last render. Will do nothing if
      * the thread has not been assigned a page.
@@ -215,7 +262,7 @@ class DocDrawingThread extends DrawingThread {
     onPageChange(location) {
         this.handleStop();
         this.emit('annotationevent', {
-            type: 'pagechanged',
+            type: 'softcommit',
             location
         });
     }
@@ -277,23 +324,6 @@ class DocDrawingThread extends DrawingThread {
 
         return [x1, y1, width, height];
     }
-
-    /**
-     * Creates the document drawing annotation dialog for the thread.
-     *
-     * @override
-     * @return {void}
-     */
-    createDialog() {
-        this.dialog = new DocDrawingDialog({
-            annotatedElement: this.annotatedElement,
-            container: this.container,
-            annotations: this.annotations,
-            locale: this.locale,
-            location: this.location,
-            canAnnotate: this.annotationService.canAnnotate
-        });
-    }
 }
 
 export default DocDrawingThread;