Merge pull request #180 from hypothesis/move-frame-info-to-store

Move the list of connected frames to the Redux store

h/static/scripts/annotation-ui.js

@@ -38,6 +38,7 @@ var thunk = require('redux-thunk').default;
 
 var reducers = require('./reducers');
 var annotationsReducer = require('./reducers/annotations');
+var framesReducer = require('./reducers/frames');
 var selectionReducer = require('./reducers/selection');
 var sessionReducer = require('./reducers/session');
 var viewerReducer = require('./reducers/viewer');
@@ -93,6 +94,7 @@ module.exports = function ($rootScope, settings) {
   //
   var actionCreators = redux.bindActionCreators(Object.assign({},
     annotationsReducer.actions,
+    framesReducer.actions,
     selectionReducer.actions,
     sessionReducer.actions,
     viewerReducer.actions
@@ -113,6 +115,8 @@ module.exports = function ($rootScope, settings) {
     findIDsForTags: annotationsReducer.findIDsForTags,
     savedAnnotations: annotationsReducer.savedAnnotations,
 
+    frames: framesReducer.frames,
+
     isSidebar: viewerReducer.isSidebar,
   }, store.getState);
 

h/static/scripts/directive/help-panel.js

@@ -11,15 +11,15 @@ module.exports = function () {
     bindToController: true,
     controllerAs: 'vm',
     // @ngInject
-    controller: function ($scope, $window, frameSync, serviceUrl) {
+    controller: function ($scope, $window, annotationUI, serviceUrl) {
       this.userAgent = $window.navigator.userAgent;
       this.version = '__VERSION__';  // replaced by versionify
       this.dateTime = new Date();
       this.serviceUrl = serviceUrl;
 
-      $scope.$watchCollection(
+      $scope.$watch(
         function () {
-          return frameSync.frames;
+          return annotationUI.frames();
         },
         function (frames) {
           if (frames.length === 0) {

h/static/scripts/directive/share-dialog.js

@@ -3,7 +3,7 @@
 var VIA_PREFIX = 'https://via.hypothes.is/';
 
 // @ngInject
-function ShareDialogController($scope, $element, frameSync) {
+function ShareDialogController($scope, $element, annotationUI) {
   var self = this;
 
   function updateViaLink(frames) {
@@ -24,7 +24,7 @@ function ShareDialogController($scope, $element, frameSync) {
   viaInput.focus();
   viaInput.select();
 
-  $scope.$watchCollection(function () { return frameSync.frames; },
+  $scope.$watch(function () { return annotationUI.frames(); },
     updateViaLink);
 }
 

h/static/scripts/directive/test/share-dialog-test.js

@@ -5,28 +5,30 @@ var angular = require('angular');
 var util = require('./util');
 
 describe('shareDialog', function () {
-  var fakeFrameSync;
+  var fakeAnnotationUI;
 
   beforeEach(function () {
-    fakeFrameSync = { frames: [] };
+    fakeAnnotationUI = { frames: sinon.stub().returns([]) };
 
     angular.module('h', [])
       .directive('shareDialog', require('../share-dialog'))
-      .value('frameSync', fakeFrameSync)
+      .value('annotationUI', fakeAnnotationUI)
       .value('urlEncodeFilter', function (val) { return val; });
     angular.mock.module('h');
   });
 
   it('generates new via link', function () {
     var element = util.createDirective(document, 'shareDialog', {});
-    fakeFrameSync.frames.push({ uri: 'http://example.com' });
+    fakeAnnotationUI.frames.returns([{ uri: 'http://example.com' }]);
     element.scope.$digest();
     assert.equal(element.ctrl.viaPageLink, 'https://via.hypothes.is/http://example.com');
   });
 
   it('does not generate new via link if already on via', function () {
     var element = util.createDirective(document, 'shareDialog', {});
-    fakeFrameSync.frames.push({ uri: 'https://via.hypothes.is/http://example.com' });
+    fakeAnnotationUI.frames.returns([{
+      uri: 'https://via.hypothes.is/http://example.com',
+    }]);
     element.scope.$digest();
     assert.equal(element.ctrl.viaPageLink, 'https://via.hypothes.is/http://example.com');
   });

h/static/scripts/frame-sync.js

@@ -40,9 +40,6 @@ function formatAnnot(ann) {
 // @ngInject
 function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
 
-  // List of frames currently connected to the sidebar
-  var frames = [];
-
   // Set of tags of annotations that are currently loaded into the frame
   var inFrame = new Set();
 
@@ -157,16 +154,12 @@ function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
         });
       }
 
-      // The `frames` list is currently stored by this service but should in
-      // future be moved to the app state.
-      $rootScope.$apply(function () {
-        frames.push({
+      annotationUI.connectFrame({
         uri: info.uri,
         searchUris: searchUris,
         documentFingerprint: documentFingerprint,
       });
     });
-    });
   }
 
   /**
@@ -201,12 +194,6 @@ function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
   this.scrollToAnnotation = function (tag) {
     bridge.call('scrollToAnnotation', tag);
   };
-
-  /**
-   * List of frames that are connected to the app.
-   * @type {FrameInfo}
-   */
-  this.frames = frames;
 }
 
 module.exports = {

h/static/scripts/reducers/frames.js

+'use strict';
+
+var util = require('./util');
+
+function init() {
+  return {
+    // The list of frames connected to the sidebar app
+    frames: [],
+  };
+}
+
+var update = {
+  CONNECT_FRAME: function (state, action) {
+    return {frames: state.frames.concat(action.frame)};
+  },
+};
+
+var actions = util.actionTypes(update);
+
+/**
+ * Add a frame to the list of frames currently connected to the sidebar app.
+ */
+function connectFrame(frame) {
+  return {type: actions.CONNECT_FRAME, frame: frame};
+}
+
+/**
+ * Return the list of frames currently connected to the sidebar app.
+ */
+function frames(state) {
+  return state.frames;
+}
+
+module.exports = {
+  init: init,
+  update: update,
+
+  actions: {
+    connectFrame: connectFrame,
+  },
+
+  // Selectors
+  frames: frames,
+};

h/static/scripts/reducers/index.js

@@ -18,6 +18,7 @@
  */
 
 var annotations = require('./annotations');
+var frames = require('./frames');
 var selection = require('./selection');
 var session = require('./session');
 var viewer = require('./viewer');
@@ -27,6 +28,7 @@ function init(settings) {
   return Object.assign(
     {},
     annotations.init(),
+    frames.init(),
     selection.init(settings),
     session.init(),
     viewer.init()
@@ -35,6 +37,7 @@ function init(settings) {
 
 var update = util.createReducer(Object.assign(
   annotations.update,
+  frames.update,
   selection.update,
   session.update,
   viewer.update

h/static/scripts/reducers/test/frames-test.js

+'use strict';
+
+var frames = require('../frames');
+var util = require('../util');
+
+var init = frames.init;
+var actions = frames.actions;
+var update = util.createReducer(frames.update);
+
+describe('frames reducer', function () {
+  describe('#connectFrame', function () {
+    it('adds the frame to the list of connected frames', function () {
+      var frame = {uri: 'http://example.com'};
+      var state = update(init(), actions.connectFrame(frame));
+      assert.deepEqual(frames.frames(state), [frame]);
+    });
+  });
+});

h/static/scripts/test/frame-sync-test.js

@@ -54,6 +54,7 @@ describe('FrameSync', function () {
 
   beforeEach(function () {
     fakeAnnotationUI = fakeStore({annotations: []}, {
+      connectFrame: sinon.stub(),
       findIDsForTags: sinon.stub(),
       focusAnnotations: sinon.stub(),
       selectAnnotations: sinon.stub(),
@@ -173,11 +174,11 @@ describe('FrameSync', function () {
 
       fakeBridge.emit('connect', fakeChannel);
 
-      assert.deepEqual(frameSync.frames, [{
+      assert.calledWith(fakeAnnotationUI.connectFrame, {
         documentFingerprint: undefined,
         searchUris: [frameInfo.uri],
         uri: frameInfo.uri,
-      }]);
+      });
     });
 
     it('adds the document fingerprint for PDFs', function () {
@@ -185,11 +186,11 @@ describe('FrameSync', function () {
 
       fakeBridge.emit('connect', fakeChannel);
 
-      assert.deepEqual(frameSync.frames, [{
+      assert.calledWith(fakeAnnotationUI.connectFrame, {
         documentFingerprint: frameInfo.metadata.documentFingerprint,
         searchUris: [frameInfo.uri, 'urn:1234'],
         uri: frameInfo.uri,
-      }]);
+      });
     });
   });
 

h/static/scripts/test/widget-controller-test.js

@@ -77,7 +77,6 @@ describe('WidgetController', function () {
     fakeFrameSync = {
       focusAnnotations: sinon.stub(),
       scrollToAnnotation: sinon.stub(),
-      frames: [],
     };
 
     fakeDrafts = {
@@ -125,12 +124,16 @@ describe('WidgetController', function () {
     $provide.value('settings', fakeSettings);
   }));
 
+  function setFrames(frames) {
+    annotationUI.frames.returns(frames);
+  }
 
   beforeEach(angular.mock.inject(function ($controller, _annotationUI_, _$rootScope_) {
     $rootScope = _$rootScope_;
     $scope = $rootScope.$new();
     $scope.auth = {'status': 'unknown'};
     annotationUI = _annotationUI_;
+    annotationUI.frames = sinon.stub().returns([]);
     $controller('WidgetController', {$scope: $scope});
   }));
 
@@ -144,11 +147,13 @@ describe('WidgetController', function () {
       // before reloading annotations for each currently-connected client
       annotationUI.addAnnotations([{id: '123'}]);
       var uri1 = 'http://example.com/page-a';
-      fakeFrameSync.frames.push({uri: uri1, searchUris: [uri1]});
+      var frames = [{uri: uri1, searchUris: [uri1]}];
+      setFrames(frames);
       $scope.$digest();
       fakeAnnotationMapper.unloadAnnotations = sandbox.spy();
       var uri2 = 'http://example.com/page-b';
-      fakeFrameSync.frames.push({uri: uri2, searchUris: [uri2]});
+      frames = frames.concat({uri: uri2, searchUris: [uri2]});
+      setFrames(frames);
       $scope.$digest();
       assert.calledWith(fakeAnnotationMapper.unloadAnnotations,
         annotationUI.getState().annotations);
@@ -156,7 +161,7 @@ describe('WidgetController', function () {
 
     it('loads all annotations for a frame', function () {
       var uri = 'http://example.com';
-      fakeFrameSync.frames.push({uri: uri, searchUris: [uri]});
+      setFrames([{uri: uri, searchUris: [uri]}]);
       $scope.$digest();
       var loadSpy = fakeAnnotationMapper.loadAnnotations;
       assert.calledWith(loadSpy, [sinon.match({id: uri + '123'})]);
@@ -166,7 +171,7 @@ describe('WidgetController', function () {
     it('loads all annotations for a frame with multiple urls', function () {
       var uri = 'http://example.com/test.pdf';
       var fingerprint = 'urn:x-pdf:fingerprint';
-      fakeFrameSync.frames.push({uri: uri, searchUris: [uri, fingerprint]});
+      setFrames([{uri: uri, searchUris: [uri, fingerprint]}]);
       $scope.$digest();
       var loadSpy = fakeAnnotationMapper.loadAnnotations;
       assert.calledWith(loadSpy, [sinon.match({id: uri + '123'})]);
@@ -177,9 +182,9 @@ describe('WidgetController', function () {
 
     it('loads all annotations for all frames', function () {
       var uris = ['http://example.com', 'http://foobar.com'];
-      fakeFrameSync.frames = uris.map(function (uri) {
+      setFrames(uris.map(function (uri) {
         return {uri: uri, searchUris: [uri]};
-      });
+      }));
       $scope.$digest();
       var loadSpy = fakeAnnotationMapper.loadAnnotations;
       assert.calledWith(loadSpy, [sinon.match({id: uris[0] + '123'})]);
@@ -193,7 +198,7 @@ describe('WidgetController', function () {
       var id = uri + '123';
 
       beforeEach(function () {
-        fakeFrameSync.frames = [{uri: uri, searchUris: [uri]}];
+        setFrames([{uri: uri, searchUris: [uri]}]);
         annotationUI.selectAnnotations([id]);
         $scope.$digest();
       });
@@ -223,7 +228,7 @@ describe('WidgetController', function () {
       var uri = 'http://example.com';
 
       beforeEach(function () {
-        fakeFrameSync.frames = [{uri: uri, searchUris: [uri]}];
+        setFrames([{uri: uri, searchUris: [uri]}]);
         fakeGroups.focused = function () { return { id: 'a-group' }; };
         $scope.$digest();
       });
@@ -246,7 +251,7 @@ describe('WidgetController', function () {
       var id = uri + 'does-not-exist';
 
       beforeEach(function () {
-        fakeFrameSync.frames = [{uri: uri, searchUris: [uri]}];
+        setFrames([{uri: uri, searchUris: [uri]}]);
         annotationUI.selectAnnotations([id]);
         fakeGroups.focused = function () { return { id: 'private-group' }; };
         $scope.$digest();
@@ -264,7 +269,7 @@ describe('WidgetController', function () {
     it('focuses and scrolls to the annotation if already selected', function () {
       var uri = 'http://example.com';
       annotationUI.selectAnnotations(['123']);
-      fakeFrameSync.frames.push({uri: uri, searchUris: [uri]});
+      setFrames([{uri: uri, searchUris: [uri]}]);
       var annot = {
         $tag: 'atag',
         id: '123',
@@ -283,7 +288,7 @@ describe('WidgetController', function () {
       annotationUI.addAnnotations([{id: '123'}]);
       annotationUI.addAnnotations = sinon.stub();
       fakeDrafts.unsaved.returns([{id: uri + '123'}, {id: uri + '456'}]);
-      fakeFrameSync.frames.push({uri: uri, searchUris: [uri]});
+      setFrames([{uri: uri, searchUris: [uri]}]);
       var loadSpy = fakeAnnotationMapper.loadAnnotations;
 
       $scope.$broadcast(events.GROUP_FOCUSED);
@@ -300,12 +305,12 @@ describe('WidgetController', function () {
 
     beforeEach(function () {
       // The document has finished loading.
-      fakeFrameSync.frames = [
+      setFrames([
         {
           uri: 'http://www.example.com',
           searchUris: [],
         },
-      ];
+      ]);
 
       // There is a direct-linked annotation
       fakeSettings.annotations = 'test';
@@ -336,7 +341,7 @@ describe('WidgetController', function () {
       // There is a selection but the selected annotation isn't available.
       annotationUI.selectAnnotations(['missing']);
       // The document hasn't finished loading.
-      fakeFrameSync.frames = [];
+      setFrames([]);
       $scope.$digest();
 
       assert.isFalse($scope.selectedAnnotationUnavailable());

h/static/scripts/widget-controller.js

@@ -160,7 +160,7 @@ module.exports = function WidgetController(
   }
 
   function isLoading() {
-    if (!frameSync.frames.some(function (frame) { return frame.uri; })) {
+    if (!annotationUI.frames().some(function (frame) { return frame.uri; })) {
       // The document's URL isn't known so the document must still be loading.
       return true;
     }
@@ -262,12 +262,12 @@ module.exports = function WidgetController(
       return;
     }
     annotationUI.clearSelectedAnnotations();
-    loadAnnotations(frameSync.frames);
+    loadAnnotations(annotationUI.frames());
   });
 
   // Watch anything that may require us to reload annotations.
-  $scope.$watchCollection(function () {
-    return frameSync.frames;
+  $scope.$watch(function () {
+    return annotationUI.frames();
   }, loadAnnotations);
 
   $scope.setCollapsed = function (id, collapsed) {