Commit 561d37f8 authored by Eduardo Sanz García's avatar Eduardo Sanz García Committed by Eduardo

Remove event emitter from `Guest`

The main `Guest` was assumed to be always in the same frame as the
`host` frame. In that scenario (which is currently the most common),
`Guest` was able to communicate with the `host` via `TinyEmitter`.

This PR removes that assumption an enforce all communication between the
`Guest` and the `host` to occur via an inter-frame communication
channel. This paves the work for the main `Guest` to be in a different
frame than the `host`, which is the case in the Ebook readers and
VitalSource.
parent 71a3b0cc
...@@ -30,7 +30,6 @@ import { normalizeURI } from './util/url'; ...@@ -30,7 +30,6 @@ import { normalizeURI } from './util/url';
* @typedef {import('../types/bridge-events').GuestToHostEvent} GuestToHostEvent * @typedef {import('../types/bridge-events').GuestToHostEvent} GuestToHostEvent
* @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent * @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
* @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent * @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent
* @typedef {import('./util/emitter').EventBus} EventBus
*/ */
/** /**
...@@ -122,16 +121,13 @@ export default class Guest { ...@@ -122,16 +121,13 @@ export default class Guest {
* @param {HTMLElement} element - * @param {HTMLElement} element -
* The root element in which the `Guest` instance should be able to anchor * The root element in which the `Guest` instance should be able to anchor
* or create annotations. In an ordinary web page this typically `document.body`. * or create annotations. In an ordinary web page this typically `document.body`.
* @param {EventBus} eventBus -
* Enables communication between components sharing the same eventBus
* @param {Record<string, any>} [config] * @param {Record<string, any>} [config]
* @param {Window} [hostFrame] - * @param {Window} [hostFrame] -
* Host frame which this guest is associated with. This is expected to be * Host frame which this guest is associated with. This is expected to be
* an ancestor of the guest frame. It may be same or cross origin. * an ancestor of the guest frame. It may be same or cross origin.
*/ */
constructor(element, eventBus, config = {}, hostFrame = window) { constructor(element, config = {}, hostFrame = window) {
this.element = element; this.element = element;
this._emitter = eventBus.createEmitter();
this._hostFrame = hostFrame; this._hostFrame = hostFrame;
this._highlightsVisible = false; this._highlightsVisible = false;
this._isAdderVisible = false; this._isAdderVisible = false;
...@@ -440,7 +436,6 @@ export default class Guest { ...@@ -440,7 +436,6 @@ export default class Guest {
removeAllHighlights(this.element); removeAllHighlights(this.element);
this._integration.destroy(); this._integration.destroy();
this._emitter.destroy();
this._sidebarRPC.destroy(); this._sidebarRPC.destroy();
} }
...@@ -593,8 +588,8 @@ export default class Guest { ...@@ -593,8 +588,8 @@ export default class Guest {
*/ */
_updateAnchors(anchors, notify) { _updateAnchors(anchors, notify) {
this.anchors = anchors; this.anchors = anchors;
if (notify) { if (notify && this._frameIdentifier === null) {
this._emitter.publish('anchorsChanged', this.anchors); this._hostRPC.call('anchorsChanged');
} }
} }
......
...@@ -42,8 +42,7 @@ function init() { ...@@ -42,8 +42,7 @@ function init() {
const hostFrame = annotatorConfig.subFrameIdentifier ? window.parent : window; const hostFrame = annotatorConfig.subFrameIdentifier ? window.parent : window;
// Create the guest that handles creating annotations and displaying highlights. // Create the guest that handles creating annotations and displaying highlights.
const eventBus = new EventBus(); const guest = new Guest(document.body, annotatorConfig, hostFrame);
const guest = new Guest(document.body, eventBus, annotatorConfig, hostFrame);
let sidebar; let sidebar;
let notebook; let notebook;
...@@ -55,6 +54,7 @@ function init() { ...@@ -55,6 +54,7 @@ function init() {
portProvider = new PortProvider(hypothesisAppsOrigin); portProvider = new PortProvider(hypothesisAppsOrigin);
portProvider.listen(); portProvider.listen();
const eventBus = new EventBus();
sidebar = new Sidebar(document.body, eventBus, guest, sidebarConfig); sidebar = new Sidebar(document.body, eventBus, guest, sidebarConfig);
notebook = new Notebook(document.body, eventBus, getConfig('notebook')); notebook = new Notebook(document.body, eventBus, getConfig('notebook'));
......
...@@ -117,7 +117,7 @@ export default class Sidebar { ...@@ -117,7 +117,7 @@ export default class Sidebar {
const bucketBar = new BucketBar(this.iframeContainer, guest, { const bucketBar = new BucketBar(this.iframeContainer, guest, {
contentContainer: guest.contentContainer(), contentContainer: guest.contentContainer(),
}); });
this._emitter.subscribe('anchorsChanged', () => bucketBar.update()); this._guestRPC.on('anchorsChanged', () => bucketBar.update());
this.bucketBar = bucketBar; this.bucketBar = bucketBar;
} }
......
import { delay } from '../../test-util/wait'; import { delay } from '../../test-util/wait';
import Guest, { $imports } from '../guest'; import Guest, { $imports } from '../guest';
import { EventBus } from '../util/emitter';
class FakeAdder { class FakeAdder {
constructor(container, options) { constructor(container, options) {
...@@ -30,7 +29,6 @@ class FakeTextRange { ...@@ -30,7 +29,6 @@ class FakeTextRange {
describe('Guest', () => { describe('Guest', () => {
const sandbox = sinon.createSandbox(); const sandbox = sinon.createSandbox();
let eventBus;
let guests; let guests;
let highlighter; let highlighter;
let hostFrame; let hostFrame;
...@@ -45,8 +43,7 @@ describe('Guest', () => { ...@@ -45,8 +43,7 @@ describe('Guest', () => {
const createGuest = (config = {}) => { const createGuest = (config = {}) => {
const element = document.createElement('div'); const element = document.createElement('div');
eventBus = new EventBus(); const guest = new Guest(element, config, hostFrame);
const guest = new Guest(element, eventBus, config, hostFrame);
guests.push(guest); guests.push(guest);
return guest; return guest;
}; };
...@@ -148,60 +145,6 @@ describe('Guest', () => { ...@@ -148,60 +145,6 @@ describe('Guest', () => {
$imports.$restore(); $imports.$restore();
}); });
describe('communication with sidebar component', () => {
describe('event subscription', () => {
let emitter;
let guest;
beforeEach(() => {
guest = createGuest();
emitter = eventBus.createEmitter();
});
afterEach(() => {
emitter.destroy();
});
it('proxies the event into the annotator event system', () => {
const fooHandler = sandbox.stub();
const barHandler = sandbox.stub();
emitter.subscribe('foo', fooHandler);
emitter.subscribe('bar', barHandler);
guest._emitter.publish('foo', '1', '2');
guest._emitter.publish('bar', '1', '2');
assert.calledWith(fooHandler, '1', '2');
assert.calledWith(barHandler, '1', '2');
});
});
describe('event publication', () => {
let emitter;
let guest;
beforeEach(() => {
guest = createGuest();
emitter = eventBus.createEmitter();
});
it('proxies all other events into the annotator event system', () => {
const fooHandler = sandbox.stub();
const barHandler = sandbox.stub();
guest._emitter.subscribe('foo', fooHandler);
guest._emitter.subscribe('bar', barHandler);
emitter.publish('foo', '1', '2');
emitter.publish('bar', '1', '2');
assert.calledWith(fooHandler, '1', '2');
assert.calledWith(barHandler, '1', '2');
});
});
});
describe('events from sidebar frame', () => { describe('events from sidebar frame', () => {
describe('on "focusAnnotations" event', () => { describe('on "focusAnnotations" event', () => {
it('focuses any annotations with a matching tag', () => { it('focuses any annotations with a matching tag', () => {
...@@ -405,7 +348,6 @@ describe('Guest', () => { ...@@ -405,7 +348,6 @@ describe('Guest', () => {
emitSidebarEvent('loadAnnotations', [ann1, ann2]); emitSidebarEvent('loadAnnotations', [ann1, ann2]);
await delay(0); await delay(0);
assert.calledTwice(fakeBridge.call);
assert.calledWith( assert.calledWith(
fakeBridge.call, fakeBridge.call,
'syncAnchoringStatus', 'syncAnchoringStatus',
...@@ -421,14 +363,11 @@ describe('Guest', () => { ...@@ -421,14 +363,11 @@ describe('Guest', () => {
describe('on "deleteAnnotation" event', () => { describe('on "deleteAnnotation" event', () => {
it('detaches annotation', () => { it('detaches annotation', () => {
const guest = createGuest(); createGuest();
const callback = sinon.stub();
guest._emitter.subscribe('anchorsChanged', callback);
emitSidebarEvent('deleteAnnotation', 'tag1'); emitSidebarEvent('deleteAnnotation', 'tag1');
assert.calledOnce(callback); assert.deepEqual(fakeBridge.call.lastCall.args, ['anchorsChanged']);
assert.calledWith(callback, []);
}); });
}); });
}); });
...@@ -1046,15 +985,16 @@ describe('Guest', () => { ...@@ -1046,15 +985,16 @@ describe('Guest', () => {
}); });
}); });
it('emits an `anchorsChanged` event', async () => { it('calls "syncAnchoringStatus" RPC method', async () => {
const guest = createGuest(); const guest = createGuest();
const annotation = {}; const annotation = {};
const anchorsChanged = sandbox.stub();
guest._emitter.subscribe('anchorsChanged', anchorsChanged);
await guest.anchor(annotation); await guest.anchor(annotation);
assert.calledWith(anchorsChanged, guest.anchors); assert.match(fakeBridge.call.lastCall.args, [
'syncAnchoringStatus',
annotation,
]);
}); });
it('returns a promise of the anchors for the annotation', () => { it('returns a promise of the anchors for the annotation', () => {
...@@ -1190,15 +1130,13 @@ describe('Guest', () => { ...@@ -1190,15 +1130,13 @@ describe('Guest', () => {
); );
}); });
it('emits an `anchorsChanged` event with updated anchors', () => { it('calls "anchorsChanged" RPC method', () => {
const guest = createGuest(); const guest = createGuest();
const anchor = createAnchor(); const anchor = createAnchor();
const anchorsChanged = sandbox.stub();
guest._emitter.subscribe('anchorsChanged', anchorsChanged);
guest.detach(anchor.annotation.$tag); guest.detach(anchor.annotation.$tag);
assert.calledWith(anchorsChanged, guest.anchors); assert.deepEqual(fakeBridge.call.lastCall.args, ['anchorsChanged']);
}); });
}); });
......
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// with various combinations of selector are anchored. // with various combinations of selector are anchored.
import Guest, { $imports as guestImports } from '../../guest'; import Guest, { $imports as guestImports } from '../../guest';
import { EventBus } from '../../util/emitter';
import testPageHTML from './test-page.html'; import testPageHTML from './test-page.html';
function quoteSelector(quote) { function quoteSelector(quote) {
...@@ -49,7 +48,6 @@ describe('anchoring', () => { ...@@ -49,7 +48,6 @@ describe('anchoring', () => {
container = document.createElement('div'); container = document.createElement('div');
container.innerHTML = testPageHTML; container.innerHTML = testPageHTML;
document.body.appendChild(container); document.body.appendChild(container);
const eventBus = new EventBus();
const fakePortFinder = { const fakePortFinder = {
discover: sinon.stub().resolves(new MessageChannel().port1), discover: sinon.stub().resolves(new MessageChannel().port1),
destroy: sinon.stub(), destroy: sinon.stub(),
...@@ -59,7 +57,7 @@ describe('anchoring', () => { ...@@ -59,7 +57,7 @@ describe('anchoring', () => {
PortFinder: sinon.stub().returns(fakePortFinder), PortFinder: sinon.stub().returns(fakePortFinder),
}, },
}); });
guest = new Guest(container, eventBus); guest = new Guest(container);
}); });
afterEach(() => { afterEach(() => {
......
...@@ -454,6 +454,16 @@ describe('Sidebar', () => { ...@@ -454,6 +454,16 @@ describe('Sidebar', () => {
assert.called(onHelpRequest); assert.called(onHelpRequest);
})); }));
describe('on "anchorsChanged" event', () => {
it('updates the bucket bar', () => {
const sidebar = createSidebar();
emitEvent('anchorsChanged');
assert.calledOnce(sidebar.bucketBar.update);
});
});
}); });
describe('pan gestures', () => { describe('pan gestures', () => {
...@@ -947,11 +957,5 @@ describe('Sidebar', () => { ...@@ -947,11 +957,5 @@ describe('Sidebar', () => {
sinon.match({ contentContainer }) sinon.match({ contentContainer })
); );
}); });
it('updates the bucket bar when an `anchorsChanged` event is received', () => {
const sidebar = createSidebar();
sidebar._emitter.publish('anchorsChanged');
assert.calledOnce(sidebar.bucketBar.update);
});
}); });
}); });
...@@ -15,7 +15,12 @@ export type GuestToHostEvent = ...@@ -15,7 +15,12 @@ export type GuestToHostEvent =
/** /**
* The guest informs the host that text has been selected at a frame with that identifier. * The guest informs the host that text has been selected at a frame with that identifier.
*/ */
| 'textSelectedIn'; | 'textSelectedIn'
/**
* The guest informs the host that the anchors have been changed in the main annotatable frame.
*/
| 'anchorsChanged';
/** /**
* Events that the guest sends to the sidebar * Events that the guest sends to the sidebar
......
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