Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename options to config in annotator #404

Merged
merged 4 commits into from
May 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/annotator/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ var docs = 'https://h.readthedocs.io/en/latest/embedding.html';
*
* @param {Window} window_ - The Window object to read config from.
*/
function config(window_) {
var options = {
function configFrom(window_) {
var config = {
app: window_.
document.querySelector('link[type="application/annotator+html"]').href,
};

// Parse config from `<script class="js-hypothesis-config">` tags
try {
Object.assign(options, settings(window_.document));
Object.assign(config, settings(window_.document));
} catch (err) {
console.warn('Could not parse settings from js-hypothesis-config tags',
err);
Expand All @@ -27,16 +27,16 @@ function config(window_) {
// Parse config from `window.hypothesisConfig` function
if (window_.hasOwnProperty('hypothesisConfig')) {
if (typeof window_.hypothesisConfig === 'function') {
Object.assign(options, window_.hypothesisConfig());
Object.assign(config, window_.hypothesisConfig());
} else {
throw new TypeError('hypothesisConfig must be a function, see: ' + docs);
}
}

// Convert legacy keys/values in options to corresponding current
// Convert legacy keys/values in config to corresponding current
// configuration.
if (typeof options.showHighlights === 'boolean') {
options.showHighlights = options.showHighlights ? 'always' : 'never';
if (typeof config.showHighlights === 'boolean') {
config.showHighlights = config.showHighlights ? 'always' : 'never';
}

// Extract the default query from the URL.
Expand All @@ -49,9 +49,9 @@ function config(window_) {
// we try to retrieve it from the URL here.
var directLinkedID = annotationQuery.extractAnnotationQuery(window_.location.href);
if (directLinkedID) {
Object.assign(options, directLinkedID);
Object.assign(config, directLinkedID);
}
return options;
return config;
}

module.exports = config;
module.exports = configFrom;
6 changes: 3 additions & 3 deletions src/annotator/delegator.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ module.exports = class Delegator
# hash and extends the @options object.
#
# element - The DOM element that this instance represents.
# options - An Object literal of options.
# config - An Object literal of config settings.
#
# Examples
#
Expand All @@ -40,8 +40,8 @@ module.exports = class Delegator
# })
#
# Returns a new instance of Delegator.
constructor: (element, options) ->
@options = $.extend(true, {}, @options, options)
constructor: (element, config) ->
@options = $.extend(true, {}, @options, config)
@element = $(element)

# Delegator creates closures for each event it binds. This is a private
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/guest.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ module.exports = class Guest extends Delegator
html:
adder: '<hypothesis-adder></hypothesis-adder>'

constructor: (element, options) ->
constructor: (element, config) ->
super

this.adder = $(this.html.adder).appendTo(@element).hide()
Expand Down
29 changes: 15 additions & 14 deletions src/annotator/host.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ $ = require('jquery')
Guest = require('./guest')

module.exports = class Host extends Guest
constructor: (element, options) ->
constructor: (element, config) ->

# Some options' values are not JSON-stringifiable (e.g. JavaScript
# functions) and will be omitted when the options are JSON-stringified.
# Some config settings are not JSON-stringifiable (e.g. JavaScript
# functions) and will be omitted when the config is JSON-stringified.
# Add a JSON-stringifiable option for each of these so that the sidebar can
# at least know whether the callback functions were provided or not.
if options.services?[0]
service = options.services[0]
if config.services?[0]
service = config.services[0]
if service.onLoginRequest
service.onLoginRequestProvided = true
if service.onLogoutRequest
Expand All @@ -22,22 +22,23 @@ module.exports = class Host extends Guest
if service.onHelpRequest
service.onHelpRequestProvided = true

# Make a copy of all options except `options.app`, the app base URL, and `options.pluginClasses`
# Make a copy of all config settings except `config.app`, the app base URL,
# and `config.pluginClasses`
configParam = 'config=' + encodeURIComponent(
JSON.stringify(Object.assign({}, options, {app:undefined, pluginClasses: undefined }))
JSON.stringify(Object.assign({}, config, {app:undefined, pluginClasses: undefined }))
)
if options.app and '?' in options.app
options.app += '&' + configParam
if config.app and '?' in config.app
config.app += '&' + configParam
else
options.app += '?' + configParam
config.app += '?' + configParam

# Create the iframe
app = $('<iframe></iframe>')
.attr('name', 'hyp_sidebar_frame')
# enable media in annotations to be shown fullscreen
.attr('allowfullscreen', '')
.attr('seamless', '')
.attr('src', options.app)
.attr('src', config.app)
.addClass('h-sidebar-iframe')

@frame = $('<div></div>')
Expand All @@ -51,10 +52,10 @@ module.exports = class Host extends Guest

this.on 'panelReady', =>
# Initialize tool state.
if options.showHighlights == undefined
if config.showHighlights == undefined
# Highlights are on by default.
options.showHighlights = 'always'
this.setVisibleHighlights(options.showHighlights == 'always')
config.showHighlights = 'always'
this.setVisibleHighlights(config.showHighlights == 'always')

# Show the UI
@frame.css('display', '')
Expand Down
13 changes: 7 additions & 6 deletions src/annotator/main.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

var configFrom = require('./config');
require('../shared/polyfills');


Expand Down Expand Up @@ -37,20 +38,20 @@ var pluginClasses = {

var appLinkEl =
document.querySelector('link[type="application/annotator+html"]');
var options = require('./config')(window);
var config = configFrom(window);

$.noConflict(true)(function() {
var Klass = window.PDFViewerApplication ?
PdfSidebar :
Sidebar;
if (options.hasOwnProperty('constructor')) {
Klass = options.constructor;
delete options.constructor;
if (config.hasOwnProperty('constructor')) {
Klass = config.constructor;
delete config.constructor;
}

options.pluginClasses = pluginClasses;
config.pluginClasses = pluginClasses;

window.annotator = new Klass(document.body, options);
window.annotator = new Klass(document.body, config);
appLinkEl.addEventListener('destroy', function () {
appLinkEl.parentElement.removeChild(appLinkEl);
window.annotator.destroy();
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/plugin/bucket-bar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ module.exports = class BucketBar extends Plugin
</div>
"""

# Plugin configuration
# Plugin options
options:
# gapSize parameter is used by the clustering algorithm
# If an annotation is farther then this gapSize from the next bucket
Expand Down
6 changes: 3 additions & 3 deletions src/annotator/sidebar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ module.exports = class Sidebar extends Host
renderFrame: null
gestureState: null

constructor: (element, options) ->
constructor: (element, config) ->
super
this.hide()

if options.openSidebar || options.annotations || options.query
if config.openSidebar || config.annotations || config.query
this.on 'panelReady', => this.show()

if @plugins.BucketBar?
Expand All @@ -37,7 +37,7 @@ module.exports = class Sidebar extends Host
this._setupGestures()

# The partner-provided callback functions.
serviceConfig = options.services?[0]
serviceConfig = config.services?[0]
if serviceConfig
@onLoginRequest = serviceConfig.onLoginRequest
@onLogoutRequest = serviceConfig.onLogoutRequest
Expand Down
58 changes: 29 additions & 29 deletions src/annotator/test/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var proxyquire = require('proxyquire');
var fakeSettings = sinon.stub();
var fakeExtractAnnotationQuery = {};

var config = proxyquire('../config', {
var configFrom = proxyquire('../config', {
'../shared/settings': fakeSettings,
'./util/extract-annotation-query': fakeExtractAnnotationQuery,
});
Expand Down Expand Up @@ -52,8 +52,8 @@ describe('annotator.config', function() {
document.head.removeChild(link);
});

it("returns the <link>'s href as options.app", function() {
assert.equal(config(window).app, link.href);
it("returns the <link>'s href as config.app", function() {
assert.equal(configFrom(window).app, link.href);
});
});

Expand All @@ -62,28 +62,28 @@ describe('annotator.config', function() {
var window_ = fakeWindow();
window_.document.querySelector.returns(null);

assert.throws(function() { config(window_); }, TypeError);
assert.throws(function() { configFrom(window_); }, TypeError);
});
});

it('gets the JSON settings from the document', function() {
var window_ = fakeWindow();

config(window_);
configFrom(window_);

assert.calledOnce(fakeSettings);
assert.calledWithExactly(fakeSettings, window_.document);
});

context('when settings() returns a non-empty object', function() {
it('reads the setting into the returned options', function() {
// config() just blindly adds any key: value settings that settings()
// returns into the returns options object.
it('reads the setting into the returned config', function() {
// configFrom() just blindly adds any key: value settings that settings()
// returns into the returned config object.
fakeSettings.returns({foo: 'bar'});

var options = config(fakeWindow());
var config = configFrom(fakeWindow());

assert.equal(options.foo, 'bar');
assert.equal(config.foo, 'bar');
});
});

Expand All @@ -93,24 +93,24 @@ describe('annotator.config', function() {
});

it('catches the error', function() {
config(fakeWindow());
configFrom(fakeWindow());
});

it('logs a warning', function() {
config(fakeWindow());
configFrom(fakeWindow());

assert.called(console.warn);
});
});

context("when there's a window.hypothesisConfig() function", function() {
it('reads arbitrary settings from hypothesisConfig() into options', function() {
it('reads arbitrary settings from hypothesisConfig() into config', function() {
var window_ = fakeWindow();
window_.hypothesisConfig = sinon.stub().returns({foo: 'bar'});

var options = config(window_);
var config = configFrom(window_);

assert.equal(options.foo, 'bar');
assert.equal(config.foo, 'bar');
});

specify('hypothesisConfig() settings override js-hypothesis-config ones', function() {
Expand All @@ -119,20 +119,20 @@ describe('annotator.config', function() {
foo: 'fooFromHypothesisConfigFunc'});
fakeSettings.returns({foo: 'fooFromJSHypothesisConfigObj'});

var options = config(window_);
var config = configFrom(window_);

assert.equal(options.foo, 'fooFromHypothesisConfigFunc');
assert.equal(config.foo, 'fooFromHypothesisConfigFunc');
});

context('if hypothesisConfig() returns a non-object value', function() {
it("doesn't add anything into the options", function() {
it("doesn't add anything into the config", function() {
var window_ = fakeWindow();
window_.hypothesisConfig = sinon.stub().returns(42);

var options = config(window_);
var config = configFrom(window_);

delete options.app; // We don't care about options.app for this test.
assert.deepEqual({}, options);
delete config.app; // We don't care about config.app for this test.
assert.deepEqual({}, config);
});
});
});
Expand All @@ -143,7 +143,7 @@ describe('annotator.config', function() {
window_.hypothesisConfig = 'notAFunction';

assert.throws(
function() { config(window_); }, TypeError,
function() { configFrom(window_); }, TypeError,
'hypothesisConfig must be a function, see: https://h.readthedocs.io/en/latest/embedding.html'
);
});
Expand All @@ -162,7 +162,7 @@ describe('annotator.config', function() {
out: 'never',
},
// It adds any arbitrary string value for showHighlights to the
// returned options, unmodified.
// returned config, unmodified.
{
name: 'passes arbitrary strings through unmodified',
in: 'foo',
Expand All @@ -172,15 +172,15 @@ describe('annotator.config', function() {
it(test.name, function() {
fakeSettings.returns({showHighlights: test.in});

var options = config(fakeWindow());
var config = configFrom(fakeWindow());

assert.equal(options.showHighlights, test.out);
assert.equal(config.showHighlights, test.out);
});
});
});

it("extracts the annotation query from the parent page's URL", function() {
config(fakeWindow());
configFrom(fakeWindow());

assert.calledOnce(fakeExtractAnnotationQuery.extractAnnotationQuery);
assert.calledWithExactly(
Expand All @@ -194,8 +194,8 @@ describe('annotator.config', function() {
});
});

it('blindly adds the properties of the object to the options', function() {
assert.equal(config(fakeWindow()).foo, 'bar');
it('blindly adds the properties of the object to the config', function() {
assert.equal(configFrom(fakeWindow()).foo, 'bar');
});

specify('settings from extractAnnotationQuery override others', function() {
Expand All @@ -210,7 +210,7 @@ describe('annotator.config', function() {
foo: 'fromHypothesisConfig',
});

assert.equal(config(window_).foo, 'fromExtractAnnotationQuery');
assert.equal(configFrom(window_).foo, 'fromExtractAnnotationQuery');
});
});
});
Loading