Commit 67d0c469 authored by Robert Knight's avatar Robert Knight

Remove jQuery usage from `Delegator` class

- Replace usage of jQuery for the event bus in `Delegator` with
  `tiny-emitter`, which we use in various other places in the application

- Change the `element` property from a jQuery wrapper to a DOM element
  and adapt the places in `Guest` that referenced it

- Remove the `on` alias for the `subscribe` method and change existing
  users to just use `subscribe`

- Convert the one remaining use of the legacy alternative approach to
  passing configuration to `Delegator` to instead pass options in the
  `super(...)` call. This allows removing support for this from the
  `Delegator` constructor.

- Add basic tests for the `Delegator` base class
parent 8fb23743
import $ from 'jquery'; import { TinyEmitter as EventEmitter } from 'tiny-emitter';
// Adapted from: // Adapted from:
// https://github.com/openannotation/annotator/blob/v1.2.x/src/class.coffee // https://github.com/openannotation/annotator/blob/v1.2.x/src/class.coffee
...@@ -31,13 +31,18 @@ export default class Delegator { ...@@ -31,13 +31,18 @@ export default class Delegator {
* @param {Object} [config] * @param {Object} [config]
*/ */
constructor(element, config) { constructor(element, config) {
// Some subclasses rely on a legacy mechanism of defining options at this.options = { ...config };
// the class level by defining an `options` property on the prototype. this.element = element;
const classOptions = this.options || {};
this.options = { ...classOptions, ...config };
this.element = $(element);
this.on = this.subscribe; const el = /** @type {any} */ (element);
let eventBus = el._hypothesisEventBus;
if (!eventBus) {
eventBus = new EventEmitter();
el._hypothesisEventBus = eventBus;
}
this._eventBus = eventBus;
this._subscriptions = [];
} }
/** /**
...@@ -47,7 +52,9 @@ export default class Delegator { ...@@ -47,7 +52,9 @@ export default class Delegator {
* the base implementation. * the base implementation.
*/ */
destroy() { destroy() {
// FIXME - This should unbind any event handlers registered via `subscribe`. for (let [event, callback] of this._subscriptions) {
this._eventBus.off(event, callback);
}
} }
/** /**
...@@ -59,8 +66,8 @@ export default class Delegator { ...@@ -59,8 +66,8 @@ export default class Delegator {
* @param {string} event * @param {string} event
* @param {any[]} [args] * @param {any[]} [args]
*/ */
publish(event, args) { publish(event, args = []) {
this.element.triggerHandler(event, args); this._eventBus.emit(event, ...args);
} }
/** /**
...@@ -70,18 +77,8 @@ export default class Delegator { ...@@ -70,18 +77,8 @@ export default class Delegator {
* @param {Function} callback * @param {Function} callback
*/ */
subscribe(event, callback) { subscribe(event, callback) {
// Wrapper that strips the `event` argument. this._eventBus.on(event, callback);
const closure = (event, ...args) => callback(...args); this._subscriptions.push([event, callback]);
// Ensure both functions have the same unique id so that jQuery will accept
// callback when unbinding closure.
//
// @ts-expect-error - `guid` property is non-standard
closure.guid = callback.guid = $.guid += 1;
// Ignore false positive lint warning about function bind.
// eslint-disable-next-line
this.element.bind(event, closure);
} }
/** /**
...@@ -91,6 +88,10 @@ export default class Delegator { ...@@ -91,6 +88,10 @@ export default class Delegator {
* @param {Function} callback * @param {Function} callback
*/ */
unsubscribe(event, callback) { unsubscribe(event, callback) {
this.element.unbind(event, callback); this._eventBus.off(event, callback);
this._subscriptions = this._subscriptions.filter(
([subEvent, subCallback]) =>
subEvent !== event || subCallback !== callback
);
} }
} }
...@@ -98,7 +98,7 @@ export default class Guest extends Delegator { ...@@ -98,7 +98,7 @@ export default class Guest extends Delegator {
this.adderToolbar = document.createElement('hypothesis-adder'); this.adderToolbar = document.createElement('hypothesis-adder');
this.adderToolbar.style.display = 'none'; this.adderToolbar.style.display = 'none';
this.element[0].appendChild(this.adderToolbar); this.element.appendChild(this.adderToolbar);
this.adderCtrl = new Adder(this.adderToolbar, { this.adderCtrl = new Adder(this.adderToolbar, {
onAnnotate: () => { onAnnotate: () => {
...@@ -170,7 +170,7 @@ export default class Guest extends Delegator { ...@@ -170,7 +170,7 @@ export default class Guest extends Delegator {
// highlights. // highlights.
_setupElementEvents() { _setupElementEvents() {
const addListener = (event, callback) => { const addListener = (event, callback) => {
this.element[0].addEventListener(event, callback); this.element.addEventListener(event, callback);
this._elementEventListeners.push({ event, callback }); this._elementEventListeners.push({ event, callback });
}; };
...@@ -220,13 +220,13 @@ export default class Guest extends Delegator { ...@@ -220,13 +220,13 @@ export default class Guest extends Delegator {
_removeElementEvents() { _removeElementEvents() {
this._elementEventListeners.forEach(({ event, callback }) => { this._elementEventListeners.forEach(({ event, callback }) => {
this.element[0].removeEventListener(event, callback); this.element.removeEventListener(event, callback);
}); });
} }
addPlugin(name, options) { addPlugin(name, options) {
const Klass = this.options.pluginClasses[name]; const Klass = this.options.pluginClasses[name];
this.plugins[name] = new Klass(this.element[0], options); this.plugins[name] = new Klass(this.element, options);
this.plugins[name].annotator = this; this.plugins[name].annotator = this;
this.plugins[name].pluginInit?.(); this.plugins[name].pluginInit?.();
} }
...@@ -299,7 +299,7 @@ export default class Guest extends Delegator { ...@@ -299,7 +299,7 @@ export default class Guest extends Delegator {
cancelable: true, cancelable: true,
detail: anchor.range, detail: anchor.range,
}); });
const defaultNotPrevented = this.element[0].dispatchEvent(event); const defaultNotPrevented = this.element.dispatchEvent(event);
if (defaultNotPrevented) { if (defaultNotPrevented) {
scrollIntoView(anchor.highlights[0]); scrollIntoView(anchor.highlights[0]);
} }
...@@ -324,7 +324,7 @@ export default class Guest extends Delegator { ...@@ -324,7 +324,7 @@ export default class Guest extends Delegator {
this.selections.unsubscribe(); this.selections.unsubscribe();
this.adderToolbar.remove(); this.adderToolbar.remove();
removeAllHighlights(this.element[0]); removeAllHighlights(this.element);
for (let name of Object.keys(this.plugins)) { for (let name of Object.keys(this.plugins)) {
this.plugins[name].destroy(); this.plugins[name].destroy();
...@@ -335,7 +335,7 @@ export default class Guest extends Delegator { ...@@ -335,7 +335,7 @@ export default class Guest extends Delegator {
anchor(annotation) { anchor(annotation) {
let anchor; let anchor;
const root = this.element[0]; const root = this.element;
// Anchors for all annotations are in the `anchors` instance property. These // Anchors for all annotations are in the `anchors` instance property. These
// are anchors for this annotation only. After all the targets have been // are anchors for this annotation only. After all the targets have been
...@@ -488,7 +488,7 @@ export default class Guest extends Delegator { ...@@ -488,7 +488,7 @@ export default class Guest extends Delegator {
} }
createAnnotation(annotation = {}) { createAnnotation(annotation = {}) {
const root = this.element[0]; const root = this.element;
const ranges = this.selectedRanges ?? []; const ranges = this.selectedRanges ?? [];
this.selectedRanges = null; this.selectedRanges = null;
...@@ -598,7 +598,7 @@ export default class Guest extends Delegator { ...@@ -598,7 +598,7 @@ export default class Guest extends Delegator {
// Pass true to show the highlights in the frame or false to disable. // Pass true to show the highlights in the frame or false to disable.
setVisibleHighlights(shouldShowHighlights) { setVisibleHighlights(shouldShowHighlights) {
setHighlightsVisible(this.element[0], shouldShowHighlights); setHighlightsVisible(this.element, shouldShowHighlights);
this.visibleHighlights = shouldShowHighlights; this.visibleHighlights = shouldShowHighlights;
if (this.toolbar) { if (this.toolbar) {
......
...@@ -86,14 +86,14 @@ export default class Host extends Guest { ...@@ -86,14 +86,14 @@ export default class Host extends Guest {
this.frame = frame; this.frame = frame;
(frame || externalFrame).appendChild(sidebarFrame); (frame || externalFrame).appendChild(sidebarFrame);
this.on('panelReady', () => { this.subscribe('panelReady', () => {
// Show the UI // Show the UI
if (this.frame) { if (this.frame) {
this.frame.style.display = ''; this.frame.style.display = '';
} }
}); });
this.on('beforeAnnotationCreated', annotation => { this.subscribe('beforeAnnotationCreated', annotation => {
// When a new non-highlight annotation is created, focus // When a new non-highlight annotation is created, focus
// the sidebar so that the text editor can be focused as // the sidebar so that the text editor can be focused as
// soon as the annotation card appears // soon as the annotation card appears
......
...@@ -25,16 +25,6 @@ module.exports = class BucketBar extends Plugin ...@@ -25,16 +25,6 @@ module.exports = class BucketBar extends Plugin
</div> </div>
""" """
# Plugin options
options:
# gapSize parameter is used by the clustering algorithm
# If an annotation is farther then this gapSize from the next bucket
# then that annotation will not be merged into the bucket
gapSize: 60
# Selectors for the scrollable elements on the page
scrollables: ['body']
# buckets of annotations that overlap # buckets of annotations that overlap
buckets: [] buckets: []
...@@ -45,7 +35,16 @@ module.exports = class BucketBar extends Plugin ...@@ -45,7 +35,16 @@ module.exports = class BucketBar extends Plugin
tabs: null tabs: null
constructor: (element, options) -> constructor: (element, options) ->
super $(@html), options defaultOptions = {
# gapSize parameter is used by the clustering algorithm
# If an annotation is farther then this gapSize from the next bucket
# then that annotation will not be merged into the bucket
gapSize: 60
# Selectors for the scrollable elements on the page
scrollables: ['body']
}
super $(@html), Object.assign(defaultOptions, options)
if @options.container? if @options.container?
$(@options.container).append @element $(@options.container).append @element
......
...@@ -38,7 +38,7 @@ export default class Sidebar extends Host { ...@@ -38,7 +38,7 @@ export default class Sidebar extends Host {
config.query || config.query ||
config.group config.group
) { ) {
this.on('panelReady', () => this.show()); this.subscribe('panelReady', () => this.show());
} }
if (this.plugins.BucketBar) { if (this.plugins.BucketBar) {
......
import Delegator from '../delegator';
describe('Delegator', () => {
it('constructor sets `element` and `options` properties', () => {
const el = document.createElement('div');
const config = { foo: 'bar' };
const delegator = new Delegator(el, config);
assert.equal(delegator.element, el);
assert.deepEqual(delegator.options, config);
});
it('supports publishing and subscribing to events', () => {
const element = document.createElement('div');
const delegatorA = new Delegator(element);
const delegatorB = new Delegator(element);
const callback = sinon.stub();
delegatorB.subscribe('someEvent', callback);
delegatorA.publish('someEvent', ['foo', 'bar']);
assert.calledOnce(callback);
assert.calledWith(callback, 'foo', 'bar');
delegatorB.unsubscribe('someEvent', callback);
delegatorA.publish('someEvent', ['foo', 'bar']);
assert.calledOnce(callback);
});
describe('#destroy', () => {
it('removes all event subscriptions created by current instance', () => {
const element = document.createElement('div');
const delegator = new Delegator(element);
const callback = sinon.stub();
delegator.subscribe('someEvent', callback);
delegator.publish('someEvent');
assert.calledOnce(callback);
delegator.destroy();
delegator.publish('someEvent');
assert.calledOnce(callback);
});
});
});
...@@ -22,8 +22,8 @@ class FakeAdder { ...@@ -22,8 +22,8 @@ class FakeAdder {
FakeAdder.instance = null; FakeAdder.instance = null;
class FakePlugin extends Plugin { class FakePlugin extends Plugin {
constructor() { constructor(element, config) {
super(); super(element, config);
FakePlugin.instance = this; FakePlugin.instance = this;
this.pluginInit = sinon.stub(); this.pluginInit = sinon.stub();
...@@ -291,7 +291,7 @@ describe('Guest', () => { ...@@ -291,7 +291,7 @@ describe('Guest', () => {
]; ];
return new Promise(resolve => { return new Promise(resolve => {
guest.element.on('scrolltorange', event => { guest.element.addEventListener('scrolltorange', event => {
assert.equal(event.detail, fakeRange); assert.equal(event.detail, fakeRange);
resolve(); resolve();
}); });
...@@ -312,7 +312,9 @@ describe('Guest', () => { ...@@ -312,7 +312,9 @@ describe('Guest', () => {
}, },
]; ];
guest.element.on('scrolltorange', event => event.preventDefault()); guest.element.addEventListener('scrolltorange', event =>
event.preventDefault()
);
emitGuestEvent('scrollToAnnotation', 'tag1'); emitGuestEvent('scrollToAnnotation', 'tag1');
assert.notCalled(scrollIntoView); assert.notCalled(scrollIntoView);
}); });
...@@ -382,14 +384,14 @@ describe('Guest', () => { ...@@ -382,14 +384,14 @@ describe('Guest', () => {
emitGuestEvent('setVisibleHighlights', true); emitGuestEvent('setVisibleHighlights', true);
assert.calledWith( assert.calledWith(
highlighter.setHighlightsVisible, highlighter.setHighlightsVisible,
guest.element[0], guest.element,
true true
); );
emitGuestEvent('setVisibleHighlights', false); emitGuestEvent('setVisibleHighlights', false);
assert.calledWith( assert.calledWith(
highlighter.setHighlightsVisible, highlighter.setHighlightsVisible,
guest.element[0], guest.element,
false false
); );
}); });
...@@ -404,7 +406,7 @@ describe('Guest', () => { ...@@ -404,7 +406,7 @@ describe('Guest', () => {
beforeEach(() => { beforeEach(() => {
fakeSidebarFrame = null; fakeSidebarFrame = null;
guest = createGuest(); guest = createGuest();
rootElement = guest.element[0]; rootElement = guest.element;
}); });
afterEach(() => { afterEach(() => {
......
...@@ -90,7 +90,7 @@ describe('Host', () => { ...@@ -90,7 +90,7 @@ describe('Host', () => {
describe('config', () => { describe('config', () => {
it('disables highlighting if showHighlights: false is given', done => { it('disables highlighting if showHighlights: false is given', done => {
const host = createHost({ showHighlights: false }); const host = createHost({ showHighlights: false });
host.on('panelReady', () => { host.subscribe('panelReady', () => {
assert.isFalse(host.visibleHighlights); assert.isFalse(host.visibleHighlights);
done(); done();
}); });
......
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