Merge pull request #182 from hypothesis/anchoring-pending-state

Show annotations in Annotations tab but with quote struck through after anchoring timeout

h/static/scripts/annotation-metadata.js

@@ -114,9 +114,16 @@ function hasSelector(annotation) {
             annotation.target[0].selector);
 }
 
-/** Return `true` if the given annotation is not yet anchored. */
+/**
+ * Return `true` if the given annotation is not yet anchored.
+ *
+ * Returns false if anchoring is still in process but the flag indicating that
+ * the initial timeout allowed for anchoring has expired.
+ */
 function isWaitingToAnchor(annotation) {
-  return hasSelector(annotation) && (typeof annotation.$orphan === 'undefined');
+  return hasSelector(annotation) &&
+         (typeof annotation.$orphan === 'undefined') &&
+         !annotation.$anchorTimeout;
 }
 
 /** Return `true` if the given annotation is an orphan. */

h/static/scripts/directive/annotation.js

@@ -426,7 +426,13 @@ function AnnotationController(
   var indicateOrphans = features.flagEnabled('orphans_tab');
 
   vm.isOrphan = function() {
-    return vm.annotation.$orphan && indicateOrphans;
+    if (!indicateOrphans) {
+      return false;
+    }
+    if (typeof vm.annotation.$orphan === 'undefined') {
+      return vm.annotation.$anchorTimeout;
+    }
+    return vm.annotation.$orphan;
   };
 
   vm.updated = function() {

h/static/scripts/directive/test/annotation-test.js

@@ -698,6 +698,33 @@ describe('annotation', function() {
       });
     });
 
+    describe('#isOrphan', function () {
+      it('returns false if the annotation is not an orphan', function () {
+        var controller = createDirective().controller;
+        controller.annotation.$orphan = false;
+        assert.isFalse(controller.isOrphan());
+      });
+
+      it('returns true if the annotation is an orphan', function () {
+        var controller = createDirective().controller;
+        controller.annotation.$orphan = true;
+        assert.isTrue(controller.isOrphan());
+      });
+
+      it('returns true if the anchoring timeout expired', function () {
+        var controller = createDirective().controller;
+        controller.annotation.$anchorTimeout = true;
+        assert.isTrue(controller.isOrphan());
+      });
+
+      it('returns false if the anchoring timeout expired but anchoring did complete', function () {
+        var controller = createDirective().controller;
+        controller.annotation.$orphan = false;
+        controller.annotation.$anchorTimeout = true;
+        assert.isFalse(controller.isOrphan());
+      });
+    });
+
     describe('saving a new annotation', function() {
       var annotation;
 

h/static/scripts/reducers/annotations.js

@@ -63,6 +63,8 @@ function initializeAnnot(annotation, tag) {
   }
 
   return Object.assign({}, annotation, {
+    // Flag indicating whether waiting for the annotation to anchor timed out.
+    $anchorTimeout: false,
     $tag: annotation.$tag || tag,
     $orphan: orphan,
   });
@@ -150,6 +152,7 @@ var update = {
                   (annot.$tag && annot.$tag === action.tag);
       if (match) {
         return Object.assign({}, annot, {
+          $anchorTimeout: action.anchorTimeout || annot.$anchorTimeout,
           $orphan: action.isOrphan,
           $tag: action.tag,
         });
@@ -209,9 +212,9 @@ function addAnnotations(annotations, now) {
           .forEach(function (orphan) {
             dispatch({
               type: actions.UPDATE_ANCHOR_STATUS,
+              anchorTimeout: true,
               id: orphan.id,
               tag: orphan.$tag,
-              isOrphan: true,
             });
           });
       }, ANCHORING_TIMEOUT);

h/static/scripts/test/annotation-metadata-test.js

@@ -287,5 +287,12 @@ describe('annotation-metadata', function () {
     it('returns false for page notes', function () {
       assert.isFalse(isWaitingToAnchor(fixtures.oldPageNote()));
     });
+
+    it('returns false if the anchoring timeout flag was set', function () {
+      var pending = Object.assign({}, fixtures.defaultAnnotation(), {
+        $anchorTimeout: true,
+      });
+      assert.isFalse(isWaitingToAnchor(pending));
+    });
   });
 });

h/static/scripts/test/annotation-ui-test.js

@@ -146,32 +146,23 @@ describe('annotationUI', function () {
       assert.isFalse(updatedAnnot.$orphan);
     });
 
-    it('marks annotations as orphans if they fail to anchor within a time limit', function () {
-      var isOrphan = function () {
-        return !!metadata.isOrphan(annotationUI.getState().annotations[0]);
-      };
-
+    it('sets the timeout flag on annotations that fail to anchor within a time limit', function () {
       var annot = defaultAnnotation();
       annotationUI.addAnnotations([annot]);
-      assert.isFalse(isOrphan());
 
       clock.tick(ANCHOR_TIME_LIMIT);
 
-      assert.isTrue(isOrphan());
+      assert.isTrue(annotationUI.getState().annotations[0].$anchorTimeout);
     });
 
-    it('does not mark annotations as orphans if they do anchor within a time limit', function () {
-      var isOrphan = function () {
-        return !!metadata.isOrphan(annotationUI.getState().annotations[0]);
-      };
-
+    it('does not set the timeout flag on annotations that do anchor within a time limit', function () {
       var annot = defaultAnnotation();
       annotationUI.addAnnotations([annot]);
       annotationUI.updateAnchorStatus(annot.id, 'atag', false);
 
       clock.tick(ANCHOR_TIME_LIMIT);
 
-      assert.isFalse(isOrphan());
+      assert.isFalse(annotationUI.getState().annotations[0].$anchorTimeout);
     });
 
     it('does not attempt to modify orphan status if annotations are removed before anchoring timeout expires', function () {

h/static/scripts/test/tabs-test.js

@@ -46,6 +46,7 @@ describe('tabs', function () {
   describe('shouldShowInTab', function () {
     unroll('returns true if the annotation should be shown', function (testCase) {
       var ann = fixtures.defaultAnnotation();
+      ann.$anchorTimeout = testCase.anchorTimeout;
       ann.$orphan = testCase.orphan;
 
       assert.equal(tabs.shouldShowInTab(ann, uiConstants.TAB_ANNOTATIONS,
@@ -53,18 +54,51 @@ describe('tabs', function () {
       assert.equal(tabs.shouldShowInTab(ann, uiConstants.TAB_ORPHANS,
         testCase.separateOrphans), testCase.expectedTab === uiConstants.TAB_ORPHANS);
     }, [{
+      // Orphans tab disabled, anchoring in progress.
+      anchorTimeout: false,
       orphan: undefined,
       separateOrphans: false,
       expectedTab: uiConstants.TAB_ANNOTATIONS,
     },{
+      // Orphans tab disabled, anchoring succeeded.
+      anchorTimeout: false,
+      orphan: false,
+      separateOrphans: false,
+      expectedTab: uiConstants.TAB_ANNOTATIONS,
+    },{
+      // Orphans tab disabled, anchoring failed
+      anchorTimeout: false,
+      orphan: true,
+      separateOrphans: false,
+      expectedTab: uiConstants.TAB_ANNOTATIONS,
+    },{
+      // Orphans tab enabled, anchoring in progress.
+      anchorTimeout: false,
       orphan: undefined,
       separateOrphans: true,
       expectedTab: null,
     },{
+      // Orphans tab enabled, anchoring succeeded.
+      anchorTimeout: false,
       orphan: false,
       separateOrphans: true,
       expectedTab: uiConstants.TAB_ANNOTATIONS,
     },{
+      // Orphans tab enabled, anchoring failed.
+      anchorTimeout: false,
+      orphan: true,
+      separateOrphans: true,
+      expectedTab: uiConstants.TAB_ORPHANS,
+    },{
+      // Orphans tab enabled, anchoring timed out.
+      anchorTimeout: true,
+      orphan: undefined,
+      separateOrphans: true,
+      expectedTab: uiConstants.TAB_ANNOTATIONS,
+    },{
+      // Orphans tab enabled, anchoring initially timed out but eventually
+      // failed.
+      anchorTimeout: true,
       orphan: true,
       separateOrphans: true,
       expectedTab: uiConstants.TAB_ORPHANS,