Skip to content

Commit

Permalink
Strengthen the types for Bridge events
Browse files Browse the repository at this point in the history
I have created type definitions for all the event names that are sent
across the different frames using various `Bridge`s. It is based on the
previous `bridge-events.js`. I broke down the events in four sections
based on the direction of the messages:

* guest -> sidebar events
* host -> sidebar events
* sidebar -> guest/s events
* sidebar -> host events

For those events that didn't have a description I added one.

This is more stringent and less verbose than the previous lookup system.
  • Loading branch information
esanzgar committed Oct 16, 2021
1 parent 910b3da commit 1748ca4
Show file tree
Hide file tree
Showing 22 changed files with 276 additions and 166 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@
"scripts": {
"build": "cross-env NODE_ENV=production gulp build",
"lint": "eslint .",
"checkformatting": "prettier --check '**/*.{js,scss}'",
"format": "prettier --list-different --write '**/*.{js,scss}'",
"checkformatting": "prettier --check '**/*.{js,scss,d.ts}'",
"format": "prettier --list-different --write '**/*.{js,scss,d.ts}'",
"test": "gulp test",
"typecheck": "tsc --build tsconfig.json",
"report-coverage": "codecov -f coverage/coverage-final.json",
Expand Down
5 changes: 2 additions & 3 deletions src/annotator/annotation-counts.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import events from '../shared/bridge-events';

const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count';

/**
Expand All @@ -11,8 +9,9 @@ const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count';
* display annotation count.
* @param {import('../shared/bridge').Bridge} bridge - Channel for host-sidebar communication
*/

export function annotationCounts(rootEl, bridge) {
bridge.on(events.PUBLIC_ANNOTATION_COUNT_CHANGED, updateAnnotationCountElems);
bridge.on('publicAnnotationCountChanged', updateAnnotationCountElems);

function updateAnnotationCountElems(newCount) {
const elems = rootEl.querySelectorAll('[' + ANNOTATION_COUNT_ATTR + ']');
Expand Down
21 changes: 14 additions & 7 deletions src/annotator/annotation-sync.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
/**
* @typedef {import('../shared/bridge').Bridge} Bridge
* @typedef {import('../types/annotator').AnnotationData} AnnotationData
* @typedef {import('../types/annotator').Destroyable} Destroyable
* @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
* @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent
* @typedef {import('./util/emitter').EventBus} EventBus
*/

/**
* @template {GuestToSidebarEvent} T
* @template {SidebarToGuestEvent} U
* @typedef {import('../shared/bridge').Bridge<T,U>} Bridge
*/

/**
* Message sent between the sidebar and annotator about an annotation that has
* changed.
Expand All @@ -26,11 +33,11 @@
export class AnnotationSync {
/**
* @param {EventBus} eventBus - Event bus for communicating with the annotator code (eg. the Guest)
* @param {Bridge} bridge - Channel for communicating with the sidebar
* @param {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>} bridge - Channel for communicating with the sidebar
*/
constructor(eventBus, bridge) {
this._emitter = eventBus.createEmitter();
this.bridge = bridge;
this._sidebar = bridge;

/**
* Mapping from annotation tags to annotation objects for annotations which
Expand All @@ -43,7 +50,7 @@ export class AnnotationSync {
this.destroyed = false;

// Relay events from the sidebar to the rest of the annotator.
this.bridge.on('deleteAnnotation', (body, callback) => {
this._sidebar.on('deleteAnnotation', (body, callback) => {
if (this.destroyed) {
callback(null);
return;
Expand All @@ -55,7 +62,7 @@ export class AnnotationSync {
callback(null);
});

this.bridge.on('loadAnnotations', (bodies, callback) => {
this._sidebar.on('loadAnnotations', (bodies, callback) => {
if (this.destroyed) {
callback(null);
return;
Expand All @@ -70,7 +77,7 @@ export class AnnotationSync {
if (annotation.$tag) {
return;
}
this.bridge.call('beforeCreateAnnotation', this._format(annotation));
this._sidebar.call('beforeCreateAnnotation', this._format(annotation));
});
}

Expand All @@ -87,7 +94,7 @@ export class AnnotationSync {
return;
}

this.bridge.call(
this._sidebar.call(
'sync',
annotations.map(ann => this._format(ann))
);
Expand Down
3 changes: 1 addition & 2 deletions src/annotator/features.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import events from '../shared/bridge-events';
import warnOnce from '../shared/warn-once';

let _features = {};
Expand All @@ -12,7 +11,7 @@ export const features = {
* @param {import('../shared/bridge').Bridge} bridge - Channel for host-sidebar communication
*/
init: function (bridge) {
bridge.on(events.FEATURE_FLAGS_UPDATED, _set);
bridge.on('featureFlagsUpdated', _set);
},

reset: function () {
Expand Down
8 changes: 7 additions & 1 deletion src/annotator/guest.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import { normalizeURI } from './util/url';
* @typedef {import('../types/annotator').Destroyable} Destroyable
* @typedef {import('../types/annotator').SidebarLayout} SidebarLayout
* @typedef {import('../types/api').Target} Target
* @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
* @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent
* @typedef {import('./util/emitter').EventBus} EventBus
*/

Expand Down Expand Up @@ -163,7 +165,11 @@ export default class Guest {
// The "top" guest instance will have this as null since it's in a top frame not a sub frame
this._frameIdentifier = config.subFrameIdentifier || null;

// Set up listeners for messages coming from the sidebar to this guest.
/**
* Channel for sidebar-guest communication.
*
* @type {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>}
*/
this._bridge = new Bridge();
this._bridge.onConnect(() => {
this._emitter.publish('panelReady');
Expand Down
19 changes: 13 additions & 6 deletions src/annotator/sidebar.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Hammer from 'hammerjs';

import { Bridge } from '../shared/bridge';
import events from '../shared/bridge-events';
import { ListenerCollection } from '../shared/listener-collection';

import { annotationCounts } from './annotation-counts';
Expand All @@ -14,6 +13,8 @@ import { createShadowRoot } from './util/shadow-root';

/**
* @typedef {import('./guest').default} Guest
* @typedef {import('../types/bridge-events').HostToSidebarEvent} HostToSidebarEvent
* @typedef {import('../types/bridge-events').SidebarToHostEvent} SidebarToHostEvent
* @typedef {import('../types/annotator').SidebarLayout} SidebarLayout
* @typedef {import('../types/annotator').Destroyable} Destroyable
*/
Expand Down Expand Up @@ -64,6 +65,11 @@ export default class Sidebar {
constructor(element, eventBus, guest, config = {}) {
this._emitter = eventBus.createEmitter();

/**
* Channel for sidebar-host communication.
*
* @type {Bridge<HostToSidebarEvent,SidebarToHostEvent>}
*/
this._sidebarRPC = new Bridge();

/**
Expand Down Expand Up @@ -252,12 +258,13 @@ export default class Sidebar {
this.show();
});

/** @type {Array<[SidebarToHostEvent, function]>} */
const eventHandlers = [
[events.LOGIN_REQUESTED, this.onLoginRequest],
[events.LOGOUT_REQUESTED, this.onLogoutRequest],
[events.SIGNUP_REQUESTED, this.onSignupRequest],
[events.PROFILE_REQUESTED, this.onProfileRequest],
[events.HELP_REQUESTED, this.onHelpRequest],
['loginRequested', this.onLoginRequest],
['logoutRequested', this.onLogoutRequest],
['signupRequested', this.onSignupRequest],
['profileRequested', this.onProfileRequest],
['helpRequested', this.onHelpRequest],
];
eventHandlers.forEach(([event, handler]) => {
if (handler) {
Expand Down
5 changes: 2 additions & 3 deletions src/annotator/test/annotation-sync-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { EventBus } from '../util/emitter';

import { AnnotationSync } from '../annotation-sync';
import { EventBus } from '../util/emitter';

describe('AnnotationSync', () => {
let createAnnotationSync;
Expand Down Expand Up @@ -49,7 +48,7 @@ describe('AnnotationSync', () => {
assert.calledWith(eventStub, ann);
});

it("calls the 'deleteAnnotation' event's callback function", done => {
it('calls the "deleteAnnotation" event\'s callback function', done => {
const ann = { id: 1, $tag: 'tag1' };
const callback = function (err, result) {
assert.isNull(err);
Expand Down
3 changes: 1 addition & 2 deletions src/annotator/test/features-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import events from '../../shared/bridge-events';
import { features, $imports } from '../features';

describe('features - annotation layer', () => {
Expand All @@ -22,7 +21,7 @@ describe('features - annotation layer', () => {

features.init({
on: function (topic, handler) {
if (topic === events.FEATURE_FLAGS_UPDATED) {
if (topic === 'featureFlagsUpdated') {
featureFlagsUpdateHandler = handler;
}
},
Expand Down
29 changes: 14 additions & 15 deletions src/annotator/test/guest-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import Guest from '../guest';
import Guest, { $imports } from '../guest';
import { EventBus } from '../util/emitter';
import { $imports } from '../guest';

class FakeAdder {
constructor(container, options) {
Expand Down Expand Up @@ -234,7 +233,7 @@ describe('Guest', () => {
});

describe('events from sidebar', () => {
const emitGuestEvent = (event, ...args) => {
const emitSidebarEvent = (event, ...args) => {
for (let [evt, fn] of fakeBridge.on.args) {
if (event === evt) {
fn(...args);
Expand All @@ -252,7 +251,7 @@ describe('Guest', () => {
{ annotation: { $tag: 'tag2' }, highlights: [highlight1] },
];

emitGuestEvent('focusAnnotations', ['tag1']);
emitSidebarEvent('focusAnnotations', ['tag1']);

assert.calledWith(
highlighter.setHighlightsFocused,
Expand All @@ -270,7 +269,7 @@ describe('Guest', () => {
{ annotation: { $tag: 'tag2' }, highlights: [highlight1] },
];

emitGuestEvent('focusAnnotations', ['tag1']);
emitSidebarEvent('focusAnnotations', ['tag1']);

assert.calledWith(
highlighter.setHighlightsFocused,
Expand All @@ -282,8 +281,8 @@ describe('Guest', () => {
it('updates focused tag set', () => {
const guest = createGuest();

emitGuestEvent('focusAnnotations', ['tag1']);
emitGuestEvent('focusAnnotations', ['tag2', 'tag3']);
emitSidebarEvent('focusAnnotations', ['tag1']);
emitSidebarEvent('focusAnnotations', ['tag2', 'tag3']);

assert.deepEqual([...guest.focusedAnnotationTags], ['tag2', 'tag3']);
});
Expand All @@ -302,7 +301,7 @@ describe('Guest', () => {
},
];

emitGuestEvent('scrollToAnnotation', 'tag1');
emitSidebarEvent('scrollToAnnotation', 'tag1');

assert.called(fakeIntegration.scrollToAnchor);
assert.calledWith(fakeIntegration.scrollToAnchor, guest.anchors[0]);
Expand All @@ -326,7 +325,7 @@ describe('Guest', () => {
resolve();
});

emitGuestEvent('scrollToAnnotation', 'tag1');
emitSidebarEvent('scrollToAnnotation', 'tag1');
});
});

Expand All @@ -345,7 +344,7 @@ describe('Guest', () => {
event.preventDefault()
);

emitGuestEvent('scrollToAnnotation', 'tag1');
emitSidebarEvent('scrollToAnnotation', 'tag1');

assert.notCalled(fakeIntegration.scrollToAnchor);
});
Expand All @@ -354,7 +353,7 @@ describe('Guest', () => {
const guest = createGuest();

guest.anchors = [{ annotation: { $tag: 'tag1' } }];
emitGuestEvent('scrollToAnnotation', 'tag1');
emitSidebarEvent('scrollToAnnotation', 'tag1');

assert.notCalled(fakeIntegration.scrollToAnchor);
});
Expand All @@ -374,7 +373,7 @@ describe('Guest', () => {
const eventEmitted = sandbox.stub();
guest.element.addEventListener('scrolltorange', eventEmitted);

emitGuestEvent('scrollToAnnotation', 'tag1');
emitSidebarEvent('scrollToAnnotation', 'tag1');

assert.notCalled(eventEmitted);
assert.notCalled(fakeIntegration.scrollToAnchor);
Expand Down Expand Up @@ -408,7 +407,7 @@ describe('Guest', () => {

fakeIntegration.getMetadata.resolves(metadata);

emitGuestEvent(
emitSidebarEvent(
'getDocumentInfo',
createCallback('https://example.com/test.pdf', metadata, done)
);
Expand All @@ -419,14 +418,14 @@ describe('Guest', () => {
it('sets visibility of highlights in document', () => {
const guest = createGuest();

emitGuestEvent('setVisibleHighlights', true);
emitSidebarEvent('setVisibleHighlights', true);
assert.calledWith(
highlighter.setHighlightsVisible,
guest.element,
true
);

emitGuestEvent('setVisibleHighlights', false);
emitSidebarEvent('setVisibleHighlights', false);
assert.calledWith(
highlighter.setHighlightsVisible,
guest.element,
Expand Down
Loading

0 comments on commit 1748ca4

Please sign in to comment.