Merge pull request #225 from hypothesis/init-annot-counts

Wait for annotations to be fetched before displaying annotation counts in the frame.

src/annotator/annotation-counts.js

@@ -13,8 +13,6 @@ var ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count';
  */
 
 function annotationCounts(rootEl, crossframe) {
-  updateAnnotationCountElems(0);
-
   crossframe.on(events.PUBLIC_ANNOTATION_COUNT_CHANGED, updateAnnotationCountElems);
 
   function updateAnnotationCountElems(newCount) {

src/sidebar/frame-sync.js

@@ -51,11 +51,13 @@ function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
   function setupSyncToFrame() {
     // List of loaded annotations in previous state
     var prevAnnotations = [];
+    var prevFrames = [];
     var prevPublicAnns = 0;
 
     annotationUI.subscribe(function () {
       var state = annotationUI.getState();
-      if (state.annotations === prevAnnotations) {
+      if (state.annotations === prevAnnotations &&
+          state.frames === prevFrames) {
         return;
       }
 
@@ -82,6 +84,7 @@ function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
         return !inSidebar.has(annot.$tag);
       });
       prevAnnotations = state.annotations;
+      prevFrames = state.frames;
 
       // We currently only handle adding and removing annotations from the frame
       // when they are added or removed in the sidebar, but not re-anchoring
@@ -97,9 +100,14 @@ function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
         inFrame.delete(annot.$tag);
       });
 
-      if (publicAnns !== prevPublicAnns) {
-        bridge.call(bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, publicAnns);
-        prevPublicAnns = publicAnns;
+      var frames = annotationUI.frames();
+      if (frames.length > 0) {
+        if (frames.every(function (frame) { return frame.isAnnotationFetchComplete; })) {
+          if (publicAnns === 0 || publicAnns !== prevPublicAnns) {
+            bridge.call(bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, publicAnns);
+            prevPublicAnns = publicAnns;
+          }
+        }
       }
     });
   }

src/sidebar/reducers/frames.js

@@ -13,6 +13,22 @@ var update = {
   CONNECT_FRAME: function (state, action) {
     return {frames: state.frames.concat(action.frame)};
   },
+
+  UPDATE_FRAME_ANNOTATION_FETCH_STATUS: function (state, action) {
+    var frames = state.frames.map(function (frame) {
+      var match = (frame.uri && frame.uri === action.uri);
+      if (match) {
+        return Object.assign({}, frame, {
+          isAnnotationFetchComplete: action.isAnnotationFetchComplete,
+        });
+      } else {
+        return frame;
+      }
+    });
+    return {
+      frames: frames,
+    };
+  },
 };
 
 var actions = util.actionTypes(update);
@@ -24,6 +40,17 @@ function connectFrame(frame) {
   return {type: actions.CONNECT_FRAME, frame: frame};
 }
 
+/**
+ * Update the `isAnnotationFetchComplete` flag of the frame.
+ */
+function updateFrameAnnotationFetchStatus(uri, status) {
+  return {
+    type: actions.UPDATE_FRAME_ANNOTATION_FETCH_STATUS,
+    isAnnotationFetchComplete: status,
+    uri: uri,
+  };
+}
+
 /**
  * Return the list of frames currently connected to the sidebar app.
  */
@@ -37,6 +64,7 @@ module.exports = {
 
   actions: {
     connectFrame: connectFrame,
+    updateFrameAnnotationFetchStatus: updateFrameAnnotationFetchStatus,
   },
 
   // Selectors

src/sidebar/reducers/test/frames-test.js

@@ -15,4 +15,30 @@ describe('frames reducer', function () {
       assert.deepEqual(frames.frames(state), [frame]);
     });
   });
+
+  describe('#updateFrameAnnotationFetchStatus', function () {
+    it('updates the isAnnotationFetchComplete status of the frame', function () {
+      var frame = {
+        uri: 'http://example.com',
+      };
+      var expectedFrame = {
+        uri: 'http://example.com',
+        isAnnotationFetchComplete: true,
+      };
+      var connectedState = update(init(), actions.connectFrame(frame));
+      var updatedState = update(connectedState,
+        actions.updateFrameAnnotationFetchStatus(frame.uri, true));
+      assert.deepEqual(frames.frames(updatedState), [expectedFrame]);
+    });
+
+    it('does not update the isAnnotationFetchComplete status of the wrong frame', function () {
+      var frame = {
+        uri: 'http://example.com',
+      };
+      var connectedState = update(init(), actions.connectFrame(frame));
+      var updatedState = update(connectedState,
+        actions.updateFrameAnnotationFetchStatus('http://anotherexample.com', true));
+      assert.deepEqual(frames.frames(updatedState), [frame]);
+    });
+  });
 });

src/sidebar/test/frame-sync-test.js

@@ -57,6 +57,7 @@ describe('FrameSync', function () {
       connectFrame: sinon.stub(),
       findIDsForTags: sinon.stub(),
       focusAnnotations: sinon.stub(),
+      frames: sinon.stub().returns([{uri: 'http://example.com', isAnnotationFetchComplete: true }]),
       selectAnnotations: sinon.stub(),
       selectTab: sinon.stub(),
       toggleSelectedAnnotations: sinon.stub(),
@@ -117,15 +118,40 @@ describe('FrameSync', function () {
 
     it('does not send a "loadAnnotations" message for replies', function () {
       fakeAnnotationUI.setState({annotations: [annotationFixtures.newReply()]});
-      assert.notCalled(fakeBridge.call);
+      assert.isFalse(fakeBridge.call.calledWith('loadAnnotations'));
     });
   });
 
   context('when annotation count has changed', function () {
-    it('sends a "publicAnnotationCountChanged" message to the frame', function () {
-      fakeAnnotationUI.setState({annotations: [annotationFixtures.publicAnnotation()]});
+    it('sends a "publicAnnotationCountChanged" message to the frame when there are public annotations', function () {
+      fakeAnnotationUI.setState({
+        annotations: [annotationFixtures.publicAnnotation()],
+      });
       assert.calledWithMatch(fakeBridge.call, 'publicAnnotationCountChanged', sinon.match(1));
     });
+
+    it('sends a "publicAnnotationCountChanged" message to the frame when there are only private annotations', function () {
+      fakeAnnotationUI.setState({
+        annotations: [annotationFixtures.defaultAnnotation()],
+      });
+      assert.calledWithMatch(fakeBridge.call, 'publicAnnotationCountChanged', sinon.match(0));
+    });
+
+    it('does not send a "publicAnnotationCountChanged" message to the frame if annotation fetch is not complete', function () {
+      fakeAnnotationUI.frames.returns([{uri: 'http://example.com'}]);
+      fakeAnnotationUI.setState({
+        annotations: [annotationFixtures.publicAnnotation()],
+      });
+      assert.isFalse(fakeBridge.call.calledWith('publicAnnotationCountChanged'));
+    });
+
+    it('does not send a "publicAnnotationCountChanged" message if there are no connected frames', function () {
+      fakeAnnotationUI.frames.returns([]);
+      fakeAnnotationUI.setState({
+        annotations: [annotationFixtures.publicAnnotation()],
+      });
+      assert.isFalse(fakeBridge.call.calledWith('publicAnnotationCountChanged'));
+    });
   });
 
   context('when annotations are removed from the sidebar', function () {

src/sidebar/test/widget-controller-test.js

@@ -141,6 +141,7 @@ describe('WidgetController', function () {
     $scope.auth = {'status': 'unknown'};
     annotationUI = _annotationUI_;
     annotationUI.frames = sinon.stub().returns([]);
+    annotationUI.updateFrameAnnotationFetchStatus = sinon.stub();
     $controller('WidgetController', {$scope: $scope});
   }));
 
@@ -200,6 +201,17 @@ describe('WidgetController', function () {
       assert.calledWith(loadSpy, [sinon.match({id: uris[1] + '456'})]);
     });
 
+    it('updates annotation fetch status for all frames', function () {
+      var frameUris = ['http://example.com', 'http://foobar.com'];
+      setFrames(frameUris.map(function (frameUri) {
+        return {uri: frameUri, searchUris: [frameUri]};
+      }));
+      $scope.$digest();
+      var updateSpy = annotationUI.updateFrameAnnotationFetchStatus;
+      assert.isTrue(updateSpy.calledWith(frameUris[0], true));
+      assert.isTrue(updateSpy.calledWith(frameUris[1], true));
+    });
+
     context('when there is a selection', function () {
       var uri = 'http://example.com';
       var id = uri + '123';

src/sidebar/widget-controller.js

@@ -135,6 +135,12 @@ module.exports = function WidgetController(
       $scope.$evalAsync(function () {
         searchClients.splice(searchClients.indexOf(searchClient), 1);
       });
+
+      annotationUI.frames().forEach(function (frame) {
+        if (0 <= uris.indexOf(frame.uri)) {
+          annotationUI.updateFrameAnnotationFetchStatus(frame.uri, true);
+        }
+      });
     });
     searchClient.get({uri: uris, group: group});
   }
@@ -155,19 +161,15 @@ module.exports = function WidgetController(
 
   /**
    * Load annotations for all URLs associated with `frames`.
-   *
-   * @param {Array<{uri:string}>} frames - Hypothesis client frames
-   *        to load annotations for.
    */
-  function loadAnnotations(frames, reset) {
-    if (reset || typeof reset === 'undefined') {
-      _resetAnnotations();
-    }
+  function loadAnnotations() {
+    _resetAnnotations();
 
     searchClients.forEach(function (client) {
       client.cancel();
     });
 
+    var frames = annotationUI.frames();
     var searchUris = frames.reduce(function (uris, frame) {
       for (var i = 0; i < frame.searchUris.length; i++) {
         var uri = frame.searchUris[i];
@@ -245,13 +247,15 @@ module.exports = function WidgetController(
       return;
     }
     annotationUI.clearSelectedAnnotations();
-    loadAnnotations(annotationUI.frames());
+    loadAnnotations();
   });
 
   // Watch anything that may require us to reload annotations.
   $scope.$watch(function () {
-    return annotationUI.frames();
-  }, loadAnnotations);
+    return annotationUI.frames().map(function(frame) {
+      return frame.uri;
+    });
+  }, loadAnnotations, true);
 
   $scope.setCollapsed = function (id, collapsed) {
     annotationUI.setCollapsed(id, collapsed);