Move the list of connected frames to the Redux store

Move metadata about the frames that are connected to the sidebar app,
such as the document's URL and fingerprint, to the central app state
store.

This is part of an effort to unify how important application state is
managed.

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,14 +154,10 @@ 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({
-          uri: info.uri,
-          searchUris: searchUris,
-          documentFingerprint: documentFingerprint,
-        });
+      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(),
@@ -174,11 +175,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 () {
@@ -186,11 +187,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) {