From e6b39e5eabd8962b00762588cb4229d0ee9a8bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= Date: Fri, 8 Oct 2021 14:06:34 +0200 Subject: [PATCH] Strengthen the types for `Bridge` events 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. --- package.json | 4 +- src/annotator/annotation-counts.js | 5 +- src/annotator/annotation-sync.js | 21 ++- src/annotator/features.js | 3 +- src/annotator/guest.js | 8 +- src/annotator/sidebar.js | 19 ++- src/annotator/test/annotation-sync-test.js | 5 +- src/annotator/test/features-test.js | 3 +- src/annotator/test/guest-test.js | 29 ++-- src/annotator/test/sidebar-test.js | 33 ++-- src/shared/bridge-events.js | 48 ------ src/shared/bridge.js | 11 +- src/sidebar/components/HypothesisApp.js | 9 +- src/sidebar/components/TopBar.js | 7 +- src/sidebar/components/UserMenu.js | 5 +- .../components/test/HypothesisApp-test.js | 16 +- src/sidebar/components/test/TopBar-test.js | 10 +- src/sidebar/components/test/UserMenu-test.js | 10 +- src/sidebar/services/features.js | 6 +- src/sidebar/services/frame-sync.js | 24 ++- src/sidebar/services/test/features-test.js | 5 +- src/types/bridge-events.d.ts | 161 ++++++++++++++++++ 22 files changed, 276 insertions(+), 166 deletions(-) delete mode 100644 src/shared/bridge-events.js create mode 100644 src/types/bridge-events.d.ts diff --git a/package.json b/package.json index de69c6d953f..7397996ad2c 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/annotator/annotation-counts.js b/src/annotator/annotation-counts.js index 0be3ef4749e..519e866fc52 100644 --- a/src/annotator/annotation-counts.js +++ b/src/annotator/annotation-counts.js @@ -1,5 +1,3 @@ -import events from '../shared/bridge-events'; - const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count'; /** @@ -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 + ']'); diff --git a/src/annotator/annotation-sync.js b/src/annotator/annotation-sync.js index f8a517f1f3e..95352188dee 100644 --- a/src/annotator/annotation-sync.js +++ b/src/annotator/annotation-sync.js @@ -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} Bridge + */ + /** * Message sent between the sidebar and annotator about an annotation that has * changed. @@ -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} 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 @@ -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; @@ -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; @@ -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)); }); } @@ -87,7 +94,7 @@ export class AnnotationSync { return; } - this.bridge.call( + this._sidebar.call( 'sync', annotations.map(ann => this._format(ann)) ); diff --git a/src/annotator/features.js b/src/annotator/features.js index ba0b3554418..e2ae1c47994 100644 --- a/src/annotator/features.js +++ b/src/annotator/features.js @@ -1,4 +1,3 @@ -import events from '../shared/bridge-events'; import warnOnce from '../shared/warn-once'; let _features = {}; @@ -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 () { diff --git a/src/annotator/guest.js b/src/annotator/guest.js index 3fd6d35c5fb..148c5960bc6 100644 --- a/src/annotator/guest.js +++ b/src/annotator/guest.js @@ -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 */ @@ -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} + */ this._bridge = new Bridge(); this._bridge.onConnect(() => { this._emitter.publish('panelReady'); diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js index fa77745b9b8..bde6af62f4f 100644 --- a/src/annotator/sidebar.js +++ b/src/annotator/sidebar.js @@ -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'; @@ -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 */ @@ -64,6 +65,11 @@ export default class Sidebar { constructor(element, eventBus, guest, config = {}) { this._emitter = eventBus.createEmitter(); + /** + * Channel for sidebar-host communication. + * + * @type {Bridge} + */ this._sidebarRPC = new Bridge(); /** @@ -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) { diff --git a/src/annotator/test/annotation-sync-test.js b/src/annotator/test/annotation-sync-test.js index cec1a63af71..6f56de6d457 100644 --- a/src/annotator/test/annotation-sync-test.js +++ b/src/annotator/test/annotation-sync-test.js @@ -1,6 +1,5 @@ -import { EventBus } from '../util/emitter'; - import { AnnotationSync } from '../annotation-sync'; +import { EventBus } from '../util/emitter'; describe('AnnotationSync', () => { let createAnnotationSync; @@ -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); diff --git a/src/annotator/test/features-test.js b/src/annotator/test/features-test.js index 881cfef4af2..ff392635499 100644 --- a/src/annotator/test/features-test.js +++ b/src/annotator/test/features-test.js @@ -1,4 +1,3 @@ -import events from '../../shared/bridge-events'; import { features, $imports } from '../features'; describe('features - annotation layer', () => { @@ -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; } }, diff --git a/src/annotator/test/guest-test.js b/src/annotator/test/guest-test.js index fdfc336a302..dcba92677ca 100644 --- a/src/annotator/test/guest-test.js +++ b/src/annotator/test/guest-test.js @@ -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) { @@ -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); @@ -252,7 +251,7 @@ describe('Guest', () => { { annotation: { $tag: 'tag2' }, highlights: [highlight1] }, ]; - emitGuestEvent('focusAnnotations', ['tag1']); + emitSidebarEvent('focusAnnotations', ['tag1']); assert.calledWith( highlighter.setHighlightsFocused, @@ -270,7 +269,7 @@ describe('Guest', () => { { annotation: { $tag: 'tag2' }, highlights: [highlight1] }, ]; - emitGuestEvent('focusAnnotations', ['tag1']); + emitSidebarEvent('focusAnnotations', ['tag1']); assert.calledWith( highlighter.setHighlightsFocused, @@ -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']); }); @@ -302,7 +301,7 @@ describe('Guest', () => { }, ]; - emitGuestEvent('scrollToAnnotation', 'tag1'); + emitSidebarEvent('scrollToAnnotation', 'tag1'); assert.called(fakeIntegration.scrollToAnchor); assert.calledWith(fakeIntegration.scrollToAnchor, guest.anchors[0]); @@ -326,7 +325,7 @@ describe('Guest', () => { resolve(); }); - emitGuestEvent('scrollToAnnotation', 'tag1'); + emitSidebarEvent('scrollToAnnotation', 'tag1'); }); }); @@ -345,7 +344,7 @@ describe('Guest', () => { event.preventDefault() ); - emitGuestEvent('scrollToAnnotation', 'tag1'); + emitSidebarEvent('scrollToAnnotation', 'tag1'); assert.notCalled(fakeIntegration.scrollToAnchor); }); @@ -354,7 +353,7 @@ describe('Guest', () => { const guest = createGuest(); guest.anchors = [{ annotation: { $tag: 'tag1' } }]; - emitGuestEvent('scrollToAnnotation', 'tag1'); + emitSidebarEvent('scrollToAnnotation', 'tag1'); assert.notCalled(fakeIntegration.scrollToAnchor); }); @@ -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); @@ -408,7 +407,7 @@ describe('Guest', () => { fakeIntegration.getMetadata.resolves(metadata); - emitGuestEvent( + emitSidebarEvent( 'getDocumentInfo', createCallback('https://example.com/test.pdf', metadata, done) ); @@ -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, diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js index 47fbec3563b..b9cbbcf7703 100644 --- a/src/annotator/test/sidebar-test.js +++ b/src/annotator/test/sidebar-test.js @@ -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; @@ -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); }); @@ -386,7 +383,7 @@ describe('Sidebar', () => { ], }); - emitEvent(events.LOGIN_REQUESTED); + emitEvent('loginRequested'); assert.called(firstOnLogin); assert.notCalled(secondOnLogin); @@ -406,7 +403,7 @@ describe('Sidebar', () => { ], }); - emitEvent(events.LOGIN_REQUESTED); + emitEvent('loginRequested'); assert.notCalled(secondOnLogin); assert.notCalled(thirdOnLogin); @@ -414,17 +411,17 @@ describe('Sidebar', () => { 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'); }); }); @@ -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); })); diff --git a/src/shared/bridge-events.js b/src/shared/bridge-events.js deleted file mode 100644 index fad0aa4b12e..00000000000 --- a/src/shared/bridge-events.js +++ /dev/null @@ -1,48 +0,0 @@ -/** - * This module defines the set of global events that are dispatched - * across the bridge between the sidebar and annotator - */ -export default { - // Events that the sidebar sends to the annotator - // ---------------------------------------------- - - /** - * The updated feature flags for the user - */ - FEATURE_FLAGS_UPDATED: 'featureFlagsUpdated', - - /** - * The sidebar is asking the annotator to open the partner site help page. - */ - HELP_REQUESTED: 'helpRequested', - - /** The sidebar is asking the annotator to do a partner site log in - * (for example, pop up a log in window). This is used when the client is - * embedded in a partner site and a log in button in the client is clicked. - */ - LOGIN_REQUESTED: 'loginRequested', - - /** The sidebar is asking the annotator to do a partner site log out. - * This is used when the client is embedded in a partner site and a log out - * button in the client is clicked. - */ - LOGOUT_REQUESTED: 'logoutRequested', - - /** - * The sidebar is asking the annotator to open the partner site profile page. - */ - PROFILE_REQUESTED: 'profileRequested', - - /** - * The set of annotations was updated. - */ - PUBLIC_ANNOTATION_COUNT_CHANGED: 'publicAnnotationCountChanged', - - /** - * The sidebar is asking the annotator to do a partner site sign-up. - */ - SIGNUP_REQUESTED: 'signupRequested', - - // Events that the annotator sends to the sidebar - // ---------------------------------------------- -}; diff --git a/src/shared/bridge.js b/src/shared/bridge.js index c777de42836..3d505dc91df 100644 --- a/src/shared/bridge.js +++ b/src/shared/bridge.js @@ -1,11 +1,16 @@ import { PortRPC } from './port-rpc'; -/** @typedef {import('../types/annotator').Destroyable} Destroyable */ +/** + * @typedef {import('../types/annotator').Destroyable} Destroyable + * @typedef {import('../types/bridge-events').BridgeEvent} BridgeEvent + */ /** * The Bridge service sets up a channel between frames and provides an events * API on top of it. * + * @template {BridgeEvent} CallMethods - Names of methods that can be called (via {@link call}) + * @template {BridgeEvent} OnMethods - Names of methods that can be handled (via {@link on}) * @implements Destroyable */ export class Bridge { @@ -66,7 +71,7 @@ export class Bridge { * Make a method call on all channels, collect the results and pass them to a * callback when all results are collected. * - * @param {string} method - Name of remote method to call. + * @param {CallMethods} method - Name of remote method to call. * @param {any[]} args - Arguments to method. Final argument is an optional * callback with this type: `(error: string|Error|null, ...result: any[]) => void`. * This callback, if any, will be triggered once a response (via `postMessage`) @@ -132,7 +137,7 @@ export class Bridge { * Register a listener to be invoked when any connected channel sends a * message to this `Bridge`. * - * @param {string} method + * @param {'connect'|OnMethods} method * @param {(...args: any[]) => void} listener -- Final argument is an optional * callback of the type: `(error: string|Error|null, ...result: any[]) => void`. * This callback must be invoked in order to respond (via `postMessage`) diff --git a/src/sidebar/components/HypothesisApp.js b/src/sidebar/components/HypothesisApp.js index 03e89aea85e..96951090bbe 100644 --- a/src/sidebar/components/HypothesisApp.js +++ b/src/sidebar/components/HypothesisApp.js @@ -1,14 +1,13 @@ import classnames from 'classnames'; import { useEffect, useMemo } from 'preact/hooks'; -import bridgeEvents from '../../shared/bridge-events'; import { confirm } from '../../shared/prompts'; import { serviceConfig } from '../config/service-config'; -import { useStoreProxy } from '../store/use-store'; import { parseAccountID } from '../helpers/account-id'; import { shouldAutoDisplayTutorial } from '../helpers/session'; import { applyTheme } from '../helpers/theme'; import { withServices } from '../service-context'; +import { useStoreProxy } from '../store/use-store'; import AnnotationView from './AnnotationView'; import SidebarView from './SidebarView'; @@ -98,7 +97,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) { const login = async () => { if (serviceConfig(settings)) { // Let the host page handle the login request - frameSync.notifyHost(bridgeEvents.LOGIN_REQUESTED); + frameSync.notifyHost('loginRequested'); return; } @@ -116,7 +115,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) { const signUp = () => { if (serviceConfig(settings)) { // Let the host page handle the signup request - frameSync.notifyHost(bridgeEvents.SIGNUP_REQUESTED); + frameSync.notifyHost('signupRequested'); return; } window.open(store.getLink('signup')); @@ -156,7 +155,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) { store.discardAllDrafts(); if (serviceConfig(settings)) { - frameSync.notifyHost(bridgeEvents.LOGOUT_REQUESTED); + frameSync.notifyHost('logoutRequested'); return; } diff --git a/src/sidebar/components/TopBar.js b/src/sidebar/components/TopBar.js index fb7ad296010..ef02e90f8af 100644 --- a/src/sidebar/components/TopBar.js +++ b/src/sidebar/components/TopBar.js @@ -1,11 +1,10 @@ import { IconButton, LinkButton } from '@hypothesis/frontend-shared'; -import bridgeEvents from '../../shared/bridge-events'; import { serviceConfig } from '../config/service-config'; -import { useStoreProxy } from '../store/use-store'; import { isThirdPartyService } from '../helpers/is-third-party-service'; -import { withServices } from '../service-context'; import { applyTheme } from '../helpers/theme'; +import { withServices } from '../service-context'; +import { useStoreProxy } from '../store/use-store'; import GroupList from './GroupList'; import SearchInput from './SearchInput'; @@ -71,7 +70,7 @@ function TopBar({ const requestHelp = () => { const service = serviceConfig(settings); if (service && service.onHelpRequestProvided) { - frameSync.notifyHost(bridgeEvents.HELP_REQUESTED); + frameSync.notifyHost('helpRequested'); } else { store.toggleSidebarPanel('help'); } diff --git a/src/sidebar/components/UserMenu.js b/src/sidebar/components/UserMenu.js index 77b574f5766..a9c8ab3a382 100644 --- a/src/sidebar/components/UserMenu.js +++ b/src/sidebar/components/UserMenu.js @@ -1,11 +1,10 @@ import { SvgIcon } from '@hypothesis/frontend-shared'; import { useState } from 'preact/hooks'; -import bridgeEvents from '../../shared/bridge-events'; import { serviceConfig } from '../config/service-config'; import { isThirdPartyUser } from '../helpers/account-id'; -import { useStoreProxy } from '../store/use-store'; import { withServices } from '../service-context'; +import { useStoreProxy } from '../store/use-store'; import Menu from './Menu'; import MenuItem from './MenuItem'; @@ -70,7 +69,7 @@ function UserMenu({ auth, frameSync, onLogout, settings }) { }; const onProfileSelected = () => - isThirdParty && frameSync.notifyHost(bridgeEvents.PROFILE_REQUESTED); + isThirdParty && frameSync.notifyHost('profileRequested'); // Generate dynamic props for the profile component const profileItemProps = (() => { diff --git a/src/sidebar/components/test/HypothesisApp-test.js b/src/sidebar/components/test/HypothesisApp-test.js index 617106b2e8d..2ee85d94125 100644 --- a/src/sidebar/components/test/HypothesisApp-test.js +++ b/src/sidebar/components/test/HypothesisApp-test.js @@ -1,8 +1,6 @@ import { mount } from 'enzyme'; -import bridgeEvents from '../../../shared/bridge-events'; import mockImportedComponents from '../../../test-util/mock-imported-components'; - import HypothesisApp, { $imports } from '../HypothesisApp'; describe('HypothesisApp', () => { @@ -225,13 +223,10 @@ describe('HypothesisApp', () => { fakeServiceConfig.returns({}); }); - it('sends SIGNUP_REQUESTED event', () => { + it('sends "signupRequest" event', () => { const wrapper = createComponent(); clickSignUp(wrapper); - assert.calledWith( - fakeFrameSync.notifyHost, - bridgeEvents.SIGNUP_REQUESTED - ); + assert.calledWith(fakeFrameSync.notifyHost, 'signupRequested'); }); it('does not open a URL directly', () => { @@ -304,7 +299,7 @@ describe('HypothesisApp', () => { assert.equal(fakeFrameSync.notifyHost.callCount, 1); assert.isTrue( - fakeFrameSync.notifyHost.calledWithExactly(bridgeEvents.LOGIN_REQUESTED) + fakeFrameSync.notifyHost.calledWithExactly('loginRequested') ); }); }); @@ -412,10 +407,7 @@ describe('HypothesisApp', () => { await clickLogOut(wrapper); assert.calledOnce(fakeFrameSync.notifyHost); - assert.calledWithExactly( - fakeFrameSync.notifyHost, - bridgeEvents.LOGOUT_REQUESTED - ); + assert.calledWithExactly(fakeFrameSync.notifyHost, 'logoutRequested'); }); it('does not send LOGOUT_REQUESTED if the user cancels the prompt', async () => { diff --git a/src/sidebar/components/test/TopBar-test.js b/src/sidebar/components/test/TopBar-test.js index 7074d3afba2..b2fa1898abb 100644 --- a/src/sidebar/components/test/TopBar-test.js +++ b/src/sidebar/components/test/TopBar-test.js @@ -1,11 +1,8 @@ import { mount } from 'enzyme'; -import bridgeEvents from '../../../shared/bridge-events'; -import TopBar from '../TopBar'; -import { $imports } from '../TopBar'; - import { checkAccessibility } from '../../../test-util/accessibility'; import mockImportedComponents from '../../../test-util/mock-imported-components'; +import TopBar, { $imports } from '../TopBar'; describe('TopBar', () => { const fakeSettings = {}; @@ -123,10 +120,7 @@ describe('TopBar', () => { helpButton.props().onClick(); assert.equal(fakeStore.toggleSidebarPanel.callCount, 0); - assert.calledWith( - fakeFrameSync.notifyHost, - bridgeEvents.HELP_REQUESTED - ); + assert.calledWith(fakeFrameSync.notifyHost, 'helpRequested'); }); }); }); diff --git a/src/sidebar/components/test/UserMenu-test.js b/src/sidebar/components/test/UserMenu-test.js index a31ff56e5e5..7e97cbf6e69 100644 --- a/src/sidebar/components/test/UserMenu-test.js +++ b/src/sidebar/components/test/UserMenu-test.js @@ -1,11 +1,8 @@ import { mount } from 'enzyme'; import { act } from 'preact/test-utils'; -import bridgeEvents from '../../../shared/bridge-events'; -import UserMenu from '../UserMenu'; -import { $imports } from '../UserMenu'; - import mockImportedComponents from '../../../test-util/mock-imported-components'; +import UserMenu, { $imports } from '../UserMenu'; describe('UserMenu', () => { let fakeAuth; @@ -148,10 +145,7 @@ describe('UserMenu', () => { onProfileSelected(); assert.equal(fakeFrameSync.notifyHost.callCount, 1); - assert.calledWith( - fakeFrameSync.notifyHost, - bridgeEvents.PROFILE_REQUESTED - ); + assert.calledWith(fakeFrameSync.notifyHost, 'profileRequested'); }); it('should not fire profile event for first-party user', () => { diff --git a/src/sidebar/services/features.js b/src/sidebar/services/features.js index d8856c82868..1fd01a30f52 100644 --- a/src/sidebar/services/features.js +++ b/src/sidebar/services/features.js @@ -1,4 +1,3 @@ -import bridgeEvents from '../../shared/bridge-events'; import { watch } from '../util/watch'; /** @@ -26,10 +25,7 @@ export class FeaturesService { init() { const currentFlags = () => this._store.profile().features; const sendFeatureFlags = () => { - this._frameSync.notifyHost( - bridgeEvents.FEATURE_FLAGS_UPDATED, - currentFlags() || {} - ); + this._frameSync.notifyHost('featureFlagsUpdated', currentFlags() || {}); }; // Re-send feature flags to connected frames when flags change or a new diff --git a/src/sidebar/services/frame-sync.js b/src/sidebar/services/frame-sync.js index a240c3e9abd..1c4dbf8cc84 100644 --- a/src/sidebar/services/frame-sync.js +++ b/src/sidebar/services/frame-sync.js @@ -1,11 +1,14 @@ import debounce from 'lodash.debounce'; -import bridgeEvents from '../../shared/bridge-events'; import { Bridge } from '../../shared/bridge'; import { isReply, isPublic } from '../helpers/annotation-metadata'; import { watch } from '../util/watch'; /** + * @typedef {import('../../types/bridge-events').SidebarToHostEvent} SidebarToHostEvent + * @typedef {import('../../types/bridge-events').HostToSidebarEvent} HostToSidebarEvent + * @typedef {import('../../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent + * @typedef {import('../../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent * @typedef {import('../../shared/port-rpc').PortRPC} PortRPC */ @@ -55,10 +58,18 @@ export class FrameSyncService { * @param {import('../store').SidebarStore} store */ constructor($window, annotationsService, store) { - /** Channel for sidebar <-> host communication. */ + /** + * Channel for sidebar-host communication. + * + * @type {Bridge} + */ this._hostRPC = new Bridge(); - /** Channel for sidebar <-> guest(s) communication. */ + /** + * Channel for sidebar-guest(s) communication. + * + * @type {Bridge} + */ this._guestRPC = new Bridge(); this._store = store; @@ -118,10 +129,7 @@ export class FrameSyncService { if (frames.length > 0) { if (frames.every(frame => frame.isAnnotationFetchComplete)) { if (publicAnns === 0 || publicAnns !== prevPublicAnns) { - this._hostRPC.call( - bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, - publicAnns - ); + this._hostRPC.call('publicAnnotationCountChanged', publicAnns); prevPublicAnns = publicAnns; } } @@ -282,7 +290,7 @@ export class FrameSyncService { /** * Send an RPC message to the host frame. * - * @param {string} method + * @param {SidebarToHostEvent} method * @param {any[]} args */ notifyHost(method, ...args) { diff --git a/src/sidebar/services/test/features-test.js b/src/sidebar/services/test/features-test.js index 55be3a7f0b6..0ae337b0252 100644 --- a/src/sidebar/services/test/features-test.js +++ b/src/sidebar/services/test/features-test.js @@ -1,4 +1,3 @@ -import bridgeEvents from '../../../shared/bridge-events'; import { FeaturesService } from '../features'; describe('FeaturesService', () => { @@ -52,7 +51,7 @@ describe('FeaturesService', () => { assert.calledWith( fakeFrameSync.notifyHost, - bridgeEvents.FEATURE_FLAGS_UPDATED, + 'featureFlagsUpdated', fakeStore.profile().features ); }); @@ -71,7 +70,7 @@ describe('FeaturesService', () => { assert.calledWith( fakeFrameSync.notifyHost, - bridgeEvents.FEATURE_FLAGS_UPDATED, + 'featureFlagsUpdated', fakeStore.profile().features ); }); diff --git a/src/types/bridge-events.d.ts b/src/types/bridge-events.d.ts new file mode 100644 index 00000000000..932ca4fb001 --- /dev/null +++ b/src/types/bridge-events.d.ts @@ -0,0 +1,161 @@ +/** + * This module defines the set of global events that are dispatched across the + * the bridge(s) between the sidebar-host and sidebar-guest(s). + */ + +/** + * Events that the host sends to the sidebar + */ +export type HostToSidebarEvent = + /** + * The host is asking the sidebar to delete a frame. + */ + | 'destroyFrame' + + /** + * The host is asking the sidebar to set the annotation highlights on/off. + */ + | 'setVisibleHighlights' + + /** + * The host informs the sidebar that the sidebar has been opened. + */ + | 'sidebarOpened'; + +/** + * Events that the guest sends to the sidebar + */ +export type GuestToSidebarEvent = + /** + * The guest is asking the sidebar to create an annotation. + */ + | 'beforeCreateAnnotation' + + /** + * The guest is asking the sidebar to relay the message to open the sidebar. + */ + | 'closeSidebar' + + /** + * The guest is asking the sidebar to focus on certain annotations. + */ + | 'focusAnnotations' + + /** + * The guest is asking the sidebar to relay the message to open the sidebar. + */ + | 'openSidebar' + + /** + * The guest is asking the sidebar to display some annotations. + */ + | 'showAnnotations' + + /** + * The guest notifies the sidebar to synchronize about the anchoring status of annotations. + */ + | 'sync' + + /** + * The guest is asking the sidebar to toggle some annotations. + */ + | 'toggleAnnotationSelection'; + +/** + * Events that the sidebar sends to the guest(s) + */ +export type SidebarToGuestEvent = + /** + * The sidebar is asking the guest(s) to delete an annotation. + */ + | 'deleteAnnotation' + + /** + * The sidebar is asking the guest(s) to focus on certain annotations. + */ + | 'focusAnnotations' + + /** + * The sidebar is asking the guest(s) to get the document metadata. + */ + | 'getDocumentInfo' + + /** + * The sidebar is asking the guest(s) to load annotations. + */ + | 'loadAnnotations' + + /** + * The sidebar is asking the guest(s) to scroll to certain annotation. + */ + | 'scrollToAnnotation' + + /** + * The sidebar relays to the guest(s) to set the annotation highlights on/off. + */ + | 'setVisibleHighlights'; + +/** + * Events that the sidebar sends to the host + */ +export type SidebarToHostEvent = + /** + * The sidebar relays to the host to open the sidebar. + */ + | 'closeSidebar' + + /** + * The updated feature flags for the user + */ + | 'featureFlagsUpdated' + + /** + * The sidebar is asking the host to open the partner site help page. + */ + | 'helpRequested' + + /** + * The sidebar is asking the host to do a partner site log in + * (for example pop up a log in window). This is used when the client is + * embedded in a partner site and a log in button in the client is clicked. + */ + | 'loginRequested' + + /** + * The sidebar is asking the host to do a partner site log out. + * This is used when the client is embedded in a partner site and a log out + * button in the client is clicked. + */ + | 'logoutRequested' + + /** + * The sidebar is asking the host to open the notebook. + */ + | 'openNotebook' + + /** + * The sidebar is asking the host to open the sidebar (side-effect of + * creating an annotation). + */ + | 'openSidebar' + + /** + * The sidebar is asking the host to open the partner site profile page. + */ + | 'profileRequested' + + /** + * The sidebar inform the host to update the number of annotations in the partner site. + */ + | 'publicAnnotationCountChanged' + + /** + * The sidebar is asking the host to do a partner site sign-up. + */ + | 'signupRequested'; + +export type BridgeEvent = + | HostToSidebarEvent + | GuestToSidebarEvent + | SidebarToGuestEvent + | SidebarToHostEvent;