Merge pull request #880 from kossebau/improveAnnotationsView

Improve code around annotations a little + bug fixes
webodf · Apr 1, 2015 · e9b41aa · e9b41aa
2 parents d91660b + a1b32f3
commit e9b41aa
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 30 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -5,6 +5,7 @@
 ### Fixes
 
 * Fix breaking all empty annotations on merging the paragraph they are contained in with the one before ([#877](https://github.com/kogmbh/WebODF/pull/877)))
+* Fix error message popup on deleting an annotation starting at the end of a paragraph or styled range ([#880](https://github.com/kogmbh/WebODF/pull/880)))
 
 ### Improvements
 

diff --git a/webodf/lib/gui/AnnotationViewManager.js b/webodf/lib/gui/AnnotationViewManager.js
@@ -243,6 +243,8 @@ gui.AnnotationViewManager = function AnnotationViewManager(canvas, odfFragment,
             } else {
                 annotationNote.style.top = '0px';
             }
+        } else {
+            annotationNote.style.top = '0px';
         }
 
         connectorAngular.style.left = connectorHorizontal.getBoundingClientRect().width / zoomLevel + 'px';
@@ -341,26 +343,33 @@ gui.AnnotationViewManager = function AnnotationViewManager(canvas, odfFragment,
     this.getMinimumHeightForAnnotationPane = getMinimumHeightForAnnotationPane;
 
     /**
-     * Adds an annotation to track, and wraps and highlights it
-     * @param {!odf.AnnotationElement} annotation
+     * Adds annotations to track, and wraps and highlights them
+     * @param {!Array.<!odf.AnnotationElement>} annotationElements
      * @return {undefined}
      */
-    function addAnnotation(annotation) {
+    function addAnnotations(annotationElements) {
+        if (annotationElements.length === 0) {
+            return;
+        }
+
         showAnnotationsPane(true);
 
-        // TODO: make use of the fact that current list is already sorted
-        // instead just iterate over the list until the right index to insert is found
-        annotations.push(annotation);
+        annotationElements.forEach(function (annotation) {
+            // TODO: make use of the fact that current list is already sorted
+            // instead just iterate over the list until the right index to insert is found
+            annotations.push(annotation);
+
+            wrapAnnotation(annotation);
+            if (annotation.annotationEndElement) {
+                highlightAnnotation(annotation);
+            }
+        });
 
         sortAnnotations();
 
-        wrapAnnotation(annotation);
-        if (annotation.annotationEndElement) {
-            highlightAnnotation(annotation);
-        }
         rerenderAnnotations();
     }
-    this.addAnnotation = addAnnotation;
+    this.addAnnotations = addAnnotations;
 
     /**
      * Unhighlights, unwraps, and ejects an annotation from the tracking
@@ -378,6 +387,7 @@ gui.AnnotationViewManager = function AnnotationViewManager(canvas, odfFragment,
             showAnnotationsPane(false);
         }
     }
+    this.forgetAnnotation = forgetAnnotation;
 
     /**
      * Untracks, unwraps, and unhighlights all annotations

diff --git a/webodf/lib/odf/OdfCanvas.js b/webodf/lib/odf/OdfCanvas.js
@@ -1046,24 +1046,14 @@
             zoomHelper.setZoomableElement(sizer);
         }
 
-        /**
-        * Wraps all annotations and renders them using the Annotation View Manager.
-        * @param {!Element} odffragment
-        * @return {undefined}
-        */
-        function modifyAnnotations(odffragment) {
-            var annotationNodes = /**@type{!Array.<!odf.AnnotationElement>}*/(domUtils.getElementsByTagNameNS(odffragment, officens, 'annotation'));
-
-            annotationNodes.forEach(annotationViewManager.addAnnotation);
-            annotationViewManager.rerenderAnnotations();
-        }
-
         /**
          * This should create an annotations pane if non existent, and then populate it with annotations
          * If annotations are disallowed, it should remove the pane and all annotations
          * @param {!odf.ODFDocumentElement} odfnode
          */
         function handleAnnotations(odfnode) {
+            var annotationNodes;
+
             if (allowAnnotations) {
                 if (!annotationsPane.parentNode) {
                     sizer.appendChild(annotationsPane);
@@ -1072,7 +1062,9 @@
                     annotationViewManager.forgetAnnotations();
                 }
                 annotationViewManager = new gui.AnnotationViewManager(self, odfnode.body, annotationsPane, showAnnotationRemoveButton);
-                modifyAnnotations(odfnode.body);
+                annotationNodes = /**@type{!Array.<!odf.AnnotationElement>}*/(domUtils.getElementsByTagNameNS(odfnode.body, officens, 'annotation'));
+                annotationViewManager.addAnnotations(annotationNodes);
+
                 fixContainerSize();
             } else {
                 if (annotationsPane.parentNode) {
@@ -1286,18 +1278,19 @@
          */
         this.addAnnotation = function (annotation) {
             if (annotationViewManager) {
-                annotationViewManager.addAnnotation(annotation);
+                annotationViewManager.addAnnotations([annotation]);
                 fixContainerSize();
             }
         };
 
         /**
-         * Stops annotations and unwraps it
+         * Stops an annotation and unwraps it
+         * @param {!odf.AnnotationElement} annotation
          * @return {undefined}
          */
-        this.forgetAnnotations = function () {
+        this.forgetAnnotation = function (annotation) {
             if (annotationViewManager) {
-                annotationViewManager.forgetAnnotations();
+                annotationViewManager.forgetAnnotation(annotation);
                 fixContainerSize();
             }
         };

diff --git a/webodf/lib/ops/OdtDocument.js b/webodf/lib/ops/OdtDocument.js
@@ -119,6 +119,8 @@ ops.OdtDocument = function OdtDocument(odfCanvas) {
         }
         initialDoc = rootElement.cloneNode(true);
         odfCanvas.refreshAnnotations();
+        // workaround AnnotationViewManager not fixing up cursor positions after creating the highlighting
+        self.fixCursorPositions();
         return initialDoc;
     };
 

diff --git a/webodf/lib/ops/OpRemoveAnnotation.js b/webodf/lib/ops/OpRemoveAnnotation.js
@@ -72,7 +72,7 @@ ops.OpRemoveAnnotation = function OpRemoveAnnotation() {
         annotationEnd = annotationNode.annotationEndElement;
 
         // Untrack and unwrap annotation
-        odtDocument.getOdfCanvas().forgetAnnotations();
+        odtDocument.getOdfCanvas().forgetAnnotation(annotationNode);
 
         /**
          * @param {!Node} node
@@ -92,8 +92,8 @@ ops.OpRemoveAnnotation = function OpRemoveAnnotation() {
         // The specified position is the first walkable step in the annotation. The position is always just before the first point of change
         odtDocument.emit(ops.OdtDocument.signalStepsRemoved, {position: position > 0 ? position - 1 : position});
 
+        odtDocument.getOdfCanvas().rerenderAnnotations();
         odtDocument.fixCursorPositions();
-        odtDocument.getOdfCanvas().refreshAnnotations();
         return true;
     };