Merge pull request #3284 from hypothesis/316-annotation-thread-view

New threading 3/N - Render annotation threads using new threading infrastructure

h/static/scripts/annotation-mapper.js

@@ -4,37 +4,26 @@ var angular = require('angular');
 
 var events = require('./events');
 
-// Fetch the container object for the passed annotation from the threading
-// service, but only return it if it has an associated message.
-function getContainer(threading, annotation) {
-  var container = threading.idTable[annotation.id];
-  if (container === null || typeof container === 'undefined') {
-    return null;
-  }
-  // Also return null if the container has no message
-  if (!container.message) {
-    return null;
-  }
-  return container;
+function getExistingAnnotation(annotationUI, id) {
+  return annotationUI.getState().annotations.find(function (annot) {
+    return annot.id === id;
+  });
 }
 
-
 // Wraps the annotation store to trigger events for the CRUD actions
 // @ngInject
-function annotationMapper($rootScope, threading, store) {
+function annotationMapper($rootScope, annotationUI, store) {
   function loadAnnotations(annotations, replies) {
     annotations = annotations.concat(replies || []);
 
     var loaded = [];
-
     annotations.forEach(function (annotation) {
-      var container = getContainer(threading, annotation);
-      if (container !== null) {
-        angular.copy(annotation, container.message);
-        $rootScope.$emit(events.ANNOTATION_UPDATED, container.message);
+      var existing = getExistingAnnotation(annotationUI, annotation.id);
+      if (existing) {
+        angular.copy(annotation, existing);
+        $rootScope.$emit(events.ANNOTATION_UPDATED, existing);
         return;
       }
-
       loaded.push(new store.AnnotationResource(annotation));
     });
 
@@ -43,9 +32,9 @@ function annotationMapper($rootScope, threading, store) {
 
   function unloadAnnotations(annotations) {
     var unloaded = annotations.map(function (annotation) {
-      var container = getContainer(threading, annotation);
-      if (container !== null && annotation !== container.message) {
-        annotation = angular.copy(annotation, container.message);
+      var existing = getExistingAnnotation(annotationUI, annotation.id);
+      if (existing && annotation !== existing) {
+        annotation = angular.copy(annotation, existing);
       }
       return annotation;
     });

h/static/scripts/annotation-ui.js

@@ -49,6 +49,10 @@ function initialState(settings) {
     // Set of IDs of annotations that have been explicitly shown
     // by the user even if they do not match the current search filter
     forceVisible: {},
+
+    filterQuery: null,
+
+    sortMode: 'Location',
   });
 }
 
@@ -61,6 +65,8 @@ var types = {
   ADD_ANNOTATIONS: 'ADD_ANNOTATIONS',
   REMOVE_ANNOTATIONS: 'REMOVE_ANNOTATIONS',
   CLEAR_ANNOTATIONS: 'CLEAR_ANNOTATIONS',
+  SET_FILTER_QUERY: 'SET_FILTER_QUERY',
+  SORT_BY: 'SORT_BY',
 };
 
 function excludeAnnotations(current, annotations) {
@@ -104,6 +110,14 @@ function reducer(state, action) {
       return Object.assign({}, state, {forceVisible: action.forceVisible});
     case types.SET_EXPANDED:
       return Object.assign({}, state, {expanded: action.expanded});
+    case types.SET_FILTER_QUERY:
+      return Object.assign({}, state, {
+        filterQuery: action.query,
+        forceVisible: {},
+        expanded: {},
+      });
+    case types.SORT_BY:
+      return Object.assign({}, state, {sortMode: action.mode});
     default:
       return state;
   }
@@ -204,17 +218,6 @@ module.exports = function (settings) {
       });
     },
 
-    /**
-     * Clear the set of annotations which have been explicitly shown by
-     * setForceVisible()
-     */
-    clearForceVisible: function () {
-      store.dispatch({
-        type: types.SET_FORCE_VISIBLE,
-        forceVisible: {},
-      });
-    },
-
     /**
      * Returns true if the annotation with the given `id` is selected.
      */
@@ -289,5 +292,21 @@ module.exports = function (settings) {
     clearAnnotations: function () {
       store.dispatch({type: types.CLEAR_ANNOTATIONS});
     },
+
+    /** Set the query used to filter displayed annotations. */
+    setFilterQuery: function (query) {
+      store.dispatch({
+        type: types.SET_FILTER_QUERY,
+        query: query,
+      });
+    },
+
+    /** Sets the sort mode for the annotation list. */
+    sortBy: function (mode) {
+      store.dispatch({
+        type: types.SORT_BY,
+        mode: mode,
+      });
+    },
   };
 };

h/static/scripts/annotation-viewer-controller.js

@@ -5,7 +5,7 @@ var angular = require('angular');
 // @ngInject
 function AnnotationViewerController (
   $location, $routeParams, $scope,
-  streamer, store, streamFilter, annotationMapper, threading
+  annotationUI, rootThread, streamer, store, streamFilter, annotationMapper
 ) {
   var id = $routeParams.id;
 
@@ -17,9 +17,16 @@ function AnnotationViewerController (
     $location.path('/stream').search('q', query);
   };
 
+  $scope.rootThread = function () {
+    return rootThread.thread();
+  };
+
+  $scope.setCollapsed = function (id, collapsed) {
+    annotationUI.setCollapsed(id, collapsed);
+  };
+
   store.AnnotationResource.get({ id: id }, function (annotation) {
     annotationMapper.loadAnnotations([annotation]);
-    $scope.threadRoot = { children: [threading.idTable[id]] };
   });
 
   store.SearchResource.get({ references: id }, function (data) {

h/static/scripts/app.js

@@ -25,7 +25,6 @@ if (settings.raven) {
   angular.module('ngRaven', []);
 }
 
-var mail = require('./vendor/jwz');
 var streamer = require('./streamer');
 
 // Fetch external state that the app needs before it can run. This includes the
@@ -40,20 +39,6 @@ var resolve = {
     return store.$promise;
   },
   streamer: streamer.connect,
-  // @ngInject
-  threading: function (annotationMapper, drafts, threading) {
-    // Unload all the annotations
-    annotationMapper.unloadAnnotations(threading.annotationList());
-
-    // Reset the threading root
-    threading.createIdTable([]);
-    threading.root = mail.messageContainer();
-
-    // Reload all new, unsaved annotations
-    threading.thread(drafts.unsaved());
-
-    return threading;
-  },
 };
 
 // @ngInject
@@ -117,15 +102,6 @@ function processAppOpts() {
   }
 }
 
-// @ngInject
-function setupTemplateCache($templateCache) {
-  // 'thread.html' is used via ng-include so it needs to be added
-  // to the template cache. Other components just require() templates
-  // directly as strings.
-  $templateCache.put('thread.html',
-    require('../../templates/client/thread.html'));
-}
-
 module.exports = angular.module('h', [
   // Angular addons which export the Angular module name
   // via module.exports
@@ -155,7 +131,7 @@ module.exports = angular.module('h', [
   .directive('aboutThisVersionDialog', require('./directive/about-this-version-dialog'))
   .directive('annotation', require('./directive/annotation').directive)
   .directive('annotationShareDialog', require('./directive/annotation-share-dialog'))
-  .directive('deepCount', require('./directive/deep-count'))
+  .directive('annotationThread', require('./directive/annotation-thread'))
   .directive('dropdownMenuBtn', require('./directive/dropdown-menu-btn'))
   .directive('excerpt', require('./directive/excerpt').directive)
   .directive('feedbackLink', require('./directive/feedback-link'))
@@ -176,8 +152,6 @@ module.exports = angular.module('h', [
   .directive('sortDropdown', require('./directive/sort-dropdown'))
   .directive('spinner', require('./directive/spinner'))
   .directive('statusButton', require('./directive/status-button'))
-  .directive('thread', require('./directive/thread'))
-  .directive('threadFilter', require('./directive/thread-filter'))
   .directive('topBar', require('./directive/top-bar'))
   .directive('windowScroll', require('./directive/window-scroll'))
 
@@ -195,12 +169,11 @@ module.exports = angular.module('h', [
   .service('localStorage', require('./local-storage'))
   .service('permissions', require('./permissions'))
   .service('queryParser', require('./query-parser'))
-  .service('render', require('./render'))
+  .service('rootThread', require('./root-thread'))
   .service('searchFilter', require('./search-filter'))
   .service('session', require('./session'))
   .service('streamFilter', require('./stream-filter'))
   .service('tags', require('./tags'))
-  .service('threading', require('./threading'))
   .service('unicode', require('./unicode'))
   .service('viewFilter', require('./view-filter'))
 
@@ -219,7 +192,6 @@ module.exports = angular.module('h', [
   .config(configureRoutes)
 
   .run(setupCrossFrame)
-  .run(setupHttp)
-  .run(setupTemplateCache);
+  .run(setupHttp);
 
 processAppOpts();

h/static/scripts/build-thread.js

@@ -210,14 +210,6 @@ function hasVisibleChildren(thread) {
   });
 }
 
-
-function hasSelectedChildren(thread, selected) {
-  return thread.children.some(function (child) {
-    return selected.indexOf(child.id) !== -1 ||
-           hasSelectedChildren(child, selected);
-  });
-}
-
 /**
  * Default options for buildThread()
  */
@@ -279,16 +271,22 @@ function buildThread(annotations, opts) {
     if (opts.forceVisible && opts.forceVisible.indexOf(id(annotation)) !== -1) {
       return true;
     }
-    if (opts.selected.length > 0 &&
-        opts.selected.indexOf(id(annotation)) === -1) {
-      return false;
-    }
     if (opts.filterFn && !opts.filterFn(annotation)) {
       return false;
     }
     return true;
   };
 
+  // When there is a selection, only include top-level threads (annotations)
+  // that are selected
+  if (opts.selected.length > 0) {
+    thread = Object.assign({}, thread, {
+      children: thread.children.filter(function (child) {
+        return opts.selected.indexOf(child.id) !== -1;
+      }),
+    });
+  }
+
   // Set the visibility of threads based on whether they match
   // the current search filter
   thread = mapThread(thread, function (thread) {
@@ -301,8 +299,7 @@ function buildThread(annotations, opts) {
 
   // Expand any threads which:
   // 1) Have been explicitly expanded OR
-  // 2) Have children matching the filter OR
-  // 3) Contain children which have been selected
+  // 2) Have children matching the filter
   thread = mapThread(thread, function (thread) {
     var id = thread.id;
 
@@ -315,8 +312,7 @@ function buildThread(annotations, opts) {
 
     return Object.assign({}, thread, {
       collapsed: thread.collapsed &&
-                 !hasUnfilteredChildren &&
-                 !hasSelectedChildren(thread, opts.selected)
+                 !hasUnfilteredChildren,
     });
   });
 

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

+'use strict';
+
+function hiddenCount(thread) {
+  var isHidden = thread.annotation && !thread.visible;
+  return thread.children.reduce(function (count, reply) {
+    return count + hiddenCount(reply);
+  }, isHidden ? 1 : 0);
+}
+
+function visibleCount(thread) {
+  var isVisible = thread.annotation && thread.visible;
+  return thread.children.reduce(function (count, reply) {
+    return count + visibleCount(reply);
+  }, isVisible ? 1 : 0);
+}
+
+function showAllChildren(thread, showFn) {
+  thread.children.forEach(function (child) {
+    showFn({thread: child});
+    showAllChildren(child, showFn);
+  });
+}
+
+function showAllParents(thread, showFn) {
+  while (thread.parent && thread.parent.annotation) {
+    showFn({thread: thread.parent});
+    thread = thread.parent;
+  }
+}
+
+// @ngInject
+function AnnotationThreadController() {
+  this.toggleCollapsed = function () {
+    this.onChangeCollapsed({
+      id: this.thread.id,
+      collapsed: !this.thread.collapsed,
+    });
+  };
+
+  /**
+   * Show this thread and any of its children
+   */
+  this.showThreadAndReplies = function () {
+    showAllParents(this.thread, this.onForceVisible);
+    this.onForceVisible({thread: this.thread});
+    showAllChildren(this.thread, this.onForceVisible);
+  };
+
+  /**
+   * Return the total number of annotations in the current
+   * thread which have been hidden because they do not match the current
+   * search filter.
+   */
+  this.hiddenCount = function () {
+    return hiddenCount(this.thread);
+  };
+
+  this.shouldShowReply = function (child) {
+    return visibleCount(child) > 0;
+  };
+}
+
+/**
+ * Renders a thread of annotations.
+ */
+module.exports = function () {
+  return {
+    restrict: 'E',
+    bindToController: true,
+    controllerAs: 'vm',
+    controller: AnnotationThreadController,
+    scope: {
+      /** The annotation thread to render. */
+      thread: '<',
+      /**
+       * Specify whether document information should be shown
+       * on annotation cards.
+       */
+      showDocumentInfo: '<',
+      /** Called when the user clicks on the expand/collapse replies toggle. */
+      onChangeCollapsed: '&',
+      /**
+       * Called when the user clicks the button to show this thread or
+       * one of its replies.
+       */
+      onForceVisible: '&',
+    },
+    template: require('../../../templates/client/annotation_thread.html'),
+  };
+};

h/static/scripts/directive/annotation.js

@@ -721,51 +721,7 @@ function AnnotationController(
 
 function link(scope, elem, attrs, controllers) {
   var ctrl = controllers[0];
-  var thread = controllers[1];
-  var threadFilter = controllers[2];
-  var counter = controllers[3];
-
   elem.on('keydown', ctrl.onKeydown);
-
-  // FIXME: Replace this counting code with something more sane.
-  if (counter !== null) {
-    scope.$watch((function() {return ctrl.editing();}), function(editing, old) {
-      if (editing) {  // The user has just started editing this annotation.
-
-        // Keep track of edits going on in the thread.
-        // This 'edit' count is for example to uncollapse a thread if one of
-        // the replies in the thread is currently being edited when the
-        // annotations are first rendered (this can happen when switching
-        // focus to a different group then back again, for example).
-        counter.count('edit', 1);
-
-        // Always show an annotation if it is being edited, even if there's an
-        // active search filter that does not match the annotation.
-        if ((thread !== null) && (threadFilter !== null)) {
-          threadFilter.active(false);
-          threadFilter.freeze(true);
-        }
-
-      } else if (old) {  // The user has just finished editing this annotation.
-        counter.count('edit', -1);
-        if (threadFilter) {
-          threadFilter.freeze(false);
-        }
-
-        // Ensure that the just-edited annotation remains visible.
-        if (thread.parent) {
-          thread.parent.toggleCollapsed(false);
-        }
-
-      }
-    });
-
-    scope.$on('$destroy', function() {
-      if (ctrl.editing() && counter) {
-        counter.count('edit', -1);
-      }
-    });
-  }
 }
 
 /**
@@ -785,12 +741,12 @@ function annotation() {
     controller: AnnotationController,
     controllerAs: 'vm',
     link: link,
-    require: ['annotation', '?^thread', '?^threadFilter', '?^deepCount'],
+    require: ['annotation'],
     scope: {
       annotation: '<',
       // Indicates whether this is the last reply in a thread.
       isLastReply: '<',
-      isSidebar: '<',
+      showDocumentInfo: '<',
       onReplyCountClick: '&',
       replyCount: '<',
       isCollapsed: '<'

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

@@ -182,104 +182,6 @@ describe('annotation', function() {
           'keydown', mockAnnotationController.onKeydown)
       );
     });
-
-    it('increments the "edit" count when editing() becomes true', function () {
-      link(scope, mockElement, mockAttributes, mockControllers);
-
-      mockAnnotationController.editing.returns(true);
-      scope.$digest();
-
-      assert.equal(true, mockDeepCountController.count.calledOnce);
-      assert.equal(
-        true, mockDeepCountController.count.calledWithExactly('edit', 1)
-      );
-    });
-
-    it('decrements the "edit" count when editing() turns false', function () {
-      mockAnnotationController.editing.returns(true);
-      link(scope, mockElement, mockAttributes, mockControllers);
-      scope.$digest();
-
-      mockAnnotationController.editing.returns(false);
-      scope.$digest();
-
-      assert.equal(
-        true,
-        mockDeepCountController.count.lastCall.calledWithExactly('edit', -1)
-      );
-    });
-
-    it('decrements the edit count when destroyed while editing', function () {
-      mockAnnotationController.editing.returns(true);
-      link(scope, mockElement, mockAttributes, mockControllers);
-
-      scope.$destroy();
-
-      assert.equal(1, mockDeepCountController.count.callCount);
-      assert.equal(
-        true, mockDeepCountController.count.calledWithExactly('edit', -1)
-      );
-    });
-
-    it('does not decrement the edit count when destroyed while not editing',
-      function () {
-        mockAnnotationController.editing.returns(false);
-        link(scope, mockElement, mockAttributes, mockControllers);
-
-        scope.$destroy();
-
-        assert.equal(0, mockDeepCountController.count.callCount);
-      }
-    );
-
-    it('deactivates the thread filter when editing() turns true', function () {
-      mockAnnotationController.editing.returns(false);
-      link(scope, mockElement, mockAttributes, mockControllers);
-
-      mockAnnotationController.editing.returns(true);
-      scope.$digest();
-
-      assert.equal(1, mockThreadFilterController.active.callCount);
-      assert.equal(
-        true, mockThreadFilterController.active.calledWithExactly(false));
-    });
-
-    it('freezes the thread filter when editing', function () {
-      mockAnnotationController.editing.returns(false);
-      link(scope, mockElement, mockAttributes, mockControllers);
-
-      mockAnnotationController.editing.returns(true);
-      scope.$digest();
-
-      assert.equal(1, mockThreadFilterController.freeze.callCount);
-      assert.equal(
-        true, mockThreadFilterController.freeze.calledWithExactly(true));
-    });
-
-    it('unfreezes the thread filter when editing becomes false', function () {
-      mockAnnotationController.editing.returns(true);
-      link(scope, mockElement, mockAttributes, mockControllers);
-      scope.$digest();
-
-      mockAnnotationController.editing.returns(false);
-      scope.$digest();
-
-      assert.equal(
-        true,
-        mockThreadFilterController.freeze.lastCall.calledWithExactly(false));
-    });
-
-    it('does not collapse parent thread after edit', function() {
-      mockAnnotationController.editing.returns(true);
-      link(scope, mockElement, mockAttributes, mockControllers);
-      scope.$digest();
-
-      mockAnnotationController.editing.returns(false);
-      scope.$digest();
-
-      assert.calledWith(mockThreadController.parent.toggleCollapsed, false);
-    });
-
   });
 
   describe('AnnotationController', function() {

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

+'use strict';
+
+var angular = require('angular');
+
+var annotationThread = require('../annotation-thread');
+var util = require('./util');
+
+function PageObject(element) {
+  this.annotations = function () {
+    return Array.from(element[0].querySelectorAll('annotation'));
+  };
+  this.visibleReplies = function () {
+    return Array.from(element[0].querySelectorAll('.thread:not(.ng-hide)'));
+  };
+  this.replyList = function () {
+    return element[0].querySelector('.thread-replies');
+  };
+  this.isHidden = function (element) {
+    return element.classList.contains('ng-hide');
+  };
+}
+
+describe('annotationThread', function () {
+  before(function () {
+    angular.module('app', [])
+      .directive('annotationThread', annotationThread);
+  });
+
+  beforeEach(function () {
+    angular.mock.module('app');
+  });
+
+  it('renders the tree structure of parent and child annotations', function () {
+    var element = util.createDirective(document, 'annotationThread', {
+      thread: {
+        id: '1',
+        annotation: {id: '1', text: 'text'},
+        children: [{
+          id: '2',
+          annotation: {id: '2', text: 'areply'},
+          children: [],
+          visible: true,
+        }],
+        visible: true,
+      },
+    });
+    var pageObject = new PageObject(element);
+    assert.equal(pageObject.annotations().length, 2);
+    assert.equal(pageObject.visibleReplies().length, 1);
+  });
+
+  it('does not render hidden threads', function () {
+    var element = util.createDirective(document, 'annotationThread', {
+      thread: {
+        id: '1',
+        annotation: {id: '1'},
+        visible: false,
+        children: []
+      }
+    });
+    var pageObject = new PageObject(element);
+    assert.equal(pageObject.annotations().length, 1);
+    assert.isTrue(pageObject.isHidden(pageObject.annotations()[0]));
+  });
+
+  it('shows replies if not collapsed', function () {
+    var element = util.createDirective(document, 'annotationThread', {
+      thread: {
+        id: '1',
+        annotation: {id: '1'},
+        visible: true,
+        children: [{
+          id: '2',
+          annotation: {id: '2'},
+          children: [],
+          visible: true,
+        }],
+        collapsed: false,
+      }
+    });
+    var pageObject = new PageObject(element);
+    assert.isFalse(pageObject.isHidden(pageObject.replyList()));
+  });
+
+  it('does not show replies if collapsed', function () {
+    var element = util.createDirective(document, 'annotationThread', {
+      thread: {
+        id: '1',
+        annotation: {id: '1'},
+        visible: true,
+        children: [{
+          id: '2',
+          annotation: {id: '2'},
+          children: [],
+          visible: true,
+        }],
+        collapsed: true,
+      }
+    });
+    var pageObject = new PageObject(element);
+    assert.isTrue(pageObject.isHidden(pageObject.replyList()));
+  });
+
+  it('only shows replies that match the search filter', function () {
+    var element = util.createDirective(document, 'annotationThread', {
+      thread: {
+        id: '1',
+        annotation: {id: '1'},
+        visible: true,
+        children: [{
+          id: '2',
+          annotation: {id: '2'},
+          children: [],
+          visible: false,
+        },{
+          id: '3',
+          annotation: {id: '3'},
+          children: [],
+          visible: true,
+        }],
+        collapsed: false,
+      }
+    });
+    var pageObject = new PageObject(element);
+    assert.equal(pageObject.visibleReplies().length, 1);
+  });
+
+  describe('#toggleCollapsed', function () {
+    it('toggles replies', function () {
+      var onChangeCollapsed = sinon.stub();
+      var element = util.createDirective(document, 'annotationThread', {
+        thread: {
+          id: '123',
+          annotation: {id: '123'},
+          children: [],
+          collapsed: true,
+        },
+        onChangeCollapsed: {
+          args: ['id', 'collapsed'],
+          callback: onChangeCollapsed,
+        }
+      });
+      element.ctrl.toggleCollapsed();
+      assert.calledWith(onChangeCollapsed, '123', false);
+    });
+  });
+
+  describe('#showThreadAndReplies', function () {
+    it('reveals all parents and replies', function () {
+      var onForceVisible = sinon.stub();
+      var thread = {
+        id: '123',
+        annotation: {id: '123'},
+        children: [{
+          id: 'child-id',
+          annotation: {id: 'child-id'},
+          children: [],
+        }],
+        parent: {
+          id: 'parent-id',
+          annotation: {id: 'parent-id'},
+        },
+      };
+      var element = util.createDirective(document, 'annotationThread', {
+        thread: thread,
+        onForceVisible: {
+          args: ['thread'],
+          callback: onForceVisible,
+        },
+      });
+      element.ctrl.showThreadAndReplies();
+      assert.calledWith(onForceVisible, thread.parent);
+      assert.calledWith(onForceVisible, thread);
+      assert.calledWith(onForceVisible, thread.children[0]);
+    });
+  });
+});

h/static/scripts/karma.config.js

@@ -46,6 +46,7 @@ module.exports = function(config) {
       './test/bootstrap.js': ['browserify'],
       '**/*-test.js': ['browserify'],
       '**/*-test.coffee': ['browserify'],
+      '**/*-it.js': ['browserify'],
     },
 
     browserify: {

h/static/scripts/root-thread.js

@@ -27,11 +27,12 @@ var sortFns = {
 /**
  * Root conversation thread for the sidebar and stream.
  *
- * Listens for annotations being loaded, created and unloaded and
- * builds a conversation thread.
+ * This performs two functions:
  *
- * The thread is sorted and filtered according to
- * current sort and filter settings.
+ * 1. It listens for annotations being loaded, created and unloaded and
+ *    dispatches annotationUI.{addAnnotations|removeAnnotations} actions.
+ * 2. Listens for changes in the UI state and rebuilds the root conversation
+ *    thread.
  *
  * The root thread is then displayed by viewer.html
  */
@@ -39,24 +40,23 @@ var sortFns = {
 module.exports = function ($rootScope, annotationUI, searchFilter, viewFilter) {
   var thread;
 
-  var sortFn = sortFns.Location;
-  var searchQuery;
-
   /**
    * Rebuild the root conversation thread. This should be called
    * whenever the set of annotations to render or the sort/search/filter
    * settings change.
    */
   function rebuildRootThread() {
+    var sortFn = sortFns[annotationUI.getState().sortMode];
+
     var filters;
-    if (searchQuery) {
-      // TODO - Only regenerate the filter function when the search
-      // query changes
-      filters = searchFilter.generateFacetedFilter(searchQuery);
+    var filterQuery = annotationUI.getState().filterQuery;
+
+    if (filterQuery) {
+      filters = searchFilter.generateFacetedFilter(filterQuery);
     }
 
     var filterFn;
-    if (searchQuery) {
+    if (filterQuery) {
       filterFn = function (annot) {
         return viewFilter.filter([annot], filters).length > 0;
       };
@@ -77,7 +77,10 @@ module.exports = function ($rootScope, annotationUI, searchFilter, viewFilter) {
   annotationUI.subscribe(rebuildRootThread);
 
   // Listen for annotations being created or loaded
-  // and show them in the UI
+  // and show them in the UI.
+  //
+  // Note: These events could all be converted into actions that are handled by
+  // the Redux store in annotationUI.
   var loadEvents = [events.BEFORE_ANNOTATION_CREATED,
                     events.ANNOTATION_CREATED,
                     events.ANNOTATIONS_LOADED];
@@ -123,27 +126,5 @@ module.exports = function ($rootScope, annotationUI, searchFilter, viewFilter) {
     thread: function () {
       return thread;
     },
-
-    /**
-     * Set the sort order for annotations.
-     * @param {'Location'|'Newest'|'Oldest'} mode
-     */
-    sortBy: function (mode) {
-      if (!sortFns[mode]) {
-        throw new Error('Unknown sort mode: ' + mode);
-      }
-      sortFn = sortFns[mode];
-      rebuildRootThread();
-    },
-
-    /**
-     * Set the query to use when filtering annotations.
-     * @param {string} query - The filter query
-     */
-    setSearchQuery: function (query) {
-      searchQuery = query;
-      annotationUI.clearForceVisible();
-      rebuildRootThread();
-    },
   };
 };

h/static/scripts/stream-controller.coffee

 angular = require('angular')
-mail = require('./vendor/jwz')
-
 
 module.exports = class StreamController
   this.$inject = [
     '$scope', '$route', '$rootScope', '$routeParams',
-    'queryParser', 'searchFilter', 'store',
-    'streamer', 'streamFilter', 'threading', 'annotationMapper'
+    'annotationUI',
+    'queryParser', 'rootThread', 'searchFilter', 'store',
+    'streamer', 'streamFilter', 'annotationMapper'
   ]
   constructor: (
      $scope,   $route,   $rootScope,   $routeParams
-     queryParser,   searchFilter,   store,
-     streamer,   streamFilter,   threading,   annotationMapper
+     annotationUI,
+     queryParser,   rootThread,   searchFilter,   store,
+     streamer,   streamFilter,   annotationMapper
   ) ->
     offset = 0
 
@@ -26,16 +26,11 @@ module.exports = class StreamController
         offset += rows.length
         annotationMapper.loadAnnotations(rows, replies)
 
-    # Disable the thread filter (client-side search)
-    $scope.$on '$routeChangeSuccess', ->
-      if $scope.threadFilter?
-        $scope.threadFilter.active(false)
-        $scope.threadFilter.freeze(true)
-
     # Reload on query change (ignore hash change)
     lastQuery = $routeParams.q
     $scope.$on '$routeUpdate', ->
       if $routeParams.q isnt lastQuery
+        annotationUI.clearAnnotations()
         $route.reload()
 
     # Initialize the base filter
@@ -51,8 +46,19 @@ module.exports = class StreamController
     # Perform the initial search
     fetch(20)
 
+    $scope.$watch('sort.name', (name) ->
+      annotationUI.sortBy(name)
+    )
+
+    $scope.setCollapsed = (id, collapsed) ->
+      annotationUI.setCollapsed(id, collapsed)
+
+    $scope.forceVisible = (id) ->
+      annotationUI.setForceVisible(id, true)
+
     $scope.isStream = true
     $scope.sortOptions = ['Newest', 'Oldest']
     $scope.sort.name = 'Newest'
-    $scope.threadRoot = threading.root
+    $scope.rootThread = ->
+      return rootThread.thread()
     $scope.loadMore = fetch

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

@@ -2,37 +2,32 @@
 
 var angular = require('angular');
 
+var annotationUIFactory = require('../annotation-ui');
 var events = require('../events');
 
 describe('annotationMapper', function() {
   var sandbox = sinon.sandbox.create();
   var $rootScope;
+  var annotationUI;
   var fakeStore;
-  var fakeThreading;
   var annotationMapper;
 
-  before(function () {
-    angular.module('h', [])
-    .service('annotationMapper', require('../annotation-mapper'));
-  });
-
-  beforeEach(angular.mock.module('h'));
-  beforeEach(angular.mock.module(function ($provide) {
+  beforeEach(function () {
     fakeStore = {
-      AnnotationResource: sandbox.stub().returns({})
+      AnnotationResource: sandbox.stub().returns({}),
     };
-    fakeThreading = {
-      idTable: {}
-    };
-
-    $provide.value('store', fakeStore);
-    $provide.value('threading', fakeThreading);
-  }));
-
-  beforeEach(angular.mock.inject(function (_annotationMapper_, _$rootScope_) {
+    annotationUI = annotationUIFactory({});
+    angular.module('app', [])
+      .service('annotationMapper', require('../annotation-mapper'))
+      .value('annotationUI', annotationUI)
+      .value('store', fakeStore);
+    angular.mock.module('app');
+
+    angular.mock.inject(function (_$rootScope_, _annotationMapper_) {
       $rootScope = _$rootScope_;
       annotationMapper = _annotationMapper_;
-  }));
+    });
+  });
 
   afterEach(function () {
     sandbox.restore();
@@ -58,24 +53,22 @@ describe('annotationMapper', function() {
         [{}, {}, {}]);
     });
 
-    it('triggers the annotationUpdated event for each annotation in the threading cache', function () {
+    it('triggers the annotationUpdated event for each loaded annotation', function () {
       sandbox.stub($rootScope, '$emit');
       var annotations = [{id: 1}, {id: 2}, {id: 3}];
-      var cached = {message: {id: 1, $$tag: 'tag1'}};
-      fakeThreading.idTable[1] = cached;
+      annotationUI.addAnnotations(angular.copy(annotations));
 
       annotationMapper.loadAnnotations(annotations);
       assert.called($rootScope.$emit);
       assert.calledWith($rootScope.$emit, events.ANNOTATION_UPDATED,
-        cached.message);
+        annotations[0]);
     });
 
     it('also triggers annotationUpdated for cached replies', function () {
       sandbox.stub($rootScope, '$emit');
       var annotations = [{id: 1}];
       var replies = [{id: 2}, {id: 3}, {id: 4}];
-      var cached = {message: {id: 3, $$tag: 'tag3'}};
-      fakeThreading.idTable[3] = cached;
+      annotationUI.addAnnotations([{id:3}]);
 
       annotationMapper.loadAnnotations(annotations, replies);
       assert($rootScope.$emit.calledWith(events.ANNOTATION_UPDATED,
@@ -85,8 +78,7 @@ describe('annotationMapper', function() {
     it('replaces the properties on the cached annotation with those from the loaded one', function () {
       sandbox.stub($rootScope, '$emit');
       var annotations = [{id: 1, url: 'http://example.com'}];
-      var cached = {message: {id: 1, $$tag: 'tag1'}};
-      fakeThreading.idTable[1] = cached;
+      annotationUI.addAnnotations([{id:1, $$tag: 'tag1'}]);
 
       annotationMapper.loadAnnotations(annotations);
       assert.called($rootScope.$emit);
@@ -99,8 +91,7 @@ describe('annotationMapper', function() {
     it('excludes cached annotations from the annotationLoaded event', function () {
       sandbox.stub($rootScope, '$emit');
       var annotations = [{id: 1, url: 'http://example.com'}];
-      var cached = {message: {id: 1, $$tag: 'tag1'}};
-      fakeThreading.idTable[1] = cached;
+      annotationUI.addAnnotations([{id: 1, $$tag: 'tag1'}]);
 
       annotationMapper.loadAnnotations(annotations);
       assert.called($rootScope.$emit);
@@ -120,8 +111,7 @@ describe('annotationMapper', function() {
     it('replaces the properties on the cached annotation with those from the deleted one', function () {
       sandbox.stub($rootScope, '$emit');
       var annotations = [{id: 1, url: 'http://example.com'}];
-      var cached = {message: {id: 1, $$tag: 'tag1'}};
-      fakeThreading.idTable[1] = cached;
+      annotationUI.addAnnotations([{id: 1, $$tag: 'tag1'}]);
 
       annotationMapper.unloadAnnotations(annotations);
       assert.calledWith($rootScope.$emit, events.ANNOTATIONS_UNLOADED, [{

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

@@ -78,14 +78,6 @@ describe('annotationUI', function () {
     });
   });
 
-  describe('#clearForceVisible()', function () {
-    it('clears the forceVisible set', function () {
-      annotationUI.setForceVisible('id1', true);
-      annotationUI.clearForceVisible();
-      assert.deepEqual(annotationUI.getState().forceVisible, {});
-    });
-  });
-
   describe('#setCollapsed()', function () {
     it('sets the expanded state of the annotation', function () {
       annotationUI.setCollapsed('parent_id', false);
@@ -211,4 +203,19 @@ describe('annotationUI', function () {
       assert.isNull(annotationUI.getState().selectedAnnotationMap);
     });
   });
+
+  describe('#setFilterQuery()', function () {
+    it('sets the filter query', function () {
+      annotationUI.setFilterQuery('a-query');
+      assert.equal(annotationUI.getState().filterQuery, 'a-query');
+    });
+
+    it('resets the force-visible and expanded sets', function () {
+      annotationUI.setForceVisible('123', true);
+      annotationUI.setCollapsed('456', false);
+      annotationUI.setFilterQuery('some-query');
+      assert.deepEqual(annotationUI.getState().forceVisible, {});
+      assert.deepEqual(annotationUI.getState().expanded, {});
+    });
+  });
 });

h/static/scripts/test/annotation-viewer-controller-test.js

@@ -28,9 +28,10 @@ describe('AnnotationViewerController', function () {
       $routeParams: opts.$routeParams || { id: 'test_annotation_id' },
       $scope: opts.$scope || {
         search: {},
-        threading: {
-          getContainer: function () {}
-        }
+      },
+      annotationUI: {},
+      rootThread: {
+        thread: sinon.stub(),
       },
       streamer: opts.streamer || { setConfig: function () {} },
       store: opts.store || {
@@ -50,7 +51,6 @@ describe('AnnotationViewerController', function () {
         getFilter: function () {}
       },
       annotationMapper: opts.annotationMapper || { loadAnnotations: sinon.spy() },
-      threading: opts.threading || { idTable: {} }
     };
     locals.ctrl = getControllerService()(
       'AnnotationViewerController', locals);

h/static/scripts/test/build-thread-test.js

@@ -201,11 +201,6 @@ describe('build-thread', function () {
       assert.isTrue(thread.children[0].children[0].collapsed);
     });
 
-    it('expands threads with selected children', function () {
-      var thread = buildThread(SIMPLE_FIXTURE, {selected: ['3']});
-      assert.isFalse(thread.children[0].collapsed);
-    });
-
     it('expands threads with visible children', function () {
       // Simulate performing a search which only matches the top-level
       // annotation, not its reply, and then clicking

h/static/scripts/test/integration/threading-test.js

@@ -63,14 +63,14 @@ describe('annotation threading', function () {
 
   it('should filter annotations when a search is set', function () {
     annotationUI.addAnnotations(fixtures.annotations);
-    rootThread.setSearchQuery('second');
+    annotationUI.setFilterQuery('second');
     assert.equal(rootThread.thread().children.length, 1);
     assert.equal(rootThread.thread().children[0].id, '2');
   });
 
   unroll('should sort annotations by #mode', function (testCase) {
     annotationUI.addAnnotations(fixtures.annotations);
-    rootThread.sortBy(testCase.mode);
+    annotationUI.sortBy(testCase.mode);
     var actualOrder = rootThread.thread().children.map(function (thread) {
       return thread.annotation.id;
     });

h/static/scripts/test/root-thread-test.js

@@ -36,6 +36,8 @@ describe('rootThread', function () {
         selectedAnnotationMap: null,
         expanded: {},
         forceVisible: {},
+        filterQuery: null,
+        sortMode: 'Location',
       },
 
       getState: function () {
@@ -46,7 +48,6 @@ describe('rootThread', function () {
       removeSelectedAnnotation: sinon.stub(),
       addAnnotations: sinon.stub(),
       setCollapsed: sinon.stub(),
-      clearForceVisible: sinon.stub(),
     };
 
     fakeBuildThread = sinon.stub().returns(fixtures.emptyThread);
@@ -146,13 +147,7 @@ describe('rootThread', function () {
     });
   });
 
-  describe('#sortBy', function () {
-    it('rebuilds the thread when the sort order changes', function () {
-      assertRebuildsThread(function () {
-        rootThread.sortBy('Newest');
-      });
-    });
-
+  describe('when the sort order changes', function () {
     function sortBy(annotations, sortCompareFn) {
       return annotations.slice().sort(function (a,b) {
         return sortCompareFn(a,b) ? -1 : sortCompareFn(b,a) ? 1 : 0;
@@ -181,7 +176,10 @@ describe('rootThread', function () {
       }];
 
       fakeBuildThread.reset();
-      rootThread.sortBy(testCase.order);
+      fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state, {
+        sortMode: testCase.order,
+      });
+      rootThread.rebuild();
       var sortCompareFn = fakeBuildThread.args[0][1].sortCompareFn;
       var actualOrder = sortBy(annotations, sortCompareFn).map(function (annot) {
         return annotations.indexOf(annot);
@@ -194,19 +192,15 @@ describe('rootThread', function () {
     ]);
   });
 
-  describe('#setSearchQuery', function () {
-    it('rebuilds the thread when the search query changes', function () {
-      assertRebuildsThread(function () {
-        rootThread.setSearchQuery('new query');
-      });
-    });
-
-    it('generates a thread filter function from the search query', function () {
+  describe('when the filter query changes', function () {
+    it('generates a thread filter function from the query', function () {
       fakeBuildThread.reset();
       var filters = [{any: {terms: ['queryterm']}}];
       var annotation = annotationFixtures.defaultAnnotation();
       fakeSearchFilter.generateFacetedFilter.returns(filters);
-      rootThread.setSearchQuery('queryterm');
+      fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state,
+        {filterQuery: 'queryterm'});
+      rootThread.rebuild();
       var filterFn = fakeBuildThread.args[0][1].filterFn;
 
       fakeViewFilter.filter.returns([annotation]);
@@ -214,11 +208,6 @@ describe('rootThread', function () {
       assert.calledWith(fakeViewFilter.filter, sinon.match([annotation]),
         filters);
     });
-
-    it('clears the set of explicitly shown conversations', function () {
-      rootThread.setSearchQuery('new query');
-      assert.called(fakeAnnotationUI.clearForceVisible);
-    });
   });
 
   context('when annotation events occur', function () {

h/static/scripts/test/stream-controller-test.coffee

@@ -4,6 +4,7 @@ describe 'StreamController', ->
   $controller = null
   $scope = null
   fakeAnnotationMapper = null
+  fakeAnnotationUI = null
   fakeParams = null
   fakeQueryParser = null
   fakeRoute = null
@@ -30,6 +31,12 @@ describe 'StreamController', ->
       loadAnnotations: sandbox.spy()
     }
 
+    fakeAnnotationUI = {
+      clearAnnotations: sandbox.spy()
+      setCollapsed: sandbox.spy()
+      setForceVisible: sandbox.spy()
+    }
+
     fakeParams = {id: 'test'}
 
     fakeQueryParser = {
@@ -63,19 +70,20 @@ describe 'StreamController', ->
       getFilter: sandbox.stub()
     }
 
-    fakeThreading = {
-      root: {}
+    fakeRootThread = {
+      thread: sandbox.stub()
     }
 
     $provide.value 'annotationMapper', fakeAnnotationMapper
+    $provide.value 'annotationUI', fakeAnnotationUI
     $provide.value '$route', fakeRoute
     $provide.value '$routeParams', fakeParams
     $provide.value 'queryParser', fakeQueryParser
+    $provide.value 'rootThread', fakeRootThread
     $provide.value 'searchFilter', fakeSearchFilter
     $provide.value 'store', fakeStore
     $provide.value 'streamer', fakeStreamer
     $provide.value 'streamFilter', fakeStreamFilter
-    $provide.value 'threading', fakeThreading
     return
 
   beforeEach inject (_$controller_, $rootScope) ->
@@ -106,17 +114,6 @@ describe 'StreamController', ->
     )
 
 
-  describe 'on $routeChangeSuccess', ->
-
-    it 'disables page search', ->
-      createController()
-      $scope.threadFilter = {active: sinon.stub(), freeze: sinon.stub()}
-      $scope.$broadcast('$routeChangeSuccess')
-      assert.calledOnce($scope.threadFilter.active)
-      assert.calledWith($scope.threadFilter.active, false)
-      assert.calledOnce($scope.threadFilter.freeze)
-      assert.calledWith($scope.threadFilter.freeze, true)
-
   describe 'on $routeUpdate', ->
 
     it 'reloads the route when the query changes', ->
@@ -124,6 +121,7 @@ describe 'StreamController', ->
       createController()
       fakeParams.q = 'new query'
       $scope.$broadcast('$routeUpdate')
+      assert.called(fakeAnnotationUI.clearAnnotations)
       assert.calledOnce(fakeRoute.reload)
 
     it 'does not reload the route when the query is the same', ->

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

@@ -27,20 +27,20 @@ function FakeSearchClient(resource, opts) {
 inherits(FakeSearchClient, EventEmitter);
 
 describe('WidgetController', function () {
-  var $scope = null;
-  var $rootScope = null;
-  var annotationUI = null;
-  var fakeAnnotationMapper = null;
-  var fakeCrossFrame = null;
-  var fakeDrafts = null;
-  var fakeStore = null;
-  var fakeStreamer = null;
-  var fakeStreamFilter = null;
-  var fakeThreading = null;
-  var fakeGroups = null;
-  var sandbox = null;
-  var viewer = null;
-  var fakeSettings = null;
+  var $rootScope;
+  var $scope;
+  var annotationUI;
+  var fakeAnnotationMapper;
+  var fakeCrossFrame;
+  var fakeDrafts;
+  var fakeGroups;
+  var fakeRootThread;
+  var fakeSettings;
+  var fakeStore;
+  var fakeStreamer;
+  var fakeStreamFilter;
+  var sandbox;
+  var viewer;
 
   before(function () {
     angular.module('h', [])
@@ -64,12 +64,13 @@ describe('WidgetController', function () {
     };
 
     annotationUI = annotationUIFactory({});
+
     fakeCrossFrame = {
       call: sinon.stub(),
       frames: [],
     };
     fakeDrafts = {
-      unsaved: sandbox.stub()
+      unsaved: sandbox.stub().returns([]),
     };
 
     fakeStreamer = {
@@ -82,19 +83,19 @@ describe('WidgetController', function () {
       getFilter: sandbox.stub().returns({})
     };
 
-    fakeThreading = {
-      root: {},
-      thread: sandbox.stub(),
-      annotationList: function () {
-        return [{id: '123'}];
-      },
-    };
-
     fakeGroups = {
       focused: function () { return {id: 'foo'}; },
       focus: sinon.stub(),
     };
 
+    fakeRootThread = {
+      thread: sinon.stub().returns({
+        totalChildren: 0,
+      }),
+      setSearchQuery: sinon.stub(),
+      sortBy: sinon.stub(),
+    };
+
     fakeSettings = {
       annotations: 'test',
     };
@@ -107,10 +108,10 @@ describe('WidgetController', function () {
     $provide.value('annotationUI', annotationUI);
     $provide.value('crossframe', fakeCrossFrame);
     $provide.value('drafts', fakeDrafts);
+    $provide.value('rootThread', fakeRootThread);
     $provide.value('store', fakeStore);
     $provide.value('streamer', fakeStreamer);
     $provide.value('streamFilter', fakeStreamFilter);
-    $provide.value('threading', fakeThreading);
     $provide.value('groups', fakeGroups);
     $provide.value('settings', fakeSettings);
   }));
@@ -129,13 +130,14 @@ describe('WidgetController', function () {
     it('unloads any existing annotations', function () {
       // When new clients connect, all existing annotations should be unloaded
       // before reloading annotations for each currently-connected client
+      annotationUI.addAnnotations([{id: '123'}]);
       fakeCrossFrame.frames.push({uri: 'http://example.com/page-a'});
       $scope.$digest();
       fakeAnnotationMapper.unloadAnnotations = sandbox.spy();
       fakeCrossFrame.frames.push({uri: 'http://example.com/page-b'});
       $scope.$digest();
       assert.calledWith(fakeAnnotationMapper.unloadAnnotations,
-        fakeThreading.annotationList());
+        annotationUI.getState().annotations);
     });
 
     it('loads all annotations for a frame', function () {
@@ -170,6 +172,10 @@ describe('WidgetController', function () {
         $scope.$digest();
       });
 
+      it('selectedAnnotationCount is > 0', function () {
+        assert.equal($scope.selectedAnnotationCount(), 1);
+      });
+
       it('switches to the selected annotation\'s group', function () {
         assert.calledWith(fakeGroups.focus, '__world__');
         assert.calledOnce(fakeAnnotationMapper.loadAnnotations);
@@ -196,6 +202,10 @@ describe('WidgetController', function () {
         $scope.$digest();
       });
 
+      it('selectedAnnotationCount is 0', function () {
+        assert.equal($scope.selectedAnnotationCount(), 0);
+      });
+
       it('fetches annotations for the current group', function () {
         assert.calledWith(searchClients[0].get, {uri: uri, group: 'a-group'});
       });
@@ -233,11 +243,7 @@ describe('WidgetController', function () {
         $$tag: 'atag',
         id: '123',
       };
-      fakeThreading.idTable = {
-        '123': {
-          message: annot,
-        },
-      };
+      annotationUI.addAnnotations([annot]);
       $scope.$digest();
       $rootScope.$broadcast(events.ANNOTATIONS_SYNCED, [{tag: 'atag'}]);
       assert.calledWith(fakeCrossFrame.call, 'focusAnnotations', ['atag']);
@@ -248,12 +254,18 @@ describe('WidgetController', function () {
   describe('when the focused group changes', function () {
     it('should load annotations for the new group', function () {
       var uri = 'http://example.com';
+
+      annotationUI.addAnnotations([{id: '123'}]);
+      annotationUI.addAnnotations = sinon.stub();
+
+      fakeDrafts.unsaved.returns([{id: uri + '123'}, {id: uri + '456'}]);
+
       fakeCrossFrame.frames.push({uri: uri});
       var loadSpy = fakeAnnotationMapper.loadAnnotations;
 
       $scope.$broadcast(events.GROUP_FOCUSED);
       assert.calledWith(fakeAnnotationMapper.unloadAnnotations, [{id: '123'}]);
-      assert.calledWith(fakeThreading.thread, fakeDrafts.unsaved());
+      assert.calledWith(annotationUI.addAnnotations, fakeDrafts.unsaved());
       $scope.$digest();
       assert.calledWith(loadSpy, [sinon.match({id: uri + '123'})]);
       assert.calledWith(loadSpy, [sinon.match({id: uri + '456'})]);
@@ -291,14 +303,13 @@ describe('WidgetController', function () {
   describe('direct linking messages', function () {
     it('displays a message if the selection is unavailable', function () {
       annotationUI.selectAnnotations(['missing']);
-      fakeThreading.idTable = {'123': {}};
       $scope.$digest();
       assert.isTrue($scope.selectedAnnotationUnavailable());
     });
 
     it('does not show a message if the selection is available', function () {
+      annotationUI.addAnnotations([{id: '123'}]);
       annotationUI.selectAnnotations(['123']);
-      fakeThreading.idTable = {'123': {}};
       $scope.$digest();
       assert.isFalse($scope.selectedAnnotationUnavailable());
     });
@@ -313,8 +324,8 @@ describe('WidgetController', function () {
       $scope.auth = {
         status: 'signed-out'
       };
+      annotationUI.addAnnotations([{id: '123'}]);
       annotationUI.selectAnnotations(['123']);
-      fakeThreading.idTable = {'123': {}};
       $scope.$digest();
       assert.isTrue($scope.shouldShowLoggedOutMessage());
     });
@@ -324,7 +335,6 @@ describe('WidgetController', function () {
         status: 'signed-out'
       };
       annotationUI.selectAnnotations(['missing']);
-      fakeThreading.idTable = {'123': {}};
       $scope.$digest();
       assert.isFalse($scope.shouldShowLoggedOutMessage());
     });
@@ -342,8 +352,8 @@ describe('WidgetController', function () {
       $scope.auth = {
         status: 'signed-in'
       };
+      annotationUI.addAnnotations([{id: '123'}]);
       annotationUI.selectAnnotations(['123']);
-      fakeThreading.idTable = {'123': {}};
       $scope.$digest();
       assert.isFalse($scope.shouldShowLoggedOutMessage());
     });
@@ -353,10 +363,46 @@ describe('WidgetController', function () {
         status: 'signed-out'
       };
       delete fakeSettings.annotations;
+      annotationUI.addAnnotations([{id: '123'}]);
       annotationUI.selectAnnotations(['123']);
-      fakeThreading.idTable = {'123': {}};
       $scope.$digest();
       assert.isFalse($scope.shouldShowLoggedOutMessage());
     });
   });
+
+  describe('#forceVisible', function () {
+    it('shows the thread', function () {
+      var thread = {id: '1'};
+      $scope.forceVisible(thread);
+      assert.deepEqual(annotationUI.getState().forceVisible, {1: true});
+    });
+
+    it('uncollapses the parent', function () {
+      var thread = {
+        id: '2',
+        parent: {id: '3'},
+      };
+      assert.equal(annotationUI.getState().expanded[thread.parent.id], undefined);
+      $scope.forceVisible(thread);
+      assert.equal(annotationUI.getState().expanded[thread.parent.id], true);
+    });
+  });
+
+  describe('#visibleCount', function () {
+    it('returns the total number of visible annotations or replies', function () {
+      fakeRootThread.thread.returns({
+        children: [{
+          id: '1',
+          visible: true,
+          children: [{ id: '3', visible: true, children: [] }],
+        },{
+          id: '2',
+          visible: false,
+          children: [],
+        }],
+      });
+      $scope.$digest();
+      assert.equal($scope.visibleCount(), 2);
+    });
+  });
 });

h/static/scripts/util/memoize.js

+'use strict';
+
+/**
+ * A simple memoization function which caches the last result of
+ * a single-argument function.
+ *
+ * The argument to the input function may be of any type and is compared
+ * using reference equality.
+ */
+function memoize(fn) {
+  if (fn.length !== 1) {
+    throw new Error('Memoize input must be a function of one argument');
+  }
+
+  var lastArg;
+  var lastResult;
+
+  return function (arg) {
+    if (arg === lastArg) {
+      return lastResult;
+    }
+    lastArg = arg;
+    lastResult = fn(arg);
+    return lastResult;
+  };
+}
+
+module.exports = memoize;

h/static/scripts/util/test/memoize-test.js

+'use strict';
+
+var memoize = require('../memoize');
+
+describe('memoize', function () {
+  var count = 0;
+  var memoized;
+
+  function square(arg) {
+    ++count;
+    return arg * arg;
+  }
+
+  beforeEach(function () {
+    count = 0;
+    memoized = memoize(square);
+  });
+
+  it('computes the result of the function', function () {
+    assert.equal(memoized(12), 144);
+  });
+
+  it('does not recompute if the input is unchanged', function () {
+    memoized(42);
+    memoized(42);
+    assert.equal(count, 1);
+  });
+
+  it('recomputes if the input changes', function () {
+    memoized(42);
+    memoized(39);
+    assert.equal(count, 2);
+  });
+});

h/static/scripts/widget-controller.js

 'use strict';
 
 var events = require('./events');
+var memoize = require('./util/memoize');
 var SearchClient = require('./search-client');
 
 function firstKey(object) {
@@ -31,11 +32,17 @@ function groupIDFromSelection(selection, results) {
 // @ngInject
 module.exports = function WidgetController(
   $scope, $rootScope, annotationUI, crossframe, annotationMapper,
-  drafts, groups, settings, streamer, streamFilter, store, threading
+  drafts, groups, rootThread, settings, streamer, streamFilter, store
 ) {
-  $scope.threadRoot = threading.root;
+
   $scope.sortOptions = ['Newest', 'Oldest', 'Location'];
 
+  function annotationExists(id) {
+    return annotationUI.getState().annotations.some(function (annot) {
+      return annot.id === id;
+    });
+  }
+
   function focusAnnotation(annotation) {
     var highlights = [];
     if (annotation) {
@@ -60,21 +67,23 @@ module.exports = function WidgetController(
   function firstSelectedAnnotation() {
     if (annotationUI.getState().selectedAnnotationMap) {
       var id = Object.keys(annotationUI.getState().selectedAnnotationMap)[0];
-      return threading.idTable[id] && threading.idTable[id].message;
+      return annotationUI.getState().annotations.find(function (annot) {
+        return annot.id === id;
+      });
     } else {
       return null;
     }
   }
 
+  var searchClients = [];
+
   function _resetAnnotations() {
     // Unload all the annotations
-    annotationMapper.unloadAnnotations(threading.annotationList());
+    annotationMapper.unloadAnnotations(annotationUI.getState().annotations);
     // Reload all the drafts
-    threading.thread(drafts.unsaved());
+    annotationUI.addAnnotations(drafts.unsaved());
   }
 
-  var searchClients = [];
-
   function _loadAnnotationsFor(uri, group) {
     var searchClient = new SearchClient(store.SearchResource, {
       // If no group is specified, we are fetching annotations from
@@ -196,6 +205,30 @@ module.exports = function WidgetController(
     return crossframe.frames;
   }, loadAnnotations);
 
+  // Watch the inputs that determine which annotations are currently
+  // visible and how they are sorted and rebuild the thread when they change
+  $scope.$watch('sort.name', function (mode) {
+    annotationUI.sortBy(mode);
+  });
+  $scope.$watch('search.query', function (query) {
+    annotationUI.setFilterQuery(query);
+  });
+
+  $scope.rootThread = function () {
+    return rootThread.thread();
+  };
+
+  $scope.setCollapsed = function (id, collapsed) {
+    annotationUI.setCollapsed(id, collapsed);
+  };
+
+  $scope.forceVisible = function (thread) {
+    annotationUI.setForceVisible(thread.id, true);
+    if (thread.parent) {
+      annotationUI.setCollapsed(thread.parent.id, false);
+    }
+  };
+
   $scope.focus = focusAnnotation;
   $scope.scrollTo = scrollToAnnotation;
 
@@ -206,14 +239,19 @@ module.exports = function WidgetController(
     return annotation.$$tag in annotationUI.getState().focusedAnnotationMap;
   };
 
-  function selectedID() {
-    return firstKey(annotationUI.getState().selectedAnnotationMap);
+  $scope.selectedAnnotationCount = function () {
+    var selection = annotationUI.getState().selectedAnnotationMap;
+    if (!selection) {
+      return 0;
     }
+    return Object.keys(selection).length;
+  };
 
   $scope.selectedAnnotationUnavailable = function () {
+    var selectedID = firstKey(annotationUI.getState().selectedAnnotationMap);
     return !isLoading() &&
-           !!selectedID() &&
-           !threading.idTable[selectedID()];
+           !!selectedID &&
+           !annotationExists(selectedID);
   };
 
   $scope.shouldShowLoggedOutMessage = function () {
@@ -231,21 +269,32 @@ module.exports = function WidgetController(
     // The user is logged out and has landed on a direct linked
     // annotation. If there is an annotation selection and that
     // selection is available to the user, show the CTA.
+    var selectedID = firstKey(annotationUI.getState().selectedAnnotationMap);
     return !isLoading() &&
-           !!selectedID() &&
-           !!threading.idTable[selectedID()];
+           !!selectedID &&
+           annotationExists(selectedID);
   };
 
   $scope.isLoading = isLoading;
 
+  var visibleCount = memoize(function (thread) {
+    return thread.children.reduce(function (count, child) {
+      return count + visibleCount(child);
+    }, thread.visible ? 1 : 0);
+  });
+
+  $scope.visibleCount = function () {
+    return visibleCount(rootThread.thread());
+  };
+
   $scope.topLevelThreadCount = function () {
-    return threading.root.children.length;
+    return rootThread.thread().totalChildren;
   };
 
   $rootScope.$on(events.BEFORE_ANNOTATION_CREATED, function (event, data) {
     if (data.$highlight || (data.references && data.references.length > 0)) {
       return;
     }
-    return $scope.clearSelection();
+    $scope.clearSelection();
   });
 };

h/templates/client/annotation.html

@@ -29,11 +29,11 @@
         <i class="h-icon-border-color" ng-show="vm.isHighlight() && !vm.editing()" title="This is a highlight. Click 'edit' to add a note or tag."></i>
         <span class="annotation-citation"
           ng-bind-html="vm.documentTitle"
-          ng-if="::!vm.isSidebar">
+          ng-if="::vm.showDocumentInfo">
         </span>
         <span class="annotation-citation-domain"
           ng-bind-html="vm.documentDomain"
-          ng-if="::!vm.isSidebar">
+          ng-if="::vm.showDocumentInfo">
         </span>
       </span>
     </span>

h/templates/client/annotation_thread.html

+<a href=""
+   class="threadexp"
+   title="{{vm.thread.collapsed && 'Expand' || 'Collapse'}}"
+   ng-click="vm.toggleCollapsed()"
+   ng-if="vm.thread.parent">
+   <span ng-class="{'h-icon-arrow-right': vm.thread.collapsed,
+                    'h-icon-arrow-drop-down': !vm.thread.collapsed}"></span>
+</a>
+
+<!-- Annotation -->
+<annotation class="annotation thread-message {{vm.thread.collapsed && 'collapsed'}}"
+         annotation="vm.thread.annotation"
+         is-collapsed="vm.thread.collapsed"
+         is-last-reply="$last"
+         is-sidebar="::vm.isSidebar"
+         name="annotation"
+         ng-if="vm.thread.annotation"
+         ng-show="vm.thread.visible"
+         show-document-info="vm.showDocumentInfo"
+         on-reply-count-click="vm.toggleCollapsed()"
+         reply-count="vm.thread.replyCount">
+</annotation>
+
+<div ng-if="!vm.thread.annotation" class="thread-deleted">
+  <p><em>Message not available.</em></p>
+</div>
+
+<div class="thread-load-more" ng-if="vm.hiddenCount() > 0">
+  <a class="load-more small"
+     href=""
+     ng-click="vm.showThreadAndReplies()"
+     ng-pluralize
+     count="vm.hiddenCount()"
+     when="{'0': '',
+           one: 'View one more in conversation',
+           other: 'View {} more in conversation'}"
+     ></a>
+</div>
+
+<!-- Replies -->
+<ul class="thread-replies" ng-show="!vm.thread.collapsed">
+  <li class="thread"
+      ng-repeat="child in vm.thread.children track by child.id"
+      ng-show="vm.shouldShowReply(child)">
+    <annotation-thread
+      show-document-info="false"
+      thread="child"
+      on-change-collapsed="vm.onChangeCollapsed({id:id, collapsed:collapsed})"
+      on-force-visible="vm.onForceVisible({thread:thread})">
+    </annotation-thread>
+  </li>
+</ul>

h/templates/client/viewer.html

@@ -4,16 +4,15 @@
  -->
 <ul class="stream-list ng-hide"
     ng-show="true"
-    deep-count="count"
-    thread-filter="search.query"
     window-scroll="loadMore(20)">
   <search-status-bar
     ng-show="!isLoading()"
-    filter-active="threadFilter.active()"
-    filter-match-count="count('match')"
+    ng-if="!isStream"
+    filter-active="search.query"
+    filter-match-count="visibleCount()"
     on-clear-selection="clearSelection()"
     search-query="search ? search.query : ''"
-    selection-count="selectedAnnotationsCount"
+    selection-count="selectedAnnotationCount()"
     total-count="topLevelThreadCount()"
     >
   </search-status-bar>
@@ -33,15 +32,17 @@
   </li>
   <li id="{{vm.id}}"
       class="annotation-card thread"
-      ng-class="{'js-hover': hasFocus(child.message)}"
-      deep-count="count"
-      thread="child" thread-filter
-      ng-include="'thread.html'"
-      ng-mouseenter="focus(child.message)"
-      ng-click="scrollTo(child.message)"
+      ng-class="{'js-hover': hasFocus(child.annotation)}"
+      ng-mouseenter="focus(child.annotation)"
+      ng-click="scrollTo(child.annotation)"
       ng-mouseleave="focus()"
-      ng-repeat="child in threadRoot.children | orderBy : sort.predicate"
-      ng-show="vm.shouldShow()">
+      ng-repeat="child in rootThread().children track by child.id">
+      <annotation-thread
+        thread="child"
+        show-document-info="::!isSidebar"
+        on-change-collapsed="setCollapsed(id, collapsed)"
+        on-force-visible="forceVisible(thread)">
+      </annotation-thread>
   </li>
   <loggedout-message ng-if="isSidebar && shouldShowLoggedOutMessage()"
     on-login="login()" ng-cloak>