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 15, 2021
1 parent 910b3da commit 545ce16
Show file tree
Hide file tree
Showing 21 changed files with 258 additions and 158 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
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 @@ -19,6 +19,8 @@ import { SelectionObserver } from './selection-observer';
import { normalizeURI } from './util/url';

/**
* @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
* @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent
* @typedef {import('../types/annotator').AnnotationData} AnnotationData
* @typedef {import('../types/annotator').Annotator} Annotator
* @typedef {import('../types/annotator').Anchor} Anchor
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<[import('../types/bridge-events').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
33 changes: 15 additions & 18 deletions src/annotator/test/sidebar-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import events from '../../shared/bridge-events';

import Sidebar, { MIN_RESIZE } from '../sidebar';
import { $imports } from '../sidebar';
import Sidebar, { MIN_RESIZE, $imports } from '../sidebar';
import { EventBus } from '../util/emitter';

const DEFAULT_WIDTH = 350;
Expand Down Expand Up @@ -361,12 +358,12 @@ describe('Sidebar', () => {
});
});

describe('on LOGIN_REQUESTED event', () => {
describe('on "loginRequest" event', () => {
it('calls the onLoginRequest callback function if one was provided', () => {
const onLoginRequest = sandbox.stub();
createSidebar({ services: [{ onLoginRequest }] });

emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');

assert.called(onLoginRequest);
});
Expand All @@ -386,7 +383,7 @@ describe('Sidebar', () => {
],
});

emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');

assert.called(firstOnLogin);
assert.notCalled(secondOnLogin);
Expand All @@ -406,25 +403,25 @@ describe('Sidebar', () => {
],
});

emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');

assert.notCalled(secondOnLogin);
assert.notCalled(thirdOnLogin);
});

it('does not crash if there is no services', () => {
createSidebar(); // No config.services
emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');
});

it('does not crash if services is an empty array', () => {
createSidebar({ services: [] });
emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');
});

it('does not crash if the first service has no onLoginRequest', () => {
createSidebar({ services: [{}] });
emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');
});
});

Expand All @@ -433,37 +430,37 @@ describe('Sidebar', () => {
const onLogoutRequest = sandbox.stub();
createSidebar({ services: [{ onLogoutRequest }] });

emitEvent(events.LOGOUT_REQUESTED);
emitEvent('logoutRequested');

assert.called(onLogoutRequest);
}));

describe('on SIGNUP_REQUESTED event', () =>
describe('on "signupRequest" event', () =>
it('calls the onSignupRequest callback function', () => {
const onSignupRequest = sandbox.stub();
createSidebar({ services: [{ onSignupRequest }] });

emitEvent(events.SIGNUP_REQUESTED);
emitEvent('signupRequested');

assert.called(onSignupRequest);
}));

describe('on PROFILE_REQUESTED event', () =>
describe('on "profileRequest" event', () =>
it('calls the onProfileRequest callback function', () => {
const onProfileRequest = sandbox.stub();
createSidebar({ services: [{ onProfileRequest }] });

emitEvent(events.PROFILE_REQUESTED);
emitEvent('profileRequested');

assert.called(onProfileRequest);
}));

describe('on HELP_REQUESTED event', () =>
describe('on "helpRequested" event', () =>
it('calls the onHelpRequest callback function', () => {
const onHelpRequest = sandbox.stub();
createSidebar({ services: [{ onHelpRequest }] });

emitEvent(events.HELP_REQUESTED);
emitEvent('helpRequested');

assert.called(onHelpRequest);
}));
Expand Down
Loading

0 comments on commit 545ce16

Please sign in to comment.