Fix: Annotations cleanup (#449)

* Fix: Localization of dialog delete button
* Ensure that toggling between plain and highlight comments properly removes temporary annotation comment and then shows the dialog on mobile devices

src/lib/annotations/AnnotationDialog.js

@@ -518,7 +518,7 @@ class AnnotationDialog extends EventEmitter {
                         ${this.localized.cancelButton}
                     </button>
                     <button class="bp-btn bp-btn-primary ${CLASS_BUTTON_DELETE_CONFIRM}" data-type="${constants.DATA_TYPE_CONFIRM_DELETE}">
-                        ${this.localized.delete}
+                        ${this.localized.deleteButton}
                     </button>
                 </div>
             </div>`.trim();

src/lib/annotations/AnnotationThread.js

@@ -498,6 +498,7 @@ class AnnotationThread extends EventEmitter {
             this.dialog.removeAnnotation(tempAnnotation.annotationID);
         }
 
+        this.showDialog();
         this.emit(THREAD_EVENT.save);
     }
 

src/lib/annotations/Annotator.js

@@ -472,6 +472,7 @@ class Annotator extends EventEmitter {
      * needs to bind event listeners to the DOM in the normal state (ie not
      * in any annotation mode).
      *
+     * @protected
      * @return {void}
      */
     bindDOMListeners() {}
@@ -719,7 +720,6 @@ class Annotator extends EventEmitter {
      * @param {number} [rotationAngle] - current angle image is rotated
      * @param {number} [pageNum] - Page number
      * @return {void}
-     * @private
      */
     rotateAnnotations(rotationAngle = 0, pageNum = 0) {
         // Only render a specific page's annotations unless no page number
@@ -767,6 +767,7 @@ class Annotator extends EventEmitter {
     /**
      * Returns click handler for toggling annotation mode.
      *
+     * @private
      * @param {string} mode - Target annotation mode
      * @return {Function|null} Click handler
      */
@@ -783,7 +784,7 @@ class Annotator extends EventEmitter {
     /**
      * Orient annotations to the correct scale and orientation of the annotated document.
      *
-     * @protected
+     * @private
      * @param {Object} data - Scale and orientation values needed to orient annotations.
      * @return {void}
      */

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

@@ -203,6 +203,7 @@ describe('lib/annotations/AnnotationThread', () => {
             stubs.create = sandbox.stub(annotationService, 'create');
             stubs.saveAnnotationToThread = sandbox.stub(thread, 'saveAnnotationToThread');
             sandbox.stub(thread, 'getThreadEventData').returns({});
+            sandbox.stub(thread, 'showDialog');
         });
 
         it('should save annotation to thread if it does not exist in annotations array', () => {

src/lib/annotations/doc/DocDrawingDialog.js

@@ -37,6 +37,8 @@ class DocDrawingDialog extends AnnotationDialog {
         if (this.pageEl && this.pageEl.contains(this.element)) {
             this.pageEl.removeChild(this.element);
         }
+
+        this.element = null;
     }
 
     /**
@@ -209,7 +211,7 @@ class DocDrawingDialog extends AnnotationDialog {
             const commitButton = annotatorUtil.generateBtn(
                 constants.CLASS_ADD_DRAWING_BTN,
                 this.localized.drawSave,
-                `${ICON_DRAW_SAVE}${this.localized.saveButton}`.trim()
+                `${ICON_DRAW_SAVE} ${this.localized.saveButton}`
             );
             drawingButtonsEl.appendChild(commitButton);
         }
@@ -218,7 +220,7 @@ class DocDrawingDialog extends AnnotationDialog {
             const deleteButton = annotatorUtil.generateBtn(
                 constants.CLASS_DELETE_DRAWING_BTN,
                 this.localized.drawDelete,
-                `${ICON_DRAW_DELETE}${this.localized.deleteButton}`.trim()
+                `${ICON_DRAW_DELETE} ${this.localized.deleteButton}`
             );
             drawingButtonsEl.appendChild(deleteButton);
         }

src/lib/annotations/doc/DocHighlightDialog.js

@@ -47,6 +47,20 @@ class DocHighlightDialog extends AnnotationDialog {
         super.addAnnotation(annotation);
     }
 
+    /**
+     * Removes an annotation from the dialog.
+     *
+     * @param {string} annotationID - ID of annotation to remove
+     * @return {void}
+     */
+    removeAnnotation(annotationID) {
+        const annotationEl = this.commentsDialogEl.querySelector(`[data-annotation-id="${annotationID}"]`);
+        if (annotationEl) {
+            annotationEl.parentNode.removeChild(annotationEl);
+            this.deactivateReply(); // Deactivate reply area and focus
+        }
+    }
+
     /** @inheritdoc */
     postAnnotation(textInput) {
         const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);

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

@@ -91,6 +91,33 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
         });
     });
 
+    describe('removeAnnotation()', () => {
+        it('should remove annotation element and deactivate reply', () => {
+            stubs.deactivate = sandbox.stub(dialog, 'deactivateReply');
+
+            dialog.addAnnotation(
+                new Annotation({
+                    annotationID: 'someID',
+                    text: 'blah',
+                    user: {},
+                    permissions: {}
+                })
+            );
+
+            dialog.removeAnnotation('someID');
+            const annotationEl = dialog.commentsDialogEl.querySelector('[data-annotation-id="someID"]');
+            expect(annotationEl).to.be.null;
+            expect(stubs.deactivate).to.be.called;
+        });
+
+        it('should not do anything if the specified annotation does not exist', () => {
+            stubs.deactivate = sandbox.stub(dialog, 'deactivateReply');
+
+            dialog.removeAnnotation('someID');
+            expect(stubs.deactivate).to.not.be.called;
+        });
+    });
+
     describe('postAnnotation()', () => {
         beforeEach(() => {
             stubs.postFunc = AnnotationDialog.prototype.postAnnotation;