Skip to content

Commit

Permalink
Rename options to config in src/annotator/
Browse files Browse the repository at this point in the history
There are two reasons for renaming the `options` object to `config`:

**First**, it's more consistent. The file that creates this object is
called `config.js`, and the function that creates the object is called
`configure()`, and yet the object ends up getting called `options` (but
you have to look in another file, `main.js`, to find this out).

`config` is also used elsewhere as the name for the main configuration
object, for example in Pyramid/h, in the client docs ("Configuring the
Client", "configuration settings") and public API
(`class="js-hypothesis-config"` scripts and `window.hypothesisConfig()`
functions), etc.

These "options" that the `src/annotator/` code reads from the host page
also end up getting renamed to `hostPageConfig` when they get passed
over in to the `src/sidebar/` code.

**Second**, it's more unique. There are a number of other objects in the
`src/annotator/` code that are called options, sometimes the main
options object and another local options object are even used in the
same function. There's no other objects called config.
  • Loading branch information
seanh committed May 25, 2017
1 parent 456f3a0 commit 6ba3e48
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 86 deletions.
16 changes: 8 additions & 8 deletions src/annotator/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ var docs = 'https://h.readthedocs.io/en/latest/embedding.html';
* @param {Window} window_ - The Window object to read config from.
*/
function configure(window_) {
var options = {
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 configure(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 configure(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 = configure;
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
12 changes: 6 additions & 6 deletions src/annotator/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,20 @@ var pluginClasses = {

var appLinkEl =
document.querySelector('link[type="application/annotator+html"]');
var options = configure(window);
var config = configure(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
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
36 changes: 18 additions & 18 deletions src/annotator/test/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('annotator.config', function() {
document.head.removeChild(link);
});

it("returns the <link>'s href as options.app", function() {
it("returns the <link>'s href as config.app", function() {
assert.equal(configure(window).app, link.href);
});
});
Expand All @@ -76,14 +76,14 @@ describe('annotator.config', function() {
});

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

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

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

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

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 = configure(window_);
var config = configure(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 = configure(window_);
var config = configure(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 = configure(window_);
var config = configure(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 Down Expand Up @@ -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,9 +172,9 @@ describe('annotator.config', function() {
it(test.name, function() {
fakeSettings.returns({showHighlights: test.in});

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

assert.equal(options.showHighlights, test.out);
assert.equal(config.showHighlights, test.out);
});
});
});
Expand All @@ -194,7 +194,7 @@ describe('annotator.config', function() {
});
});

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

Expand Down
14 changes: 7 additions & 7 deletions src/annotator/test/guest-test.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ describe 'Guest', ->
sandbox = sinon.sandbox.create()
CrossFrame = null
fakeCrossFrame = null
guestOptions = null
guestConfig = null

createGuest = (options={}) ->
options = Object.assign({}, guestOptions, options)
createGuest = (config={}) ->
config = Object.assign({}, guestConfig, config)
element = document.createElement('div')
return new Guest(element, options)
return new Guest(element, config)

beforeEach ->
FakeAdder::instance = null
Expand All @@ -64,7 +64,7 @@ describe 'Guest', ->
selectionFocusRect: sinon.stub()
}
selections = null
guestOptions = {pluginClasses: {}}
guestConfig = {pluginClasses: {}}

Guest = proxyquire('../guest', {
'./adder': {Adder: FakeAdder},
Expand All @@ -89,7 +89,7 @@ describe 'Guest', ->
}

CrossFrame = sandbox.stub().returns(fakeCrossFrame)
guestOptions.pluginClasses['CrossFrame'] = CrossFrame
guestConfig.pluginClasses['CrossFrame'] = CrossFrame

afterEach ->
sandbox.restore()
Expand All @@ -100,7 +100,7 @@ describe 'Guest', ->

beforeEach ->
FakePlugin::instance = null
guestOptions.pluginClasses['FakePlugin'] = FakePlugin
guestConfig.pluginClasses['FakePlugin'] = FakePlugin
guest = createGuest(FakePlugin: {})
fakePlugin = FakePlugin::instance

Expand Down
14 changes: 7 additions & 7 deletions src/annotator/test/host-test.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ describe 'Host', ->
sandbox = sinon.sandbox.create()
CrossFrame = null
fakeCrossFrame = null
hostOptions = {pluginClasses: {}}
hostConfig = {pluginClasses: {}}

createHost = (options={}, element=null) ->
options = Object.assign({app: '/base/annotator/test/empty.html'}, hostOptions, options)
createHost = (config={}, element=null) ->
config = Object.assign({app: '/base/annotator/test/empty.html'}, hostConfig, config)
if !element
element = document.createElement('div')
return new Host(element, options)
return new Host(element, config)

beforeEach ->
# Disable any Host logging.
Expand All @@ -24,7 +24,7 @@ describe 'Host', ->

CrossFrame = sandbox.stub()
CrossFrame.returns(fakeCrossFrame)
hostOptions.pluginClasses['CrossFrame'] = CrossFrame
hostConfig.pluginClasses['CrossFrame'] = CrossFrame

afterEach ->
sandbox.restore()
Expand Down Expand Up @@ -67,7 +67,7 @@ describe 'Host', ->
}])
assert.notCalled(frame.contentWindow.focus)

describe 'options', ->
describe 'config', ->
it 'disables highlighting if showHighlights: false is given', (done) ->
host = createHost(showHighlights: false)
host.on 'panelReady', ->
Expand All @@ -82,7 +82,7 @@ describe 'Host', ->
done()
host.publish('panelReady')

it 'passes options to the sidebar iframe', ->
it 'passes config to the sidebar iframe', ->
appURL = new URL('/base/annotator/test/empty.html', window.location.href)
host = createHost({annotations: '1234'})
configStr = encodeURIComponent(JSON.stringify({annotations: '1234'}))
Expand Down
Loading

0 comments on commit 6ba3e48

Please sign in to comment.