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

@@ -369,7 +369,7 @@ class Annotator extends EventEmitter {
         const buttonEl = event.target || this.getAnnotateButton(buttonSelector);
 
         // Exit any other annotation mode
-        this.exitAnnotationModes(mode, buttonEl);
+        this.exitAnnotationModesExcept(mode);
 
         // If in annotation mode, turn it off
         if (this.isInAnnotationMode(mode)) {
@@ -413,8 +413,6 @@ class Annotator extends EventEmitter {
                 annotatorUtil.hideElement(postButtonEl);
                 annotatorUtil.hideElement(undoButtonEl);
                 annotatorUtil.hideElement(redoButtonEl);
-                annotatorUtil.disableElement(undoButtonEl);
-                annotatorUtil.disableElement(redoButtonEl);
             }
         }
 
@@ -458,18 +456,20 @@ class Annotator extends EventEmitter {
      * Exits all annotation modes except the specified mode
      *
      * @param {string} mode - Current annotation mode
-     * @param {HTMLElement} buttonEl - Annotation mode button DOM element
      * @return {void}
      */
-    exitAnnotationModes(mode, buttonEl) {
+    exitAnnotationModesExcept(mode) {
         Object.keys(this.modeButtons).forEach((type) => {
             if (mode === type) {
                 return;
             }
 
             const buttonSelector = this.modeButtons[type].selector;
-            const modeButtonEl = buttonEl || this.getAnnotateButton(buttonSelector);
-            this.disableAnnotationMode(type, modeButtonEl);
+            if (!this.modeButtons[type].button) {
+                this.modeButtons[type].button = this.getAnnotateButton(buttonSelector);
+            }
+
+            this.disableAnnotationMode(type, this.modeButtons[type].button);
         });
     }
 
@@ -543,7 +543,7 @@ class Annotator extends EventEmitter {
         // Do not load any pre-existing annotations if the user does not have
         // the correct permissions
         if (!this.permissions.canViewAllAnnotations || !this.permissions.canViewOwnAnnotations) {
-            return this.threads;
+            return Promise.resolve(this.threads);
         }
 
         return this.annotationService.getThreadMap(this.fileVersionId).then((threadMap) => {

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__/Annotator-test.js

@@ -288,11 +288,31 @@ describe('lib/annotations/Annotator', () => {
             });
         });
 
+        describe('exitAnnotationModesExcept()', () => {
+            it('should call disableAnnotationMode on all modes except the specified one', () => {
+                annotator.modeButtons = {
+                    'type1': {
+                        selector: 'bogus',
+                        button: 'button1'
+                    },
+                    'type2': {
+                        selector: 'test',
+                        button: 'button2'
+                    }
+                };
+
+                sandbox.stub(annotator, 'disableAnnotationMode');
+                annotator.exitAnnotationModesExcept('type2');
+                expect(annotator.disableAnnotationMode).to.be.calledWith('type1', 'button1');
+                expect(annotator.disableAnnotationMode).to.not.be.calledWith('type2', 'button2');
+            });
+        });
+
         describe('toggleAnnotationHandler()', () => {
             beforeEach(() => {
                 stubs.destroyStub = sandbox.stub(annotator, 'destroyPendingThreads');
                 stubs.annotationMode = sandbox.stub(annotator, 'isInAnnotationMode');
-                stubs.exitModes = sandbox.stub(annotator, 'exitAnnotationModes');
+                stubs.exitModes = sandbox.stub(annotator, 'exitAnnotationModesExcept');
                 stubs.disable = sandbox.stub(annotator, 'disableAnnotationMode');
                 stubs.enable = sandbox.stub(annotator, 'enableAnnotationMode');
                 sandbox.stub(annotator, 'getAnnotateButton');

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);