New: drawing dialog

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 the annotation selector
+     *
+     * @return {void}
+     */
+    cleanSelector() {}
+
     /**
      * 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/Annotator.scss

@@ -57,6 +57,7 @@ $avatar-color-9: #f22c44;
     }
 }
 
+.bp-annotation-drawing-dialog,
 .bp-annotation-highlight-dialog .bp-btn-plain,
 .bp-annotation-highlight-dialog .bp-btn-plain:hover {
     padding-left: 5px;
@@ -360,6 +361,7 @@ $avatar-color-9: #f22c44;
     }
 }
 
+.bp-annotation-drawing-dialog,
 .bp-annotation-highlight-dialog,
 .bp-annotation-highlight-dialog:hover {
     background-color: $white;
@@ -372,6 +374,15 @@ $avatar-color-9: #f22c44;
         padding-top: 1px;
     }
 
+    .bp-btn-annotate-draw-add {
+        padding-bottom: 3px;
+        padding-right: 6px;
+    }
+
+    .bp-btn-annotate-draw-delete {
+        padding-bottom: 3px;
+    }
+
     .bp-highlight-comment-btn {
         padding-top: 3px;
     }
@@ -428,6 +439,7 @@ $avatar-color-9: #f22c44;
     }
 }
 
+.bp-annotation-drawing-label,
 .bp-annotation-highlight-label {
     padding-left: 5px;
     padding-right: 5px;

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,11 @@ 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_ANNOTATION_DRAWING_DIALOG = 'bp-annotation-drawing-dialog';
+export const CLASS_ANNOTATION_DRAWING_BTNS = 'bp-annotation-drawing-btns';
+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,8 @@ 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
+        if (!evt || (event.target && event.target.className !== 'textLayer')) {
             return;
         }
 
@@ -459,6 +460,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

@@ -454,6 +454,10 @@ class DocAnnotator extends Annotator {
     bindDOMListeners() {
         super.bindDOMListeners();
 
+        if (!this.permissions.canAnnotate) {
+            return;
+        }
+
         this.annotatedElement.addEventListener('mouseup', this.highlightMouseupHandler);
 
         if (this.hasTouch && this.isMobile) {
@@ -485,6 +489,12 @@ class DocAnnotator extends Annotator {
             this.highlightThrottleHandle = null;
         }
 
+        if (!this.permissions.canAnnotate) {
+            return;
+        }
+
+        Object.values(this.modeControllers).forEach((controller) => controller.cleanSelector());
+
         if (this.hasTouch && this.isMobile) {
             document.removeEventListener('selectionchange', this.onSelectionChange);
             this.annotatedElement.removeEventListener('touchstart', this.drawingSelectionHandler);