Commit 95de09d6 authored by Robert Knight's avatar Robert Knight

Replace Angular router

Implement a replacement for the Angular JS router which will enable the
root components of the app to be more easily migrated to Preact in future.
Since the app only has three routes with very simple parameters, we
don't yet need a fully-fledged router implementation but can get by with
some pretty basic URL parsing.

 - Introduce a store module which holds the currently active route
   (`null`, "sidebar", "annotation" or "stream") and any parameters of
   that route.

   This state replaces the `isSidebar` boolean in the `viewer` module,
   since whether the app is the sidebar can be determined by checking
   the active route name.

 - Add a `router` service which handles synchronizing the store route
   with the current URL (via the `sync` method or in response to a
   `popstate` DOM event) or updating the URL and active route in the
   store (via the `navigate` method).

 - Modify the `<hypothesis-app>` component to fetch the active route
   from the store and use it to conditionally render the appropriate
   content component. This replaces the Angular router configuration
   that used to be in `src/sidebar/index.js`

 - Change the places that used to read or update the current route to
   read the active route and route params from the store (`store.{route,
   routeParams}`) and switch the current route by using the `router` service
   (`router.navigate`)

 - Remove the unused angular-route package

Fixes #1878
parent 17cc1b9c
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
"@sentry/browser": "^5.6.2", "@sentry/browser": "^5.6.2",
"angular": "^1.7.5", "angular": "^1.7.5",
"angular-mocks": "^1.7.5", "angular-mocks": "^1.7.5",
"angular-route": "^1.7.5",
"angular-toastr": "^2.1.1", "angular-toastr": "^2.1.1",
"autoprefixer": "^9.4.7", "autoprefixer": "^9.4.7",
"aws-sdk": "^2.345.0", "aws-sdk": "^2.345.0",
......
...@@ -26,7 +26,6 @@ function fetchThread(api, id) { ...@@ -26,7 +26,6 @@ function fetchThread(api, id) {
// @ngInject // @ngInject
function AnnotationViewerContentController( function AnnotationViewerContentController(
$routeParams,
store, store,
api, api,
rootThread, rootThread,
...@@ -34,9 +33,9 @@ function AnnotationViewerContentController( ...@@ -34,9 +33,9 @@ function AnnotationViewerContentController(
streamFilter, streamFilter,
annotationMapper annotationMapper
) { ) {
store.setAppIsSidebar(false); store.clearAnnotations();
const id = $routeParams.id; const id = store.routeParams().id;
this.rootThread = () => rootThread.thread(store.getState()); this.rootThread = () => rootThread.thread(store.getState());
......
...@@ -35,7 +35,6 @@ function authStateFromProfile(profile) { ...@@ -35,7 +35,6 @@ function authStateFromProfile(profile) {
function HypothesisAppController( function HypothesisAppController(
$document, $document,
$rootScope, $rootScope,
$route,
$scope, $scope,
$window, $window,
analytics, analytics,
...@@ -82,6 +81,8 @@ function HypothesisAppController( ...@@ -82,6 +81,8 @@ function HypothesisAppController(
} }
}; };
this.route = () => store.route();
$scope.$on(events.USER_CHANGED, function(event, data) { $scope.$on(events.USER_CHANGED, function(event, data) {
self.onUserChange(data.profile); self.onUserChange(data.profile);
}); });
......
// @ngInject // @ngInject
function StreamContentController( function StreamContentController(
$scope, $scope,
$route,
$routeParams,
annotationMapper, annotationMapper,
store, store,
api, api,
rootThread, rootThread,
searchFilter searchFilter
) { ) {
store.setAppIsSidebar(false);
/** `offset` parameter for the next search API call. */ /** `offset` parameter for the next search API call. */
let offset = 0; let offset = 0;
...@@ -20,6 +16,8 @@ function StreamContentController( ...@@ -20,6 +16,8 @@ function StreamContentController(
annotationMapper.loadAnnotations(result.rows, result.replies); annotationMapper.loadAnnotations(result.rows, result.replies);
}; };
const currentQuery = () => store.routeParams().q;
/** /**
* Fetch the next `limit` annotations starting from `offset` from the API. * Fetch the next `limit` annotations starting from `offset` from the API.
*/ */
...@@ -30,7 +28,7 @@ function StreamContentController( ...@@ -30,7 +28,7 @@ function StreamContentController(
offset: offset, offset: offset,
limit: limit, limit: limit,
}, },
searchFilter.toObject($routeParams.q) searchFilter.toObject(currentQuery())
); );
api api
...@@ -41,21 +39,27 @@ function StreamContentController( ...@@ -41,21 +39,27 @@ function StreamContentController(
}); });
}; };
// Re-do search when query changes function clearAndFetch() {
const lastQuery = $routeParams.q;
$scope.$on('$routeUpdate', function() {
if ($routeParams.q !== lastQuery) {
store.clearAnnotations();
$route.reload();
}
});
// In case this route loaded after a client-side route change (eg. from // In case this route loaded after a client-side route change (eg. from
// '/a/:id'), clear any existing annotations. // '/a/:id'), clear any existing annotations.
store.clearAnnotations(); store.clearAnnotations();
// Perform the initial search // Fetch initial batch of annotations.
offset = 0;
fetch(20); fetch(20);
}
let lastQuery = currentQuery();
const unsubscribe = store.subscribe(() => {
const query = currentQuery();
if (query !== lastQuery) {
lastQuery = query;
clearAndFetch();
}
});
$scope.$on('$destroy', unsubscribe);
clearAndFetch();
this.setCollapsed = store.setCollapsed; this.setCollapsed = store.setCollapsed;
this.rootThread = () => rootThread.thread(store.getState()); this.rootThread = () => rootThread.thread(store.getState());
......
import { createElement } from 'preact'; import { createElement } from 'preact';
import { useEffect, useState } from 'preact/hooks';
import propTypes from 'prop-types'; import propTypes from 'prop-types';
import useStore from '../store/use-store';
import { withServices } from '../util/service-context'; import { withServices } from '../util/service-context';
import SearchInput from './search-input'; import SearchInput from './search-input';
...@@ -11,30 +11,23 @@ import SearchInput from './search-input'; ...@@ -11,30 +11,23 @@ import SearchInput from './search-input';
* *
* This displays and updates the "q" query param in the URL. * This displays and updates the "q" query param in the URL.
*/ */
function StreamSearchInput({ $location, $rootScope }) { function StreamSearchInput({ router }) {
const [query, setQuery] = useState($location.search().q); const query = useStore(store => store.routeParams().q);
const search = query => { const setQuery = query => {
$rootScope.$apply(() => {
// Re-route the user to `/stream` if they are on `/a/:id` and then set // Re-route the user to `/stream` if they are on `/a/:id` and then set
// the search query. // the search query.
$location.path('/stream').search({ q: query }); router.navigate('stream', { q: query });
});
}; };
useEffect(() => { return (
$rootScope.$on('$locationChangeSuccess', () => { <SearchInput query={query} onSearch={setQuery} alwaysExpanded={true} />
setQuery($location.search().q); );
});
}, [$location, $rootScope]);
return <SearchInput query={query} onSearch={search} alwaysExpanded={true} />;
} }
StreamSearchInput.propTypes = { StreamSearchInput.propTypes = {
$location: propTypes.object, router: propTypes.object,
$rootScope: propTypes.object,
}; };
StreamSearchInput.injectedProps = ['$location', '$rootScope']; StreamSearchInput.injectedProps = ['router'];
export default withServices(StreamSearchInput); export default withServices(StreamSearchInput);
...@@ -41,12 +41,11 @@ describe('annotationViewerContent', function() { ...@@ -41,12 +41,11 @@ describe('annotationViewerContent', function() {
function createController(opts) { function createController(opts) {
const locals = { const locals = {
$location: {},
$routeParams: { id: 'test_annotation_id' },
store: { store: {
setAppIsSidebar: sinon.stub(), clearAnnotations: sinon.stub(),
setCollapsed: sinon.stub(), setCollapsed: sinon.stub(),
highlightAnnotations: sinon.stub(), highlightAnnotations: sinon.stub(),
routeParams: sinon.stub().returns({ id: 'test_annotation_id' }),
subscribe: sinon.stub(), subscribe: sinon.stub(),
}, },
api: opts.api, api: opts.api,
......
...@@ -18,12 +18,10 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -18,12 +18,10 @@ describe('sidebar.components.hypothesis-app', function() {
let fakeFlash = null; let fakeFlash = null;
let fakeFrameSync = null; let fakeFrameSync = null;
let fakeIsSidebar = null; let fakeIsSidebar = null;
let fakeParams = null;
let fakeServiceConfig = null; let fakeServiceConfig = null;
let fakeSession = null; let fakeSession = null;
let fakeShouldAutoDisplayTutorial = null; let fakeShouldAutoDisplayTutorial = null;
let fakeGroups = null; let fakeGroups = null;
let fakeRoute = null;
let fakeServiceUrl = null; let fakeServiceUrl = null;
let fakeSettings = null; let fakeSettings = null;
let fakeWindow = null; let fakeWindow = null;
...@@ -103,8 +101,6 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -103,8 +101,6 @@ describe('sidebar.components.hypothesis-app', function() {
connect: sandbox.spy(), connect: sandbox.spy(),
}; };
fakeParams = { id: 'test' };
fakeSession = { fakeSession = {
load: sandbox.stub().returns(Promise.resolve({ userid: null })), load: sandbox.stub().returns(Promise.resolve({ userid: null })),
logout: sandbox.stub(), logout: sandbox.stub(),
...@@ -115,8 +111,6 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -115,8 +111,6 @@ describe('sidebar.components.hypothesis-app', function() {
focus: sandbox.spy(), focus: sandbox.spy(),
}; };
fakeRoute = { reload: sandbox.spy() };
fakeWindow = { fakeWindow = {
top: {}, top: {},
confirm: sandbox.stub(), confirm: sandbox.stub(),
...@@ -140,8 +134,6 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -140,8 +134,6 @@ describe('sidebar.components.hypothesis-app', function() {
$provide.value('settings', fakeSettings); $provide.value('settings', fakeSettings);
$provide.value('bridge', fakeBridge); $provide.value('bridge', fakeBridge);
$provide.value('groups', fakeGroups); $provide.value('groups', fakeGroups);
$provide.value('$route', fakeRoute);
$provide.value('$routeParams', fakeParams);
$provide.value('$window', fakeWindow); $provide.value('$window', fakeWindow);
}) })
); );
......
...@@ -12,9 +12,6 @@ class FakeRootThread extends EventEmitter { ...@@ -12,9 +12,6 @@ class FakeRootThread extends EventEmitter {
describe('StreamContentController', function() { describe('StreamContentController', function() {
let $componentController; let $componentController;
let $rootScope;
let fakeRoute;
let fakeRouteParams;
let fakeAnnotationMapper; let fakeAnnotationMapper;
let fakeStore; let fakeStore;
let fakeRootThread; let fakeRootThread;
...@@ -34,19 +31,13 @@ describe('StreamContentController', function() { ...@@ -34,19 +31,13 @@ describe('StreamContentController', function() {
fakeStore = { fakeStore = {
clearAnnotations: sinon.spy(), clearAnnotations: sinon.spy(),
setAppIsSidebar: sinon.spy(), routeParams: sinon.stub().returns({ id: 'test' }),
setCollapsed: sinon.spy(), setCollapsed: sinon.spy(),
setForceVisible: sinon.spy(), setForceVisible: sinon.spy(),
setSortKey: sinon.spy(), setSortKey: sinon.spy(),
subscribe: sinon.spy(), subscribe: sinon.spy(),
}; };
fakeRouteParams = { id: 'test' };
fakeRoute = {
reload: sinon.spy(),
};
fakeSearchFilter = { fakeSearchFilter = {
generateFacetedFilter: sinon.stub(), generateFacetedFilter: sinon.stub(),
toObject: sinon.stub().returns({}), toObject: sinon.stub().returns({}),
...@@ -74,8 +65,6 @@ describe('StreamContentController', function() { ...@@ -74,8 +65,6 @@ describe('StreamContentController', function() {
fakeRootThread = new FakeRootThread(); fakeRootThread = new FakeRootThread();
angular.mock.module('h', { angular.mock.module('h', {
$route: fakeRoute,
$routeParams: fakeRouteParams,
annotationMapper: fakeAnnotationMapper, annotationMapper: fakeAnnotationMapper,
store: fakeStore, store: fakeStore,
api: fakeApi, api: fakeApi,
...@@ -85,9 +74,8 @@ describe('StreamContentController', function() { ...@@ -85,9 +74,8 @@ describe('StreamContentController', function() {
streamer: fakeStreamer, streamer: fakeStreamer,
}); });
angular.mock.inject(function(_$componentController_, _$rootScope_) { angular.mock.inject(function(_$componentController_) {
$componentController = _$componentController_; $componentController = _$componentController_;
$rootScope = _$rootScope_;
}); });
}); });
...@@ -125,25 +113,30 @@ describe('StreamContentController', function() { ...@@ -125,25 +113,30 @@ describe('StreamContentController', function() {
}); });
}); });
context('when a $routeUpdate event occurs', function() { context('when route parameters change', function() {
it('reloads the route if the query changed', function() { it('updates annotations if the query changed', function() {
fakeRouteParams.q = 'test query'; fakeStore.routeParams.returns({ q: 'test query' });
createController(); createController();
fakeRouteParams.q = 'new query'; fakeStore.clearAnnotations.resetHistory();
$rootScope.$broadcast('$routeUpdate'); fakeApi.search.resetHistory();
fakeStore.routeParams.returns({ q: 'new query' });
fakeStore.subscribe.lastCall.callback();
assert.called(fakeStore.clearAnnotations); assert.called(fakeStore.clearAnnotations);
assert.calledOnce(fakeRoute.reload); assert.called(fakeApi.search);
}); });
it('does not reload the route if the query did not change', function() { it('does not clear annotations if the query did not change', function() {
fakeRouteParams.q = 'test query'; fakeStore.routeParams.returns({ q: 'test query' });
createController(); createController();
fakeApi.search.resetHistory();
fakeStore.clearAnnotations.resetHistory(); fakeStore.clearAnnotations.resetHistory();
$rootScope.$broadcast('$routeUpdate'); fakeStore.subscribe.lastCall.callback();
assert.notCalled(fakeStore.clearAnnotations); assert.notCalled(fakeStore.clearAnnotations);
assert.notCalled(fakeRoute.reload); assert.notCalled(fakeApi.search);
}); });
}); });
}); });
...@@ -8,20 +8,20 @@ import { $imports } from '../stream-search-input'; ...@@ -8,20 +8,20 @@ import { $imports } from '../stream-search-input';
import mockImportedComponents from '../../../test-util/mock-imported-components'; import mockImportedComponents from '../../../test-util/mock-imported-components';
describe('StreamSearchInput', () => { describe('StreamSearchInput', () => {
let fakeLocation; let fakeRouter;
let fakeRootScope; let fakeStore;
beforeEach(() => { beforeEach(() => {
fakeLocation = { fakeRouter = {
path: sinon.stub().returnsThis(), navigate: sinon.stub(),
search: sinon.stub().returns({ q: 'the-query' }),
}; };
fakeRootScope = { fakeStore = {
$apply: callback => callback(), routeParams: sinon.stub().returns({}),
$on: sinon.stub(),
}; };
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({
'../store/use-store': callback => callback(fakeStore),
});
}); });
afterEach(() => { afterEach(() => {
...@@ -29,16 +29,11 @@ describe('StreamSearchInput', () => { ...@@ -29,16 +29,11 @@ describe('StreamSearchInput', () => {
}); });
function createSearchInput(props = {}) { function createSearchInput(props = {}) {
return mount( return mount(<StreamSearchInput router={fakeRouter} {...props} />);
<StreamSearchInput
$location={fakeLocation}
$rootScope={fakeRootScope}
{...props}
/>
);
} }
it('displays current "q" search param', () => { it('displays current "q" search param', () => {
fakeStore.routeParams.returns({ q: 'the-query' });
const wrapper = createSearchInput(); const wrapper = createSearchInput();
assert.equal(wrapper.find('SearchInput').prop('query'), 'the-query'); assert.equal(wrapper.find('SearchInput').prop('query'), 'the-query');
}); });
...@@ -51,23 +46,6 @@ describe('StreamSearchInput', () => { ...@@ -51,23 +46,6 @@ describe('StreamSearchInput', () => {
.props() .props()
.onSearch('new-query'); .onSearch('new-query');
}); });
assert.calledWith(fakeLocation.path, '/stream'); assert.calledWith(fakeRouter.navigate, 'stream', { q: 'new-query' });
assert.calledWith(fakeLocation.search, { q: 'new-query' });
});
it('updates query when changed in URL', () => {
fakeLocation.search.returns({ q: 'query-b' });
const wrapper = createSearchInput();
assert.calledOnce(fakeRootScope.$on);
assert.calledWith(fakeRootScope.$on, '$locationChangeSuccess');
act(() => {
fakeRootScope.$on.lastCall.callback();
});
// Check that new query is displayed.
wrapper.update();
assert.equal(wrapper.find('SearchInput').prop('query'), 'query-b');
}); });
}); });
...@@ -32,7 +32,6 @@ import angular from 'angular'; ...@@ -32,7 +32,6 @@ import angular from 'angular';
// Angular addons which export the Angular module name via `module.exports`. // Angular addons which export the Angular module name via `module.exports`.
import angularRoute from 'angular-route';
import angularToastr from 'angular-toastr'; import angularToastr from 'angular-toastr';
// Load polyfill for :focus-visible pseudo-class. // Load polyfill for :focus-visible pseudo-class.
...@@ -49,15 +48,6 @@ if (appConfig.googleAnalytics) { ...@@ -49,15 +48,6 @@ if (appConfig.googleAnalytics) {
addAnalytics(appConfig.googleAnalytics); addAnalytics(appConfig.googleAnalytics);
} }
// Fetch external state that the app needs before it can run. This includes the
// user's profile and list of groups.
const resolve = {
// @ngInject
state: function(groups, session) {
return Promise.all([groups.load(), session.load()]);
},
};
const isSidebar = !( const isSidebar = !(
window.location.pathname.startsWith('/stream') || window.location.pathname.startsWith('/stream') ||
window.location.pathname.startsWith('/a/') window.location.pathname.startsWith('/a/')
...@@ -69,28 +59,6 @@ function configureLocation($locationProvider) { ...@@ -69,28 +59,6 @@ function configureLocation($locationProvider) {
return $locationProvider.html5Mode(true); return $locationProvider.html5Mode(true);
} }
// @ngInject
function configureRoutes($routeProvider) {
// The `vm.{auth,search}` properties used in these templates come from the
// `<hypothesis-app>` component which hosts the router's container element.
$routeProvider.when('/a/:id', {
template: '<annotation-viewer-content></annotation-viewer-content>',
reloadOnSearch: false,
resolve: resolve,
});
$routeProvider.when('/stream', {
template: '<stream-content></stream-content>',
reloadOnSearch: false,
resolve: resolve,
});
$routeProvider.otherwise({
template:
'<sidebar-content auth="vm.auth" on-login="vm.login()" on-sign-up="vm.signUp()"></sidebar-content>',
reloadOnSearch: false,
resolve: resolve,
});
}
// @ngInject // @ngInject
function configureToastr(toastrConfig) { function configureToastr(toastrConfig) {
angular.extend(toastrConfig, { angular.extend(toastrConfig, {
...@@ -103,6 +71,17 @@ function setupApi(api, streamer) { ...@@ -103,6 +71,17 @@ function setupApi(api, streamer) {
api.setClientId(streamer.clientId); api.setClientId(streamer.clientId);
} }
/**
* Perform the initial fetch of groups and user profile and then set the initial
* route to match the current URL.
*/
// @ngInject
function setupRoute(api, groups, session, router) {
Promise.all([groups.load(), session.load()]).finally(() => {
router.sync();
});
}
/** /**
* Send a page view event when the app starts up. * Send a page view event when the app starts up.
* *
...@@ -184,6 +163,7 @@ import localStorageService from './services/local-storage'; ...@@ -184,6 +163,7 @@ import localStorageService from './services/local-storage';
import permissionsService from './services/permissions'; import permissionsService from './services/permissions';
import persistedDefaultsService from './services/persisted-defaults'; import persistedDefaultsService from './services/persisted-defaults';
import rootThreadService from './services/root-thread'; import rootThreadService from './services/root-thread';
import routerService from './services/router';
import searchFilterService from './services/search-filter'; import searchFilterService from './services/search-filter';
import serviceUrlService from './services/service-url'; import serviceUrlService from './services/service-url';
import sessionService from './services/session'; import sessionService from './services/session';
...@@ -227,6 +207,7 @@ function startAngularApp(config) { ...@@ -227,6 +207,7 @@ function startAngularApp(config) {
.register('permissions', permissionsService) .register('permissions', permissionsService)
.register('persistedDefaults', persistedDefaultsService) .register('persistedDefaults', persistedDefaultsService)
.register('rootThread', rootThreadService) .register('rootThread', rootThreadService)
.register('router', routerService)
.register('searchFilter', searchFilterService) .register('searchFilter', searchFilterService)
.register('serviceUrl', serviceUrlService) .register('serviceUrl', serviceUrlService)
.register('session', sessionService) .register('session', sessionService)
...@@ -260,7 +241,7 @@ function startAngularApp(config) { ...@@ -260,7 +241,7 @@ function startAngularApp(config) {
const wrapComponent = component => wrapReactComponent(component, container); const wrapComponent = component => wrapReactComponent(component, container);
angular angular
.module('h', [angularRoute, angularToastr]) .module('h', [angularToastr])
// The root component for the application // The root component for the application
.component('hypothesisApp', hypothesisApp) .component('hypothesisApp', hypothesisApp)
...@@ -315,6 +296,7 @@ function startAngularApp(config) { ...@@ -315,6 +296,7 @@ function startAngularApp(config) {
.service('permissions', () => container.get('permissions')) .service('permissions', () => container.get('permissions'))
.service('persistedDefaults', () => container.get('persistedDefaults')) .service('persistedDefaults', () => container.get('persistedDefaults'))
.service('rootThread', () => container.get('rootThread')) .service('rootThread', () => container.get('rootThread'))
.service('router', () => container.get('router'))
.service('searchFilter', () => container.get('searchFilter')) .service('searchFilter', () => container.get('searchFilter'))
.service('serviceUrl', () => container.get('serviceUrl')) .service('serviceUrl', () => container.get('serviceUrl'))
.service('session', () => container.get('session')) .service('session', () => container.get('session'))
...@@ -329,7 +311,6 @@ function startAngularApp(config) { ...@@ -329,7 +311,6 @@ function startAngularApp(config) {
.value('settings', container.get('settings')) .value('settings', container.get('settings'))
.config(configureLocation) .config(configureLocation)
.config(configureRoutes)
.config(configureToastr) .config(configureToastr)
// Make Angular built-ins available to services constructed by `container`. // Make Angular built-ins available to services constructed by `container`.
...@@ -339,6 +320,7 @@ function startAngularApp(config) { ...@@ -339,6 +320,7 @@ function startAngularApp(config) {
.run(autosave) .run(autosave)
.run(sendPageView) .run(sendPageView)
.run(setupApi) .run(setupApi)
.run(setupRoute)
.run(crossOriginRPC.server.start); .run(crossOriginRPC.server.start);
// Work around a check in Angular's $sniffer service that causes it to // Work around a check in Angular's $sniffer service that causes it to
......
...@@ -72,7 +72,7 @@ export default function RootThread( ...@@ -72,7 +72,7 @@ export default function RootThread(
} }
let threadFilterFn; let threadFilterFn;
if (state.viewer.isSidebar && !shouldFilterThread()) { if (state.route.name === 'sidebar' && !shouldFilterThread()) {
threadFilterFn = function(thread) { threadFilterFn = function(thread) {
if (!thread.annotation) { if (!thread.annotation) {
return false; return false;
......
import * as queryString from 'query-string';
/**
* A service that manages the association between the route and route parameters
* implied by the URL and the corresponding route state in the store.
*/
// @ngInject
export default function router($window, store) {
/**
* Return the name and parameters of the current route.
*/
function currentRoute() {
const path = $window.location.pathname;
const pathSegments = path.slice(1).split('/');
const params = queryString.parse($window.location.search);
let route;
if (pathSegments[0] === 'a') {
route = 'annotation';
params.id = pathSegments[1] || '';
} else if (pathSegments[0] === 'stream') {
route = 'stream';
} else {
route = 'sidebar';
}
return { route, params };
}
/**
* Generate a URL for a given route.
*/
function routeUrl(name, params = {}) {
let url;
const queryParams = { ...params };
if (name === 'annotation') {
const id = params.id;
delete queryParams.id;
url = `/a/${id}`;
} else if (name === 'stream') {
url = '/stream';
} else {
throw new Error(`Cannot generate URL for route "${name}"`);
}
const query = queryString.stringify(queryParams);
if (query.length > 0) {
url += '?' + query;
}
return url;
}
/**
* Synchronize the route name and parameters in the store with the current
* URL.
*/
function sync() {
const { route, params } = currentRoute();
store.changeRoute(route, params);
}
/**
* Navigate to a given route.
*
* @param {string} name
* @param {Object} params
*/
function navigate(name, params) {
$window.history.pushState({}, '', routeUrl(name, params));
sync();
}
// Handle back/forward navigation.
$window.addEventListener('popstate', () => {
// All the state we need to update the route is contained in the URL, which
// has already been updated at this point, so just sync the store route
// to match the URL.
sync();
});
return { sync, navigate };
}
...@@ -41,7 +41,6 @@ describe('rootThread', function() { ...@@ -41,7 +41,6 @@ describe('rootThread', function() {
annotations: [], annotations: [],
}, },
viewer: { viewer: {
isSidebar: true,
visibleHighlights: false, visibleHighlights: false,
}, },
drafts: [], drafts: [],
...@@ -55,6 +54,10 @@ describe('rootThread', function() { ...@@ -55,6 +54,10 @@ describe('rootThread', function() {
sortKey: 'Location', sortKey: 'Location',
sortKeysAvailable: ['Location'], sortKeysAvailable: ['Location'],
}, },
route: {
name: 'sidebar',
params: {},
},
}, },
getState: function() { getState: function() {
return this.state; return this.state;
...@@ -294,7 +297,7 @@ describe('rootThread', function() { ...@@ -294,7 +297,7 @@ describe('rootThread', function() {
it('does not filter annotations when not in the sidebar', function() { it('does not filter annotations when not in the sidebar', function() {
fakeBuildThread.reset(); fakeBuildThread.reset();
fakeStore.state.viewer.isSidebar = false; fakeStore.state.route.name = 'stream';
rootThread.thread(fakeStore.state); rootThread.thread(fakeStore.state);
const threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn; const threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn;
......
import EventEmitter from 'tiny-emitter';
import router from '../router';
const fixtures = [
{
path: '/app.html',
route: 'sidebar',
params: {},
},
{
path: '/a/foo',
route: 'annotation',
params: { id: 'foo' },
},
{
path: '/stream',
search: 'q=foobar',
route: 'stream',
params: { q: 'foobar' },
},
];
describe('router', () => {
let fakeWindow;
let fakeStore;
function createService() {
return router(fakeWindow, fakeStore);
}
function updateUrl(path, search) {
fakeWindow.location.pathname = path;
fakeWindow.location.search = search;
}
beforeEach(() => {
const emitter = new EventEmitter();
fakeWindow = {
location: {
pathname: '',
search: '',
},
history: {
pushState: sinon.stub(),
},
addEventListener: emitter.on.bind(emitter),
emit: emitter.emit.bind(emitter),
};
fakeStore = {
changeRoute: sinon.stub(),
};
});
describe('#sync', () => {
fixtures.forEach(({ path, search, route, params }) => {
it('updates the active route in the store', () => {
updateUrl(path, search);
const svc = createService();
svc.sync();
assert.calledWith(fakeStore.changeRoute, route, params);
});
});
});
describe('#navigate', () => {
fixtures.forEach(({ path, search, route, params }) => {
if (route === 'sidebar') {
// You can't navigate _to_ the sidebar from another route.
return;
}
it('updates the URL', () => {
const svc = createService();
svc.navigate(route, params);
const expectedUrl = path + (search ? `?${search}` : '');
assert.calledWith(fakeWindow.history.pushState, {}, '', expectedUrl);
});
});
it('throws an error if route does not have a fixed URL', () => {
const svc = createService();
assert.throws(() => {
svc.navigate('sidebar');
}, 'Cannot generate URL for route "sidebar"');
});
it('updates the active route in the store', () => {
const svc = createService();
updateUrl('/stream', 'q=foobar');
svc.navigate('stream', { q: 'foobar' });
assert.calledWith(fakeStore.changeRoute, 'stream', { q: 'foobar' });
});
});
context('when a browser history navigation happens', () => {
fixtures.forEach(({ path, search, route, params }) => {
it('updates the active route in the store', () => {
createService();
updateUrl(path, search);
fakeWindow.emit('popstate');
assert.calledWith(fakeStore.changeRoute, route, params);
});
});
});
});
...@@ -40,6 +40,7 @@ import frames from './modules/frames'; ...@@ -40,6 +40,7 @@ import frames from './modules/frames';
import groups from './modules/groups'; import groups from './modules/groups';
import links from './modules/links'; import links from './modules/links';
import realTimeUpdates from './modules/real-time-updates'; import realTimeUpdates from './modules/real-time-updates';
import route from './modules/route';
import selection from './modules/selection'; import selection from './modules/selection';
import session from './modules/session'; import session from './modules/session';
import sidebarPanels from './modules/sidebar-panels'; import sidebarPanels from './modules/sidebar-panels';
...@@ -95,6 +96,7 @@ export default function store($rootScope, settings) { ...@@ -95,6 +96,7 @@ export default function store($rootScope, settings) {
links, links,
groups, groups,
realTimeUpdates, realTimeUpdates,
route,
selection, selection,
session, session,
sidebarPanels, sidebarPanels,
......
...@@ -9,6 +9,8 @@ import * as metadata from '../../util/annotation-metadata'; ...@@ -9,6 +9,8 @@ import * as metadata from '../../util/annotation-metadata';
import * as arrayUtil from '../../util/array'; import * as arrayUtil from '../../util/array';
import * as util from '../util'; import * as util from '../util';
import route from './route';
/** /**
* Return a copy of `current` with all matching annotations in `annotations` * Return a copy of `current` with all matching annotations in `annotations`
* removed. * removed.
...@@ -241,7 +243,7 @@ function addAnnotations(annotations) { ...@@ -241,7 +243,7 @@ function addAnnotations(annotations) {
// If we're not in the sidebar, we're done here. // If we're not in the sidebar, we're done here.
// FIXME Split the annotation-adding from the anchoring code; possibly // FIXME Split the annotation-adding from the anchoring code; possibly
// move into service // move into service
if (!getState().viewer.isSidebar) { if (route.selectors.route(getState()) !== 'sidebar') {
return; return;
} }
......
...@@ -9,7 +9,7 @@ import { actionTypes } from '../util'; ...@@ -9,7 +9,7 @@ import { actionTypes } from '../util';
import annotations from './annotations'; import annotations from './annotations';
import groups from './groups'; import groups from './groups';
import viewer from './viewer'; import route from './route';
function init() { function init() {
return { return {
...@@ -96,7 +96,7 @@ function receiveRealTimeUpdates({ ...@@ -96,7 +96,7 @@ function receiveRealTimeUpdates({
// when switching groups. // when switching groups.
if ( if (
ann.group === groups.selectors.focusedGroupId(getState()) || ann.group === groups.selectors.focusedGroupId(getState()) ||
!viewer.selectors.isSidebar(getState()) route.selectors.route(getState()) !== 'sidebar'
) { ) {
pendingUpdates[ann.id] = ann; pendingUpdates[ann.id] = ann;
} }
......
import { actionTypes } from '../util';
function init() {
return {
/**
* The current route.
* One of null (if no route active yet), "sidebar", "annotation" or "stream".
*/
name: null,
/**
* Parameters of the current route.
*
* - The "annotation" route has an "id" (annotation ID) parameter.
* - The "stream" route has a "q" (query) parameter.
* - The "sidebar" route has no parameters.
*/
params: {},
};
}
const update = {
CHANGE_ROUTE(state, { name, params }) {
return { name, params };
},
};
const actions = actionTypes(update);
/**
* Change the active route.
*
* @param {string} name - Name of the route to activate. See `init` for possible values
* @param {Object} params - Parameters associated with the route
*/
function changeRoute(name, params = {}) {
return {
type: actions.CHANGE_ROUTE,
name,
params,
};
}
/**
* Return the name of the current route.
*/
function route(state) {
return state.route.name;
}
/**
* Return any parameters for the current route, extracted from the path and
* query string.
*/
function routeParams(state) {
return state.route.params;
}
export default {
init,
namespace: 'route',
update,
actions: {
changeRoute,
},
selectors: {
route,
routeParams,
},
};
...@@ -2,12 +2,12 @@ import * as fixtures from '../../../test/annotation-fixtures'; ...@@ -2,12 +2,12 @@ import * as fixtures from '../../../test/annotation-fixtures';
import * as metadata from '../../../util/annotation-metadata'; import * as metadata from '../../../util/annotation-metadata';
import createStore from '../../create-store'; import createStore from '../../create-store';
import annotations from '../annotations'; import annotations from '../annotations';
import viewer from '../viewer'; import route from '../route';
const { actions, selectors } = annotations; const { actions, selectors } = annotations;
function createTestStore() { function createTestStore() {
return createStore([annotations, viewer], [{}]); return createStore([annotations, route], [{}]);
} }
// Tests for most of the functionality in reducers/annotations.js are currently // Tests for most of the functionality in reducers/annotations.js are currently
...@@ -30,6 +30,7 @@ describe('sidebar/store/modules/annotations', function() { ...@@ -30,6 +30,7 @@ describe('sidebar/store/modules/annotations', function() {
beforeEach(function() { beforeEach(function() {
clock = sinon.useFakeTimers(); clock = sinon.useFakeTimers();
store = createTestStore(); store = createTestStore();
store.changeRoute('sidebar', {});
}); });
afterEach(function() { afterEach(function() {
...@@ -134,7 +135,7 @@ describe('sidebar/store/modules/annotations', function() { ...@@ -134,7 +135,7 @@ describe('sidebar/store/modules/annotations', function() {
}; };
const annot = fixtures.defaultAnnotation(); const annot = fixtures.defaultAnnotation();
store.setAppIsSidebar(false); store.changeRoute('stream', { q: 'a-query' });
store.addAnnotations([annot]); store.addAnnotations([annot]);
clock.tick(ANCHOR_TIME_LIMIT); clock.tick(ANCHOR_TIME_LIMIT);
......
...@@ -11,14 +11,14 @@ const { focusGroup } = groups.actions; ...@@ -11,14 +11,14 @@ const { focusGroup } = groups.actions;
describe('sidebar/store/modules/real-time-updates', () => { describe('sidebar/store/modules/real-time-updates', () => {
let fakeAnnotationExists; let fakeAnnotationExists;
let fakeFocusedGroupId; let fakeFocusedGroupId;
let fakeIsSidebar; let fakeRoute;
let fakeSettings = {}; let fakeSettings = {};
let store; let store;
beforeEach(() => { beforeEach(() => {
fakeAnnotationExists = sinon.stub().returns(true); fakeAnnotationExists = sinon.stub().returns(true);
fakeFocusedGroupId = sinon.stub().returns('group-1'); fakeFocusedGroupId = sinon.stub().returns('group-1');
fakeIsSidebar = sinon.stub().returns(true); fakeRoute = sinon.stub().returns('sidebar');
store = createStore( store = createStore(
[realTimeUpdates, annotations, selection], [realTimeUpdates, annotations, selection],
...@@ -36,9 +36,9 @@ describe('sidebar/store/modules/real-time-updates', () => { ...@@ -36,9 +36,9 @@ describe('sidebar/store/modules/real-time-updates', () => {
selectors: { focusedGroupId: fakeFocusedGroupId }, selectors: { focusedGroupId: fakeFocusedGroupId },
}, },
}, },
'./viewer': { './route': {
default: { default: {
selectors: { isSidebar: fakeIsSidebar }, selectors: { route: fakeRoute },
}, },
}, },
}); });
...@@ -84,7 +84,7 @@ describe('sidebar/store/modules/real-time-updates', () => { ...@@ -84,7 +84,7 @@ describe('sidebar/store/modules/real-time-updates', () => {
it('always adds pending updates in the stream where there is no focused group', () => { it('always adds pending updates in the stream where there is no focused group', () => {
fakeFocusedGroupId.returns(null); fakeFocusedGroupId.returns(null);
fakeIsSidebar.returns(false); fakeRoute.returns('stream');
addPendingUpdates(store); addPendingUpdates(store);
......
...@@ -7,17 +7,8 @@ describe('store/modules/viewer', function() { ...@@ -7,17 +7,8 @@ describe('store/modules/viewer', function() {
beforeEach(() => { beforeEach(() => {
store = createStore([viewer]); store = createStore([viewer]);
}); });
describe('#setAppIsSidebar', function() {
it('sets a flag indicating that the app is not the sidebar', function() {
store.setAppIsSidebar(false);
assert.isFalse(store.isSidebar());
});
it('sets a flag indicating that the app is the sidebar', function() {
store.setAppIsSidebar(true);
assert.isTrue(store.isSidebar());
});
describe('#setShowHighlights', function() {
it('sets a flag indicating that highlights are visible', function() { it('sets a flag indicating that highlights are visible', function() {
store.setShowHighlights(true); store.setShowHighlights(true);
assert.isTrue(store.getState().viewer.visibleHighlights); assert.isTrue(store.getState().viewer.visibleHighlights);
......
...@@ -7,21 +7,11 @@ import * as util from '../util'; ...@@ -7,21 +7,11 @@ import * as util from '../util';
function init() { function init() {
return { return {
// Flag that indicates whether the app is the sidebar and connected to
// a page where annotations are being shown in context.
//
// Note that this flag is not available early in the lifecycle of the
// application.
isSidebar: true,
visibleHighlights: false, visibleHighlights: false,
}; };
} }
const update = { const update = {
SET_SIDEBAR: function(state, action) {
return { isSidebar: action.isSidebar };
},
SET_HIGHLIGHTS_VISIBLE: function(state, action) { SET_HIGHLIGHTS_VISIBLE: function(state, action) {
return { visibleHighlights: action.visible }; return { visibleHighlights: action.visible };
}, },
...@@ -29,11 +19,6 @@ const update = { ...@@ -29,11 +19,6 @@ const update = {
const actions = util.actionTypes(update); const actions = util.actionTypes(update);
/** Set whether the app is the sidebar */
function setAppIsSidebar(isSidebar) {
return { type: actions.SET_SIDEBAR, isSidebar: isSidebar };
}
/** /**
* Sets whether annotation highlights in connected documents are shown * Sets whether annotation highlights in connected documents are shown
* or not. * or not.
...@@ -42,24 +27,12 @@ function setShowHighlights(show) { ...@@ -42,24 +27,12 @@ function setShowHighlights(show) {
return { type: actions.SET_HIGHLIGHTS_VISIBLE, visible: show }; return { type: actions.SET_HIGHLIGHTS_VISIBLE, visible: show };
} }
/**
* Returns true if the app is being used as the sidebar in the annotation
* client, as opposed to the standalone annotation page or stream views.
*/
function isSidebar(state) {
return state.viewer.isSidebar;
}
export default { export default {
init: init, init: init,
namespace: 'viewer', namespace: 'viewer',
update: update, update: update,
actions: { actions: {
setAppIsSidebar: setAppIsSidebar,
setShowHighlights: setShowHighlights, setShowHighlights: setShowHighlights,
}, },
selectors: {},
selectors: {
isSidebar,
},
}; };
...@@ -10,6 +10,15 @@ ...@@ -10,6 +10,15 @@
<div class="content"> <div class="content">
<help-panel auth="vm.auth"></help-panel> <help-panel auth="vm.auth"></help-panel>
<share-annotations-panel></share-annotations-panel> <share-annotations-panel></share-annotations-panel>
<main ng-view=""></main>
<main ng-if="vm.route()">
<annotation-viewer-content ng-if="vm.route() == 'annotation'"></annotation-viewer-content>
<stream-content ng-if="vm.route() == 'stream'"></stream-content>
<sidebar-content
ng-if="vm.route() == 'sidebar'"
auth="vm.auth"
on-login="vm.login()"
on-sign-up="vm.signUp()"></sidebar-content>
</main>
</div> </div>
</div> </div>
...@@ -1140,11 +1140,6 @@ angular-mocks@^1.7.5: ...@@ -1140,11 +1140,6 @@ angular-mocks@^1.7.5:
resolved "https://registry.yarnpkg.com/angular-mocks/-/angular-mocks-1.7.9.tgz#0a3b7e28b9a493b4e3010ed2b0f69a68e9b4f79b" resolved "https://registry.yarnpkg.com/angular-mocks/-/angular-mocks-1.7.9.tgz#0a3b7e28b9a493b4e3010ed2b0f69a68e9b4f79b"
integrity sha512-LQRqqiV3sZ7NTHBnNmLT0bXtE5e81t97+hkJ56oU0k3dqKv1s6F+nBWRlOVzqHWPGFOiPS8ZJVdrS8DFzHyNIA== integrity sha512-LQRqqiV3sZ7NTHBnNmLT0bXtE5e81t97+hkJ56oU0k3dqKv1s6F+nBWRlOVzqHWPGFOiPS8ZJVdrS8DFzHyNIA==
angular-route@^1.7.5:
version "1.7.9"
resolved "https://registry.yarnpkg.com/angular-route/-/angular-route-1.7.9.tgz#f9910a2af0ba3ad7a969c5dd369b8360d0d5e4ef"
integrity sha512-vRoj5hzdQtWbODhWJqDzD1iNOEfCKshO6GFBuPVV7RHlPjzIc4R2dHCc7Qiv/8F3LDxJDohc6vSnTDMLHuaqeA==
angular-toastr@^2.1.1: angular-toastr@^2.1.1:
version "2.1.1" version "2.1.1"
resolved "https://registry.yarnpkg.com/angular-toastr/-/angular-toastr-2.1.1.tgz#9f8350ca482145a44d011a755b8fb3623d60544c" resolved "https://registry.yarnpkg.com/angular-toastr/-/angular-toastr-2.1.1.tgz#9f8350ca482145a44d011a755b8fb3623d60544c"
......
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