Commit 708f2847 authored by Robert Knight's avatar Robert Knight Committed by Nick Stenning

Move canonical sort state to Redux store

This is a step towards getting us to a place where all important state
in the front-end app is managed in the same way - kept in the Redux
store and modified only via actions.

The sort state was duplicated as a property on the $scope and also
stored in the Redux store.

Instead just expose functions on the scope which retrieve this state
directly from the store and dispatch an action to the store when the
user selects a different sort mode.

In addition, the terminology for sorting has been normalised across the
application (sortBy/sortOptions -> sortKey/sortKeysAvailable), and the
ability to sort the stream has been removed, as it was nonsensical and
broken.
parent 69451ba1
...@@ -52,7 +52,10 @@ function initialState(settings) { ...@@ -52,7 +52,10 @@ function initialState(settings) {
filterQuery: null, 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 = { ...@@ -66,7 +69,7 @@ var types = {
REMOVE_ANNOTATIONS: 'REMOVE_ANNOTATIONS', REMOVE_ANNOTATIONS: 'REMOVE_ANNOTATIONS',
CLEAR_ANNOTATIONS: 'CLEAR_ANNOTATIONS', CLEAR_ANNOTATIONS: 'CLEAR_ANNOTATIONS',
SET_FILTER_QUERY: 'SET_FILTER_QUERY', SET_FILTER_QUERY: 'SET_FILTER_QUERY',
SORT_BY: 'SORT_BY', SET_SORT_KEY: 'SET_SORT_KEY',
}; };
function excludeAnnotations(current, annotations) { function excludeAnnotations(current, annotations) {
...@@ -116,8 +119,8 @@ function reducer(state, action) { ...@@ -116,8 +119,8 @@ function reducer(state, action) {
forceVisible: {}, forceVisible: {},
expanded: {}, expanded: {},
}); });
case types.SORT_BY: case types.SET_SORT_KEY:
return Object.assign({}, state, {sortMode: action.mode}); return Object.assign({}, state, {sortKey: action.key});
default: default:
return state; return state;
} }
...@@ -301,11 +304,11 @@ module.exports = function (settings) { ...@@ -301,11 +304,11 @@ module.exports = function (settings) {
}); });
}, },
/** Sets the sort mode for the annotation list. */ /** Sets the sort key for the annotation list. */
sortBy: function (mode) { setSortKey: function (key) {
store.dispatch({ store.dispatch({
type: types.SORT_BY, type: types.SET_SORT_KEY,
mode: mode, key: key,
}); });
}, },
}; };
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
var angular = require('angular'); var angular = require('angular');
var scrollIntoView = require('scroll-into-view'); var scrollIntoView = require('scroll-into-view');
var annotationMetadata = require('./annotation-metadata');
var events = require('./events'); var events = require('./events');
var parseAccountID = require('./filter/persona').parseAccountID; var parseAccountID = require('./filter/persona').parseAccountID;
var scopeTimeout = require('./util/scope-timeout'); var scopeTimeout = require('./util/scope-timeout');
...@@ -53,12 +52,16 @@ module.exports = function AppController( ...@@ -53,12 +52,16 @@ module.exports = function AppController(
// the stream page or an individual annotation page. // the stream page or an individual annotation page.
$scope.isSidebar = $window.top !== $window; $scope.isSidebar = $window.top !== $window;
// Default sort $scope.sortKey = function () {
$scope.sort = { return annotationUI.getState().sortKey;
name: 'Location',
options: ['Newest', 'Oldest', 'Location']
}; };
$scope.sortKeysAvailable = function () {
return annotationUI.getState().sortKeysAvailable;
};
$scope.setSortKey = annotationUI.setSortKey;
// Reload the view when the user switches accounts // Reload the view when the user switches accounts
$scope.$on(events.USER_CHANGED, function (event, data) { $scope.$on(events.USER_CHANGED, function (event, data) {
$scope.auth = authStateFromUserID(data.userid); $scope.auth = authStateFromUserID(data.userid);
...@@ -79,31 +82,6 @@ module.exports = function AppController( ...@@ -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 */ /** Scroll to the view to the element matching the given selector */
function scrollToView(selector) { function scrollToView(selector) {
// Add a timeout so that if the element has just been shown (eg. via ngIf) // Add a timeout so that if the element has just been shown (eg. via ngIf)
......
'use strict';
module.exports = function () { module.exports = function () {
return { return {
restrict: 'E', restrict: 'E',
scope: { scope: {
/** The name of the currently selected sort criteria. */ /** The name of the currently selected sort key. */
sortBy: '<', sortKey: '<',
/** A list of choices that the user can opt to sort by. */ /** A list of possible keys that the user can opt to sort by. */
sortOptions: '<', sortKeysAvailable: '<',
/** If true, the menu uses just an icon, otherwise /** Called when the user changes the sort key. */
* it displays 'Sorted by {{sortBy}}' onChangeSortKey: '&',
*/
showAsIcon: '<',
/** Called when the user changes the current sort criteria. */
onChangeSortBy: '&',
}, },
template: require('../../../templates/client/sort_dropdown.html'), template: require('../../../templates/client/sort_dropdown.html'),
} }
} };
...@@ -14,13 +14,13 @@ describe('sortDropdown', function () { ...@@ -14,13 +14,13 @@ describe('sortDropdown', function () {
angular.mock.module('app'); 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 changeSpy = sinon.spy();
var elem = util.createDirective(document, 'sortDropdown', { var elem = util.createDirective(document, 'sortDropdown', {
sortOptions: ['Newest', 'Oldest'], sortKeysAvailable: ['Newest', 'Oldest'],
sortBy: 'Newest', sortKey: 'Newest',
onChangeSortBy: { onChangeSortKey: {
args: ['sortBy'], args: ['sortKey'],
callback: changeSpy, callback: changeSpy,
} }
}); });
......
...@@ -13,9 +13,9 @@ module.exports = function () { ...@@ -13,9 +13,9 @@ module.exports = function () {
searchController: '<', searchController: '<',
accountDialog: '<', accountDialog: '<',
shareDialog: '<', shareDialog: '<',
sortBy: '<', sortKey: '<',
sortOptions: '<', sortKeysAvailable: '<',
onChangeSortBy: '&', onChangeSortKey: '&',
}, },
template: require('../../../templates/client/top_bar.html'), template: require('../../../templates/client/top_bar.html'),
}; };
......
...@@ -50,7 +50,7 @@ function RootThread($rootScope, annotationUI, searchFilter, viewFilter) { ...@@ -50,7 +50,7 @@ function RootThread($rootScope, annotationUI, searchFilter, viewFilter) {
* settings change. * settings change.
*/ */
function rebuildRootThread() { function rebuildRootThread() {
var sortFn = sortFns[annotationUI.getState().sortMode]; var sortFn = sortFns[annotationUI.getState().sortKey];
var filters; var filters;
var filterQuery = annotationUI.getState().filterQuery; var filterQuery = annotationUI.getState().filterQuery;
......
...@@ -46,10 +46,6 @@ module.exports = class StreamController ...@@ -46,10 +46,6 @@ module.exports = class StreamController
# Perform the initial search # Perform the initial search
fetch(20) fetch(20)
$scope.$watch('sort.name', (name) ->
annotationUI.sortBy(name)
)
$scope.setCollapsed = (id, collapsed) -> $scope.setCollapsed = (id, collapsed) ->
annotationUI.setCollapsed(id, collapsed) annotationUI.setCollapsed(id, collapsed)
...@@ -64,9 +60,10 @@ module.exports = class StreamController ...@@ -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.isStream = true
$scope.sortOptions = ['Newest', 'Oldest']
$scope.sort.name = 'Newest'
$scope.rootThread = -> $scope.rootThread = ->
return rootThread.thread() return rootThread.thread()
$scope.loadMore = fetch $scope.loadMore = fetch
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
var angular = require('angular'); var angular = require('angular');
var proxyquire = require('proxyquire'); var proxyquire = require('proxyquire');
var annotationFixtures = require('./annotation-fixtures');
var events = require('../events'); var events = require('../events');
var util = require('./util'); var util = require('./util');
...@@ -292,24 +291,4 @@ describe('AppController', function () { ...@@ -292,24 +291,4 @@ describe('AppController', function () {
assert.equal(fakeWindow.confirm.callCount, 0); 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);
});
});
}); });
...@@ -70,16 +70,16 @@ describe('annotation threading', function () { ...@@ -70,16 +70,16 @@ describe('annotation threading', function () {
unroll('should sort annotations by #mode', function (testCase) { unroll('should sort annotations by #mode', function (testCase) {
annotationUI.addAnnotations(fixtures.annotations); annotationUI.addAnnotations(fixtures.annotations);
annotationUI.sortBy(testCase.mode); annotationUI.setSortKey(testCase.sortKey);
var actualOrder = rootThread.thread().children.map(function (thread) { var actualOrder = rootThread.thread().children.map(function (thread) {
return thread.annotation.id; return thread.annotation.id;
}); });
assert.deepEqual(actualOrder, testCase.expectedOrder); assert.deepEqual(actualOrder, testCase.expectedOrder);
}, [{ }, [{
mode: 'Oldest', sortKey: 'Oldest',
expectedOrder: ['1','2'], expectedOrder: ['1','2'],
},{ },{
mode: 'Newest', sortKey: 'Newest',
expectedOrder: ['2','1'], expectedOrder: ['2','1'],
}]); }]);
}); });
...@@ -37,7 +37,8 @@ describe('rootThread', function () { ...@@ -37,7 +37,8 @@ describe('rootThread', function () {
expanded: {}, expanded: {},
forceVisible: {}, forceVisible: {},
filterQuery: null, filterQuery: null,
sortMode: 'Location', sortKey: 'Location',
sortKeysAvailable: ['Location'],
}, },
getState: function () { getState: function () {
...@@ -177,7 +178,8 @@ describe('rootThread', function () { ...@@ -177,7 +178,8 @@ describe('rootThread', function () {
fakeBuildThread.reset(); fakeBuildThread.reset();
fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state, { fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state, {
sortMode: testCase.order, sortKey: testCase.order,
sortKeysAvailable: [testCase.order],
}); });
rootThread.rebuild(); rootThread.rebuild();
var sortCompareFn = fakeBuildThread.args[0][1].sortCompareFn; var sortCompareFn = fakeBuildThread.args[0][1].sortCompareFn;
......
...@@ -42,6 +42,7 @@ describe 'StreamController', -> ...@@ -42,6 +42,7 @@ describe 'StreamController', ->
clearAnnotations: sandbox.spy() clearAnnotations: sandbox.spy()
setCollapsed: sandbox.spy() setCollapsed: sandbox.spy()
setForceVisible: sandbox.spy() setForceVisible: sandbox.spy()
setSortKey: sandbox.spy()
} }
fakeParams = {id: 'test'} fakeParams = {id: 'test'}
...@@ -94,7 +95,6 @@ describe 'StreamController', -> ...@@ -94,7 +95,6 @@ describe 'StreamController', ->
beforeEach inject (_$controller_, $rootScope) -> beforeEach inject (_$controller_, $rootScope) ->
$controller = _$controller_ $controller = _$controller_
$scope = $rootScope.$new() $scope = $rootScope.$new()
$scope.sort = {}
afterEach -> afterEach ->
sandbox.restore() sandbox.restore()
......
...@@ -90,8 +90,6 @@ module.exports = function WidgetController( ...@@ -90,8 +90,6 @@ module.exports = function WidgetController(
visibleThreads.detach(); visibleThreads.detach();
}); });
$scope.sortOptions = ['Newest', 'Oldest', 'Location'];
function annotationExists(id) { function annotationExists(id) {
return annotationUI.getState().annotations.some(function (annot) { return annotationUI.getState().annotations.some(function (annot) {
return annot.id === id; return annot.id === id;
...@@ -262,9 +260,6 @@ module.exports = function WidgetController( ...@@ -262,9 +260,6 @@ module.exports = function WidgetController(
// Watch the inputs that determine which annotations are currently // Watch the inputs that determine which annotations are currently
// visible and how they are sorted and rebuild the thread when they change // 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) { $scope.$watch('search.query', function (query) {
annotationUI.setFilterQuery(query); annotationUI.setFilterQuery(query);
}); });
......
<span class="ng-cloak" dropdown keyboard-nav> <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 <button
type="button" type="button"
class="top-bar__btn" class="top-bar__btn"
ng-if="showAsIcon"
dropdown-toggle dropdown-toggle
title="Sort by {{sortBy}}"> title="Sort by {{sortKey}}">
<i class="h-icon-sort"></i> <i class="h-icon-sort"></i>
</button> </button>
<div class="dropdown-menu__top-arrow"></div> <div class="dropdown-menu__top-arrow"></div>
<ul class="dropdown-menu" <ul class="dropdown-menu pull-right" role="menu">
ng-class="showAsIcon ? 'pull-right' : 'pull-none'" role="menu">
<li class="dropdown-menu__row" <li class="dropdown-menu__row"
ng-repeat="sortOption in sortOptions" ng-repeat="key in sortKeysAvailable"
ng-click="onChangeSortBy({sortBy: sortOption})" ng-click="onChangeSortKey({sortKey: key})"
><span class="dropdown-menu-radio" ><span class="dropdown-menu-radio"
ng-class="{'is-selected' : sortOption === sortBy}" ng-class="{'is-selected' : sortKey === key}"
></span><a class="dropdown-menu__link" href="">{{sortOption}}</a></li> ></span><a class="dropdown-menu__link" href="">{{key}}</a></li>
</ul> </ul>
</span> </span>
...@@ -35,10 +35,9 @@ ...@@ -35,10 +35,9 @@
title="Filter the annotation list"> title="Filter the annotation list">
</simple-search> </simple-search>
<sort-dropdown <sort-dropdown
sort-options="sortOptions" sort-keys-available="sortKeysAvailable"
sort-by="sortBy" sort-key="sortKey"
show-as-icon="true" on-change-sort-key="onChangeSortKey({sortKey: sortKey})">
on-change-sort-by="onChangeSortBy({sortBy: sortBy})">
</sort-dropdown> </sort-dropdown>
<a class="top-bar__btn" <a class="top-bar__btn"
ng-click="onSharePage()" ng-click="onSharePage()"
......
...@@ -16,13 +16,6 @@ ...@@ -16,13 +16,6 @@
total-count="topLevelThreadCount()" total-count="topLevelThreadCount()"
> >
</search-status-bar> </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" <li class="annotation-unavailable-message"
ng-if="selectedAnnotationUnavailable()"> ng-if="selectedAnnotationUnavailable()">
<div class="annotation-unavailable-message__icon"></div> <div class="annotation-unavailable-message__icon"></div>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment