Skip to content

Commit

Permalink
Merge pull request #3085 from hypothesis/open-sidebar-when-annot-frag…
Browse files Browse the repository at this point in the history
…ment-present

(1/3) Open sidebar and set selection when '#annotations' URL fragment is present
  • Loading branch information
nickstenning committed Mar 16, 2016
2 parents afa5143 + 4241f62 commit fb59d11
Show file tree
Hide file tree
Showing 12 changed files with 161 additions and 39 deletions.
16 changes: 15 additions & 1 deletion h/browser/chrome/lib/hypothesis-chrome-extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ var TabStore = require('./tab-store');
var TAB_STATUS_LOADING = 'loading';
var TAB_STATUS_COMPLETE = 'complete';

/**
* Returns true if a tab URL contains a link to an annotation,
* which should result in Hypothesis being automatically injected into
* the tab.
*/
function urlHasAnnotationFragment(url) {
return url.match(/#annotations:(.*)$/);
}

/* The main extension application. This wires together all the smaller
* modules. The app listens to all new created/updated/removed tab events
* and uses the TabState object to keep track of whether the sidebar is
Expand Down Expand Up @@ -54,7 +63,7 @@ function HypothesisChromeExtension(dependencies) {
/* Sets up the extension and binds event listeners. Requires a window
* object to be passed so that it can listen for localStorage events.
*/
this.listen = function (window) {
this.listen = function () {
chromeBrowserAction.onClicked.addListener(onBrowserActionClicked);
chromeTabs.onCreated.addListener(onTabCreated);

Expand Down Expand Up @@ -174,8 +183,13 @@ function HypothesisChromeExtension(dependencies) {
if (changeInfo.status === TAB_STATUS_LOADING) {
resetTabState(tabId, tab.url);
} else if (changeInfo.status === TAB_STATUS_COMPLETE) {
var newActiveState = state.getState(tabId);
if (urlHasAnnotationFragment(tab.url)) {
newActiveState = TabState.states.ACTIVE;
}
state.setState(tabId, {
ready: true,
state: newActiveState,
});
}
}
Expand Down
14 changes: 10 additions & 4 deletions h/browser/chrome/test/hypothesis-chrome-extension-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ describe('HypothesisChromeExtension', function () {
var fakeHelpPage;
var fakeTabStore;
var fakeTabState;
var fakeTabErrorCache;
var fakeBrowserAction;
var fakeSidebarInjector;

Expand Down Expand Up @@ -96,7 +95,7 @@ describe('HypothesisChromeExtension', function () {
};

function FakeTabState(initialState, onchange) {
fakeTabState.onChangeHandler = onchange
fakeTabState.onChangeHandler = onchange;
}
FakeTabState.prototype = fakeTabState;

Expand Down Expand Up @@ -206,13 +205,13 @@ describe('HypothesisChromeExtension', function () {
}

beforeEach(function () {
fakeTabState.clearTab = sandbox.spy()
fakeTabState.clearTab = sandbox.spy();
fakeTabState.isTabActive = function (tabId) {
return tabState[tabId].state === TabState.states.ACTIVE;
};
fakeTabState.isTabErrored = function (tabId) {
return tabState[tabId].state === TabState.states.ERRORED;
}
};
fakeTabState.getState = function (tabId) {
return tabState[tabId];
};
Expand Down Expand Up @@ -244,6 +243,13 @@ describe('HypothesisChromeExtension', function () {
fakeChromeTabs.onUpdated.listener(tab.id, {status: 'loading'}, tab);
assert.equal(tabState[tab.id].state, TabState.states.ACTIVE);
});

it('injects the sidebar if a direct link is present', function () {
var tab = createTab();
tab.url += '#annotations:456';
fakeChromeTabs.onUpdated.listener(tab.id, {status: 'complete'}, tab);
assert.equal(tabState[tab.id].state, TabState.states.ACTIVE);
});
});

describe('when a tab is replaced', function () {
Expand Down
34 changes: 23 additions & 11 deletions h/static/scripts/annotation-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@

function value(selection) {
if (Object.keys(selection).length) {
return selection;
return Object.freeze(selection);
} else {
return null;
}
}

function initialSelection(settings) {
var selection = {};
if (settings.annotations) {
selection[settings.annotations] = true;
}
return value(selection);
}

/**
* Stores the UI state of the annotator in connected clients.
*
Expand All @@ -17,15 +25,17 @@ function value(selection) {
* - The state of the bucket bar
*
*/
module.exports = function () {

// @ngInject
module.exports = function (settings) {
return {
visibleHighlights: false,

// Contains a map of annotation tag:true pairs.
focusedAnnotationMap: null,

// Contains a map of annotation id:true pairs.
selectedAnnotationMap: null,
selectedAnnotationMap: initialSelection(settings),

/**
* @ngdoc method
Expand Down Expand Up @@ -62,17 +72,19 @@ module.exports = function () {
},

/**
* @ngdoc method
* @name annotationUI.selectAnnotations
* @returns nothing
* @description Takes an array of annotation objects and uses them to
* set the selectedAnnotationMap property.
* Set the currently selected annotation IDs.
*
* @param {Array<string|{id:string}>} annotations - Annotations or IDs
* of annotations to select.
*/
selectAnnotations: function (annotations) {
var selection = {};
for (var i = 0, annotation; i < annotations.length; i++) {
annotation = annotations[i];
selection[annotation.id] = true;
for (var i = 0; i < annotations.length; i++) {
if (typeof annotations[i] === 'string') {
selection[annotations[i]] = true;
} else {
selection[annotations[i].id] = true;
}
}
this.selectedAnnotationMap = value(selection);
},
Expand Down
31 changes: 31 additions & 0 deletions h/static/scripts/annotator/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

var docs = 'https://h.readthedocs.org/en/latest/hacking/customized-embedding.html';

/**
* Reads the Hypothesis configuration from the environment.
*
* @param {Window} window_ - The Window object to read config from.
*/
function config(window_) {
var options = {
app: window_.
document.querySelector('link[type="application/annotator+html"]').href,
};

if (window_.hasOwnProperty('hypothesisConfig')) {
if (typeof window_.hypothesisConfig === 'function') {
Object.assign(options, window_.hypothesisConfig());
} else {
throw new TypeError('hypothesisConfig must be a function, see: ' + docs);
}
}

var annotFragmentMatch = window_.location.hash.match(/^#annotations:(.*)/);
if (annotFragmentMatch) {
options.annotations = annotFragmentMatch[1];
}
return options;
}

module.exports = config;
8 changes: 5 additions & 3 deletions h/static/scripts/annotator/host.coffee
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
Annotator = require('annotator')
queryString = require('query-string')
$ = Annotator.$

Guest = require('./guest')


module.exports = class Host extends Guest
constructor: (element, options) ->
if options.firstRun
options.app += (if '?' in options.app then '&' else '?') + 'firstrun'
appOpts = queryString.stringify(
Object.assign({}, options, {app: undefined})
)
options.app += (if '?' in options.app then '&' else '?') + appOpts

# Create the iframe
app = $('<iframe></iframe>')
Expand Down
17 changes: 3 additions & 14 deletions h/static/scripts/annotator/main.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

require('../polyfills');

var extend = require('extend');
var Annotator = require('annotator');

// Polyfills
Expand Down Expand Up @@ -32,23 +33,11 @@ Annotator.Plugin.CrossFrame.AnnotationSync = require('../annotation-sync');
Annotator.Plugin.CrossFrame.Bridge = require('../bridge');
Annotator.Plugin.CrossFrame.Discovery = require('../discovery');

var docs = 'https://h.readthedocs.org/en/latest/hacking/customized-embedding.html';
var appLinkEl =
document.querySelector('link[type="application/annotator+html"]');
var options = {
app: appLinkEl.href,
};

if (window.hasOwnProperty('hypothesisConfig')) {
if (typeof window.hypothesisConfig === 'function') {
extend(options, window.hypothesisConfig());
} else {
throw new TypeError('hypothesisConfig must be a function, see: ' + docs);
}
}
var options = require('./config')(window);

Annotator.noConflict().$.noConflict(true)(function() {
'use strict';
var Klass = window.PDFViewerApplication ?
Annotator.PdfSidebar :
Annotator.Sidebar;
Expand Down
2 changes: 1 addition & 1 deletion h/static/scripts/annotator/sidebar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = class Sidebar extends Host
super
this.hide()

if options.firstRun
if options.firstRun || options.annotations
this.on 'panelReady', => this.show()

if @plugins.BucketBar?
Expand Down
45 changes: 45 additions & 0 deletions h/static/scripts/annotator/test/config-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';

var config = require('../config');

describe('annotator configuration', function () {
var fakeWindowBase = {
document: {
querySelector: sinon.stub().returns({href: 'app.html'}),
},
location: {hash: ''},
};

it('reads the app src from the link tag', function () {
var linkEl = document.createElement('link');
linkEl.type = 'application/annotator+html';
linkEl.href = 'https://test.hypothes.is/app.html';
document.head.appendChild(linkEl);
assert.deepEqual(config(window), {
app: linkEl.href,
});
document.head.removeChild(linkEl);
});

it('reads the #annotation query fragment', function () {
var fakeWindow = Object.assign({}, fakeWindowBase, {
location: {hash:'#annotations:456'},
});
assert.deepEqual(config(fakeWindow), {
app: 'app.html',
annotations: '456',
});
});

it('merges the config from hypothesisConfig()', function () {
var fakeWindow = Object.assign({}, fakeWindowBase, {
hypothesisConfig: function () {
return {firstRun: true};
},
});
assert.deepEqual(config(fakeWindow), {
app: 'app.html',
firstRun: true,
});
});
});
8 changes: 8 additions & 0 deletions h/static/scripts/annotator/test/host-test.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,11 @@ describe 'Host', ->
assert.isTrue(host.visibleHighlights)
done()
host.publish('panelReady')

it 'passes options to the sidebar iframe', ->
appURL = 'http://localhost:1000/app.html'
host = createHost({
app: appURL,
annotations: '1234'
})
assert.equal(host.frame[0].children[0].src, appURL + '?annotations=1234')
6 changes: 2 additions & 4 deletions h/static/scripts/app-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function authStateFromUserID(userid) {
module.exports = function AppController(
$controller, $document, $location, $rootScope, $route, $scope,
$window, annotationUI, auth, drafts, features, groups,
session
session, settings
) {
$controller('AnnotationUIController', {$scope: $scope});

Expand All @@ -42,8 +42,6 @@ module.exports = function AppController(
// Allow all child scopes access to the session
$scope.session = session;

var isFirstRun = $location.search().hasOwnProperty('firstrun');

// App dialogs
$scope.accountDialog = {visible: false};
$scope.shareDialog = {visible: false};
Expand Down Expand Up @@ -73,7 +71,7 @@ module.exports = function AppController(
// update the auth info in the top bar and show the login form
// after first install of the extension.
$scope.auth = authStateFromUserID(state.userid);
if (!state.userid && isFirstRun) {
if (!state.userid && settings.firstRun) {
$scope.login();
}
});
Expand Down
2 changes: 2 additions & 0 deletions h/static/scripts/app.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
'use strict';

require('./polyfills');
var queryString = require('query-string');

// Initialize Raven. This is required at the top of this file
// so that it happens early in the app's startup flow
var settings = require('./settings')(document);
Object.assign(settings, queryString.parse(window.location.search));
if (settings.raven) {
var raven = require('./raven');
raven.init(settings.raven);
Expand Down
17 changes: 16 additions & 1 deletion h/static/scripts/test/annotation-ui-test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
'use strict';

var annotationUIFactory = require('../annotation-ui');

describe('annotationUI', function () {
var annotationUI;

beforeEach(function () {
annotationUI = require('../annotation-ui')();
annotationUI = annotationUIFactory({});
});

describe('initialization', function () {
it('does not set a selection when settings.annotations is null', function () {
assert.isFalse(annotationUI.hasSelectedAnnotations());
});

it('sets the selection when settings.annotations is set', function () {
annotationUI = annotationUIFactory({annotations: 'testid'});
assert.deepEqual(annotationUI.selectedAnnotationMap, {
testid: true,
});
});
});

describe('.focusAnnotations()', function () {
Expand Down

0 comments on commit fb59d11

Please sign in to comment.