Merge pull request #3379 from hypothesis/sort-state-redux

Move canonical annotation list sort state to Redux store

h/static/scripts/annotation-ui.js

@@ -52,7 +52,10 @@ function initialState(settings) {
 
     filterQuery: null,
 
-    sortMode: 'Location',
+    // Key by which annotations are currently sorted.
+    sortKey: 'Location',
+    // Keys by which annotations can be sorted.
+    sortKeysAvailable: ['Newest', 'Oldest', 'Location'],
   });
 }
 
@@ -66,7 +69,7 @@ var types = {
   REMOVE_ANNOTATIONS: 'REMOVE_ANNOTATIONS',
   CLEAR_ANNOTATIONS: 'CLEAR_ANNOTATIONS',
   SET_FILTER_QUERY: 'SET_FILTER_QUERY',
-  SORT_BY: 'SORT_BY',
+  SET_SORT_KEY: 'SET_SORT_KEY',
 };
 
 function excludeAnnotations(current, annotations) {
@@ -116,8 +119,8 @@ function reducer(state, action) {
         forceVisible: {},
         expanded: {},
       });
-    case types.SORT_BY:
-      return Object.assign({}, state, {sortMode: action.mode});
+    case types.SET_SORT_KEY:
+      return Object.assign({}, state, {sortKey: action.key});
     default:
       return state;
   }
@@ -301,11 +304,11 @@ module.exports = function (settings) {
       });
     },
 
-    /** Sets the sort mode for the annotation list. */
-    sortBy: function (mode) {
+    /** Sets the sort key for the annotation list. */
+    setSortKey: function (key) {
       store.dispatch({
-        type: types.SORT_BY,
-        mode: mode,
+        type: types.SET_SORT_KEY,
+        key: key,
       });
     },
   };

h/static/scripts/app-controller.js

@@ -3,7 +3,6 @@
 var angular = require('angular');
 var scrollIntoView = require('scroll-into-view');
 
-var annotationMetadata = require('./annotation-metadata');
 var events = require('./events');
 var parseAccountID = require('./filter/persona').parseAccountID;
 var scopeTimeout = require('./util/scope-timeout');
@@ -53,12 +52,16 @@ module.exports = function AppController(
   // the stream page or an individual annotation page.
   $scope.isSidebar = $window.top !== $window;
 
-  // Default sort
-  $scope.sort = {
-    name: 'Location',
-    options: ['Newest', 'Oldest', 'Location']
+  $scope.sortKey = function () {
+    return annotationUI.getState().sortKey;
   };
 
+  $scope.sortKeysAvailable = function () {
+    return annotationUI.getState().sortKeysAvailable;
+  };
+
+  $scope.setSortKey = annotationUI.setSortKey;
+
   // Reload the view when the user switches accounts
   $scope.$on(events.USER_CHANGED, function (event, data) {
     $scope.auth = authStateFromUserID(data.userid);
@@ -79,31 +82,6 @@ module.exports = function AppController(
     }
   });
 
-  $scope.$watch('sort.name', function (name) {
-    if (!name) {
-      return;
-    }
-    var predicateFn;
-    switch (name) {
-      case 'Newest':
-        predicateFn = ['-!!message', '-message.updated'];
-        break;
-      case 'Oldest':
-        predicateFn = ['-!!message', 'message.updated'];
-        break;
-      case 'Location':
-        predicateFn = function (thread) {
-          return annotationMetadata.location(thread.message);
-        };
-        break;
-    }
-    $scope.sort = {
-      name: name,
-      predicate: predicateFn,
-      options: $scope.sort.options,
-    };
-  });
-
   /** Scroll to the view to the element matching the given selector */
   function scrollToView(selector) {
     // Add a timeout so that if the element has just been shown (eg. via ngIf)

h/static/scripts/directive/sort-dropdown.js

+'use strict';
+
 module.exports = function () {
   return {
     restrict: 'E',
     scope: {
-      /** The name of the currently selected sort criteria. */
-      sortBy: '<',
-      /** A list of choices that the user can opt to sort by. */
-      sortOptions: '<',
-      /** If true, the menu uses just an icon, otherwise
-       * it displays 'Sorted by {{sortBy}}'
-       */
-      showAsIcon: '<',
-      /** Called when the user changes the current sort criteria. */
-      onChangeSortBy: '&',
+      /** The name of the currently selected sort key. */
+      sortKey: '<',
+      /** A list of possible keys that the user can opt to sort by. */
+      sortKeysAvailable: '<',
+      /** Called when the user changes the sort key. */
+      onChangeSortKey: '&',
     },
     template: require('../../../templates/client/sort_dropdown.html'),
   }
-}
+};

h/static/scripts/directive/test/sort-dropdown-test.js

@@ -14,13 +14,13 @@ describe('sortDropdown', function () {
     angular.mock.module('app');
   });
 
-  it('should update the sort mode on click', function () {
+  it('should update the sort key on click', function () {
     var changeSpy = sinon.spy();
     var elem = util.createDirective(document, 'sortDropdown', {
-      sortOptions: ['Newest', 'Oldest'],
-      sortBy: 'Newest',
-      onChangeSortBy: {
-        args: ['sortBy'],
+      sortKeysAvailable: ['Newest', 'Oldest'],
+      sortKey: 'Newest',
+      onChangeSortKey: {
+        args: ['sortKey'],
         callback: changeSpy,
       }
     });

h/static/scripts/directive/top-bar.js

@@ -13,9 +13,9 @@ module.exports = function () {
       searchController: '<',
       accountDialog: '<',
       shareDialog: '<',
-      sortBy: '<',
-      sortOptions: '<',
-      onChangeSortBy: '&',
+      sortKey: '<',
+      sortKeysAvailable: '<',
+      onChangeSortKey: '&',
     },
     template: require('../../../templates/client/top_bar.html'),
   };

h/static/scripts/root-thread.js

@@ -50,7 +50,7 @@ function RootThread($rootScope, annotationUI, searchFilter, viewFilter) {
    * settings change.
    */
   function rebuildRootThread() {
-    var sortFn = sortFns[annotationUI.getState().sortMode];
+    var sortFn = sortFns[annotationUI.getState().sortKey];
 
     var filters;
     var filterQuery = annotationUI.getState().filterQuery;

h/static/scripts/stream-controller.coffee

@@ -46,10 +46,6 @@ 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)
 
@@ -64,9 +60,10 @@ module.exports = class StreamController
       };
     );
 
+    # Sort the stream so that the newest annotations are at the top
+    annotationUI.setSortKey('Newest')
+
     $scope.isStream = true
-    $scope.sortOptions = ['Newest', 'Oldest']
-    $scope.sort.name = 'Newest'
     $scope.rootThread = ->
       return rootThread.thread()
     $scope.loadMore = fetch

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

@@ -3,7 +3,6 @@
 var angular = require('angular');
 var proxyquire = require('proxyquire');
 
-var annotationFixtures = require('./annotation-fixtures');
 var events = require('../events');
 var util = require('./util');
 
@@ -292,24 +291,4 @@ describe('AppController', function () {
       assert.equal(fakeWindow.confirm.callCount, 0);
     });
   });
-
-  describe('sorting', function () {
-    function annotationThread() {
-      return {message: annotationFixtures.defaultAnnotation()};
-    }
-
-    it('sorts threads by location when sort name is "Location"', function () {
-      var threads = [annotationThread(), annotationThread()];
-      fakeAnnotationMetadata.location = function (annotation) {
-        return threads.findIndex(function (thread) {
-          return thread.message === annotation;
-        });
-      };
-      createController();
-      $scope.sort.name = 'Location';
-      $scope.$digest();
-      assert.equal($scope.sort.predicate(threads[0]), 0);
-      assert.equal($scope.sort.predicate(threads[1]), 1);
-    });
-  });
 });

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

@@ -70,16 +70,16 @@ describe('annotation threading', function () {
 
   unroll('should sort annotations by #mode', function (testCase) {
     annotationUI.addAnnotations(fixtures.annotations);
-    annotationUI.sortBy(testCase.mode);
+    annotationUI.setSortKey(testCase.sortKey);
     var actualOrder = rootThread.thread().children.map(function (thread) {
       return thread.annotation.id;
     });
     assert.deepEqual(actualOrder, testCase.expectedOrder);
   }, [{
-    mode: 'Oldest',
+    sortKey: 'Oldest',
     expectedOrder: ['1','2'],
   },{
-    mode: 'Newest',
+    sortKey: 'Newest',
     expectedOrder: ['2','1'],
   }]);
 });

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

@@ -37,7 +37,8 @@ describe('rootThread', function () {
         expanded: {},
         forceVisible: {},
         filterQuery: null,
-        sortMode: 'Location',
+        sortKey: 'Location',
+        sortKeysAvailable: ['Location'],
       },
 
       getState: function () {
@@ -177,7 +178,8 @@ describe('rootThread', function () {
 
       fakeBuildThread.reset();
       fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state, {
-        sortMode: testCase.order,
+        sortKey: testCase.order,
+        sortKeysAvailable: [testCase.order],
       });
       rootThread.rebuild();
       var sortCompareFn = fakeBuildThread.args[0][1].sortCompareFn;

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

@@ -42,6 +42,7 @@ describe 'StreamController', ->
       clearAnnotations: sandbox.spy()
       setCollapsed: sandbox.spy()
       setForceVisible: sandbox.spy()
+      setSortKey: sandbox.spy()
     }
 
     fakeParams = {id: 'test'}
@@ -94,7 +95,6 @@ describe 'StreamController', ->
   beforeEach inject (_$controller_, $rootScope) ->
     $controller = _$controller_
     $scope = $rootScope.$new()
-    $scope.sort = {}
 
   afterEach ->
     sandbox.restore()

h/static/scripts/widget-controller.js

@@ -90,8 +90,6 @@ module.exports = function WidgetController(
     visibleThreads.detach();
   });
 
-  $scope.sortOptions = ['Newest', 'Oldest', 'Location'];
-
   function annotationExists(id) {
     return annotationUI.getState().annotations.some(function (annot) {
       return annot.id === id;
@@ -262,9 +260,6 @@ module.exports = function WidgetController(
 
   // 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);
   });

h/templates/client/sort_dropdown.html

 <span class="ng-cloak" dropdown keyboard-nav>
-  <span role="button"
-        data-toggle="dropdown"
-        dropdown-toggle
-        ng-if="!showAsIcon">
-    Sorted by {{sortBy | lowercase}}
-    <i class="h-icon-arrow-drop-down"></i>
-  </span>
   <button
     type="button"
     class="top-bar__btn"
-    ng-if="showAsIcon"
     dropdown-toggle
-    title="Sort by {{sortBy}}">
+    title="Sort by {{sortKey}}">
     <i class="h-icon-sort"></i>
   </button>
   <div class="dropdown-menu__top-arrow"></div>
-  <ul class="dropdown-menu"
-      ng-class="showAsIcon ? 'pull-right' : 'pull-none'" role="menu">
+  <ul class="dropdown-menu pull-right" role="menu">
     <li class="dropdown-menu__row"
-        ng-repeat="sortOption in sortOptions"
-        ng-click="onChangeSortBy({sortBy: sortOption})"
+        ng-repeat="key in sortKeysAvailable"
+        ng-click="onChangeSortKey({sortKey: key})"
         ><span class="dropdown-menu-radio"
-               ng-class="{'is-selected' : sortOption === sortBy}"
-        ></span><a class="dropdown-menu__link" href="">{{sortOption}}</a></li>
+               ng-class="{'is-selected' : sortKey === key}"
+        ></span><a class="dropdown-menu__link" href="">{{key}}</a></li>
   </ul>
 </span>

h/templates/client/top_bar.html

@@ -35,10 +35,9 @@
       title="Filter the annotation list">
     </simple-search>
     <sort-dropdown
-      sort-options="sortOptions"
-      sort-by="sortBy"
-      show-as-icon="true"
-      on-change-sort-by="onChangeSortBy({sortBy: sortBy})">
+      sort-keys-available="sortKeysAvailable"
+      sort-key="sortKey"
+      on-change-sort-key="onChangeSortKey({sortKey: sortKey})">
     </sort-dropdown>
     <a class="top-bar__btn"
        ng-click="onSharePage()"

h/templates/client/viewer.html

@@ -16,13 +16,6 @@
     total-count="topLevelThreadCount()"
     >
   </search-status-bar>
-  <li ng-if="isStream">
-    <sort-dropdown
-      sort-by="sort.name"
-      sort-options="sortOptions"
-      on-change-sort-by="sort.name = sortBy">
-    </sort-dropdown>
-  </li>
   <li class="annotation-unavailable-message"
       ng-if="selectedAnnotationUnavailable()">
     <div class="annotation-unavailable-message__icon"></div>