Feature: Drawing boundary updates on each action (#356)

* Update: drawing boundary update
* Update: merge from master
* Update: tests for drawing boundary on each stroke
* Update: docs on DocDrawingDialog

src/lib/annotations/doc/DocDrawingDialog.js

@@ -0,0 +1,41 @@
+import AnnotationDialog from '../AnnotationDialog';
+
+class DocDrawingDialog extends AnnotationDialog {
+    /**
+     * Empty stub to avoid unexpected behavior.
+     *
+     * @override
+     * @protected
+     */
+    addAnnotation() {}
+
+    /**
+     * Empty stub to avoid unexpected behavior.
+     *
+     * @override
+     * @protected
+     */
+    removeAnnotation() {}
+
+    /**
+     * Sets up the drawing dialog element.
+     *
+     * @param {Annotation[]} annotations - Annotations to show in the dialog
+     * @param {HTMLElement} threadEl - Annotation icon element
+     * @return {void}
+     * @protected
+     */
+    /* eslint-disable no-unused-vars */
+    setup(annotations, threadEl) {}
+    /* eslint-enable no-unused-vars */
+
+    /**
+     * Display the dialog in the browser
+     *
+         * @protected
+     * @return {void}
+     */
+    show() {}
+}
+
+export default DocDrawingDialog;

src/lib/annotations/doc/DocDrawingThread.js

@@ -1,5 +1,6 @@
 import DrawingPath from '../drawing/DrawingPath';
 import DrawingThread from '../drawing/DrawingThread';
+import DocDrawingDialog from '../doc/DocDrawingDialog';
 import {
     STATES,
     DRAW_STATES,
@@ -96,11 +97,21 @@ class DocDrawingThread extends DrawingThread {
     handleStop() {
         this.drawingFlag = DRAW_STATES.idle;
 
-        if (this.pendingPath && !this.pendingPath.isEmpty()) {
-            this.pathContainer.insert(this.pendingPath);
-            this.emitAvailableActions();
-            this.pendingPath = null;
+        if (!this.pendingPath || this.pendingPath.isEmpty()) {
+            return;
         }
+
+        this.pathContainer.insert(this.pendingPath);
+        this.updateBoundary(this.pendingPath);
+        this.setBoundary();
+        this.emitAvailableActions();
+        this.pendingPath = null;
+
+        if (this.dialog) {
+            return;
+        }
+
+        this.createDialog();
     }
 
     /**
@@ -134,18 +145,13 @@ class DocDrawingThread extends DrawingThread {
             return;
         }
 
-        super.saveAnnotation(type, text);
         this.setBoundary();
+        super.saveAnnotation(type, text);
 
-        this.concreteContext = getContext(this.pageEl, CLASS_ANNOTATION_LAYER_DRAW);
-        if (this.concreteContext) {
-            // Move the in-progress drawing to the concrete context
-            const inProgressCanvas = this.drawingContext.canvas;
-            const width = parseInt(inProgressCanvas.style.width, 10);
-            const height = parseInt(inProgressCanvas.style.height, 10);
-            this.concreteContext.drawImage(inProgressCanvas, 0, 0, width, height);
-            this.drawingContext.clearRect(0, 0, inProgressCanvas.width, inProgressCanvas.height);
-        }
+        // Move the in-progress drawing to the concrete context
+        const inProgressCanvas = this.drawingContext.canvas;
+        this.drawingContext.clearRect(0, 0, inProgressCanvas.width, inProgressCanvas.height);
+        this.show();
     }
 
     /**
@@ -161,6 +167,9 @@ class DocDrawingThread extends DrawingThread {
 
         // Get the annotation layer context to draw with
         const context = this.selectContext();
+        if (this.state === STATES.pending) {
+            this.drawBoundary();
+        }
 
         // Generate the paths and draw to the annotation layer canvas
         this.pathContainer.applyToItems((drawing) =>
@@ -254,7 +263,7 @@ class DocDrawingThread extends DrawingThread {
      * @return {Array|null} The an array of length 4 with the first item being the x coordinate, the second item
      *                      being the y coordinate, and the 3rd/4th items respectively being the width and height
      */
-    getRectangularBoundary() {
+    getBrowserRectangularBoundary() {
         if (!this.location || !this.location.dimensions || !this.pageEl) {
             return null;
         }
@@ -268,6 +277,23 @@ 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;

src/lib/annotations/doc/__tests__/DocDrawingThread-test.js

@@ -116,22 +116,41 @@ describe('lib/annotations/doc/DocDrawingThread', () => {
     });
 
     describe('handleStop()', () => {
-        it("should set the state to 'idle' and clear the pendingPath", () => {
-            sandbox.stub(docDrawingThread, 'emitAvailableActions');
+        beforeEach(() => {
+            stubs.emitAvailableActions = sandbox.stub(docDrawingThread, 'emitAvailableActions');
+            stubs.updateBoundary = sandbox.stub(docDrawingThread, 'updateBoundary');
+            stubs.setBoundary = sandbox.stub(docDrawingThread, 'setBoundary');
+            stubs.createDialog = sandbox.stub(docDrawingThread, 'createDialog');
             docDrawingThread.drawingFlag = DRAW_STATES.drawing;
             docDrawingThread.pendingPath = {
                 isEmpty: () => false
             };
             docDrawingThread.pathContainer = {
                 insert: sandbox.stub()
-            }
-
+            };
+        })
+        it("should set the state to 'idle' and clear the pendingPath", () => {
             docDrawingThread.handleStop();
 
-            expect(docDrawingThread.emitAvailableActions).to.be.called;
+            expect(stubs.emitAvailableActions).to.be.called;
+            expect(stubs.updateBoundary).to.be.called;
+            expect(stubs.setBoundary).to.be.called;
+            expect(stubs.createDialog).to.be.called;
+            expect(docDrawingThread.pathContainer.insert).to.be.called;
             expect(docDrawingThread.drawingFlag).to.equal(DRAW_STATES.idle);
             expect(docDrawingThread.pendingPath).to.be.null;
         });
+
+        it('should not create a dialog if one already exists', () => {
+            docDrawingThread.dialog = {
+                value: 'non-empty',
+                removeAllListeners: () => {},
+                destroy: () => {}
+            }
+
+            docDrawingThread.handleStop();
+            expect(stubs.createDialog).to.not.be.called;
+        });
     });
 
     describe('onPageChange()', () => {
@@ -200,6 +219,8 @@ describe('lib/annotations/doc/DocDrawingThread', () => {
         const resetValue = AnnotationThread.prototype.saveAnnotation;
 
         beforeEach(() => {
+            stubs.setBoundary = sandbox.stub(docDrawingThread, 'setBoundary');
+            stubs.show = sandbox.stub(docDrawingThread, 'show');
             Object.defineProperty(AnnotationThread.prototype, 'saveAnnotation', { value: sandbox.stub() });
         });
 
@@ -222,6 +243,7 @@ describe('lib/annotations/doc/DocDrawingThread', () => {
             expect(docDrawingThread.emit).to.be.calledWith('annotationevent', {
                 type: 'drawcommit'
             });
+            expect(stubs.show).to.not.be.called;
         });
 
         it('should clean up and commit in-progress drawings when there are paths to be saved', () => {
@@ -236,21 +258,17 @@ describe('lib/annotations/doc/DocDrawingThread', () => {
                 height: 30,
                 clearRect: sandbox.stub()
             };
-            const context = {
-                drawImage: sandbox.stub()
-            };
 
-            sandbox.stub(docAnnotatorUtil, 'getContext').returns(context);
             sandbox.stub(docDrawingThread.pathContainer, 'getNumberOfItems').returns({
                 undoCount: 1,
                 redoCount: 0
             });
 
             docDrawingThread.saveAnnotation('draw');
-            expect(docAnnotatorUtil.getContext).to.be.called;
+            expect(stubs.show).to.be.called;
+            expect(stubs.setBoundary).to.be.called;
             expect(docDrawingThread.pathContainer.getNumberOfItems).to.be.called;
             expect(docDrawingThread.drawingContext.clearRect).to.be.called;
-            expect(context.drawImage).to.be.called;
             expect(AnnotationThread.prototype.saveAnnotation).to.be.called;
         });
     });
@@ -369,11 +387,11 @@ describe('lib/annotations/doc/DocDrawingThread', () => {
         });
     });
 
-    describe('getRectangularBoundary()', () => {
+    describe('getBrowserRectangularBoundary()', () => {
         it('should return null when no thread has not been assigned a location', () => {
             docDrawingThread.location = undefined;
 
-            const value = docDrawingThread.getRectangularBoundary();
+            const value = docDrawingThread.getBrowserRectangularBoundary();
             expect(value).to.be.null;
         });
 
@@ -388,7 +406,7 @@ describe('lib/annotations/doc/DocDrawingThread', () => {
             stubs.getBrowserCoordinates.onCall(0).returns([5, 5]);
             stubs.getBrowserCoordinates.onCall(1).returns([50, 45]);
 
-            const value = docDrawingThread.getRectangularBoundary();
+            const value = docDrawingThread.getBrowserRectangularBoundary();
             expect(stubs.createLocation).to.be.called.twice;
             expect(stubs.getBrowserCoordinates).to.be.called.twice;
             expect(value).to.deep.equal([5, 5, 45, 40]);

src/lib/annotations/drawing/DrawingPath.js

@@ -148,23 +148,24 @@ class DrawingPath {
      * Extract the path information from two paths by merging their paths and getting the bounding rectangle
      *
      * @public
-     * @param {Object} accumulator - A drawingPath or accumulator to extract information from
-     * @param {DrawingPath} pathB - Another drawingPath to extract information from
+     * @param {DrawingPath} pathA - Another drawingPath to extract information from
+     * @param {Object} accumulator - A drawingPath accumulator to retain boundary and path information
      * @return {Object} A bounding rectangle and the stroke paths it contains
      */
-    static extractDrawingInfo(accumulator, pathB) {
+    static extractDrawingInfo(pathA, accumulator) {
         let paths = accumulator.paths;
-        if (paths) {
-            paths.push(pathB.path);
+        const apath = { path: pathA.path };
+        if (!paths) {
+            paths = [apath];
         } else {
-            paths = [accumulator.path, pathB.path];
+            paths.push(apath);
         }
 
         return {
-            minX: Math.min(accumulator.minX, pathB.minX),
-            maxX: Math.max(accumulator.maxX, pathB.maxX),
-            minY: Math.min(accumulator.minY, pathB.minY),
-            maxY: Math.max(accumulator.maxY, pathB.maxY),
+            minX: accumulator.minX ? Math.min(accumulator.minX, pathA.minX) : pathA.minX,
+            maxX: accumulator.maxX ? Math.max(accumulator.maxX, pathA.maxX) : pathA.maxX,
+            minY: accumulator.minY ? Math.min(accumulator.minY, pathA.minY) : pathA.minY,
+            maxY: accumulator.maxY ? Math.max(accumulator.maxY, pathA.maxY) : pathA.maxY,
             paths
         };
     }