From b9a2b6c0116dcc46e95d3783ff84d4fddbc6744e 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] Re-enforce the use of enums for `Bridge` events In contrast to https://github.com/hypothesis/client/pull/3829, we have decided to re-enforce the use of the lookup enums for the event names used by `Bridge`. I have added definitions for all the events that are sent across the different frames using various `Bridge`s. I broke down the events into four sections based on the direction of the messages: * guest -> sidebar events * host -> sidebar events * sidebar -> guest/s events * sidebar -> host events --- src/annotator/annotation-counts.js | 8 +- src/annotator/annotation-sync.js | 31 ++-- src/annotator/features.js | 4 +- src/annotator/guest.js | 31 ++-- src/annotator/sidebar.js | 43 +++-- src/annotator/test/features-test.js | 3 +- src/annotator/test/sidebar-test.js | 25 ++- src/shared/bridge-events.js | 163 ++++++++++++++++-- src/shared/bridge.js | 9 +- src/sidebar/components/HypothesisApp.js | 8 +- src/sidebar/components/TopBar.js | 4 +- src/sidebar/components/UserMenu.js | 11 +- .../components/test/HypothesisApp-test.js | 14 +- src/sidebar/components/test/TopBar-test.js | 10 +- src/sidebar/components/test/UserMenu-test.js | 10 +- src/sidebar/services/features.js | 4 +- src/sidebar/services/frame-sync.js | 96 +++++++---- src/sidebar/services/test/features-test.js | 5 +- 18 files changed, 321 insertions(+), 158 deletions(-) diff --git a/src/annotator/annotation-counts.js b/src/annotator/annotation-counts.js index 0be3ef4749e..f8fbd5fb3b4 100644 --- a/src/annotator/annotation-counts.js +++ b/src/annotator/annotation-counts.js @@ -1,4 +1,4 @@ -import events from '../shared/bridge-events'; +import { sidebarToHostEvents } from '../shared/bridge-events'; const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count'; @@ -11,8 +11,12 @@ 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( + sidebarToHostEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, + 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..a92bc45ce84 100644 --- a/src/annotator/annotation-sync.js +++ b/src/annotator/annotation-sync.js @@ -1,3 +1,8 @@ +import { + guestToSidebarEvents, + sidebarToGuestEvents, +} from '../shared/bridge-events'; + /** * @typedef {import('../shared/bridge').Bridge} Bridge * @typedef {import('../types/annotator').AnnotationData} AnnotationData @@ -43,7 +48,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.bridge.on(sidebarToGuestEvents.DELETE_ANNOTATION, (body, callback) => { if (this.destroyed) { callback(null); return; @@ -55,22 +60,28 @@ export class AnnotationSync { callback(null); }); - this.bridge.on('loadAnnotations', (bodies, callback) => { - if (this.destroyed) { + this.bridge.on( + sidebarToGuestEvents.LOAD_ANNOTATIONS, + (bodies, callback) => { + if (this.destroyed) { + callback(null); + return; + } + const annotations = bodies.map(body => this._parse(body)); + this._emitter.publish('annotationsLoaded', annotations); callback(null); - return; } - const annotations = bodies.map(body => this._parse(body)); - this._emitter.publish('annotationsLoaded', annotations); - callback(null); - }); + ); // Relay events from annotator to sidebar. this._emitter.subscribe('beforeAnnotationCreated', annotation => { if (annotation.$tag) { return; } - this.bridge.call('beforeCreateAnnotation', this._format(annotation)); + this.bridge.call( + guestToSidebarEvents.BEFORE_CREATE_ANNOTATION, + this._format(annotation) + ); }); } @@ -88,7 +99,7 @@ export class AnnotationSync { } this.bridge.call( - 'sync', + guestToSidebarEvents.SYNC, annotations.map(ann => this._format(ann)) ); } diff --git a/src/annotator/features.js b/src/annotator/features.js index ba0b3554418..8bb4a305b5e 100644 --- a/src/annotator/features.js +++ b/src/annotator/features.js @@ -1,4 +1,4 @@ -import events from '../shared/bridge-events'; +import { sidebarToHostEvents } from '../shared/bridge-events'; import warnOnce from '../shared/warn-once'; let _features = {}; @@ -12,7 +12,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(sidebarToHostEvents.FEATURE_FLAGS_UPDATED, _set); }, reset: function () { diff --git a/src/annotator/guest.js b/src/annotator/guest.js index 3fd6d35c5fb..f614040eb39 100644 --- a/src/annotator/guest.js +++ b/src/annotator/guest.js @@ -1,3 +1,7 @@ +import { + guestToSidebarEvents, + sidebarToGuestEvents, +} from '../shared/bridge-events'; import { Bridge } from '../shared/bridge'; import { ListenerCollection } from '../shared/listener-collection'; @@ -221,7 +225,7 @@ export default class Guest { // Don't hide the sidebar if the event comes from an element that contains a highlight return; } - this._bridge.call('closeSidebar'); + this._bridge.call(guestToSidebarEvents.CLOSE_SIDEBAR); }; this._listeners.add(this.element, 'mouseup', event => { @@ -310,7 +314,7 @@ export default class Guest { _connectSidebarEvents() { // Handlers for events sent when user hovers or clicks on an annotation card // in the sidebar. - this._bridge.on('focusAnnotations', (tags = []) => { + this._bridge.on(sidebarToGuestEvents.FOCUS_ANNOTATIONS, (tags = []) => { this._focusedAnnotations.clear(); tags.forEach(tag => this._focusedAnnotations.add(tag)); @@ -322,7 +326,7 @@ export default class Guest { } }); - this._bridge.on('scrollToAnnotation', tag => { + this._bridge.on(sidebarToGuestEvents.SCROLL_TO_ANNOTATION, tag => { const anchor = this.anchors.find(a => a.annotation.$tag === tag); if (!anchor?.highlights) { return; @@ -348,16 +352,19 @@ export default class Guest { }); // Handler for when sidebar requests metadata for the current document - this._bridge.on('getDocumentInfo', cb => { + this._bridge.on(sidebarToGuestEvents.GET_DOCUMENT_INFO, cb => { this.getDocumentInfo() .then(info => cb(null, info)) .catch(reason => cb(reason)); }); // Handler for controls on the sidebar - this._bridge.on('setVisibleHighlights', showHighlights => { - this.setVisibleHighlights(showHighlights); - }); + this._bridge.on( + sidebarToGuestEvents.SET_VISIBLE_HIGHLIGHTS, + showHighlights => { + this.setVisibleHighlights(showHighlights); + } + ); } destroy() { @@ -578,7 +585,7 @@ export default class Guest { this.anchor(annotation); if (!annotation.$highlight) { - this._bridge.call('openSidebar'); + this._bridge.call(guestToSidebarEvents.OPEN_SIDEBAR); } return annotation; @@ -592,7 +599,7 @@ export default class Guest { */ _focusAnnotations(annotations) { const tags = annotations.map(a => a.$tag); - this._bridge.call('focusAnnotations', tags); + this._bridge.call(guestToSidebarEvents.FOCUS_ANNOTATIONS, tags); } /** @@ -643,11 +650,11 @@ export default class Guest { selectAnnotations(annotations, toggle = false) { const tags = annotations.map(a => a.$tag); if (toggle) { - this._bridge.call('toggleAnnotationSelection', tags); + this._bridge.call(guestToSidebarEvents.TOGGLE_ANNOTATION_SELECTION, tags); } else { - this._bridge.call('showAnnotations', tags); + this._bridge.call(guestToSidebarEvents.SHOW_ANNOTATIONS, tags); } - this._bridge.call('openSidebar'); + this._bridge.call(guestToSidebarEvents.OPEN_SIDEBAR); } /** diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js index fa77745b9b8..0665b8ea2cf 100644 --- a/src/annotator/sidebar.js +++ b/src/annotator/sidebar.js @@ -1,7 +1,10 @@ import Hammer from 'hammerjs'; import { Bridge } from '../shared/bridge'; -import events from '../shared/bridge-events'; +import { + hostToSidebarEvents, + sidebarToHostEvents, +} from '../shared/bridge-events'; import { ListenerCollection } from '../shared/listener-collection'; import { annotationCounts } from './annotation-counts'; @@ -217,7 +220,10 @@ export default class Sidebar { this._listeners.add(window, 'message', event => { const { data } = /** @type {MessageEvent} */ (event); if (data?.type === 'hypothesisGuestUnloaded') { - this._sidebarRPC.call('destroyFrame', data.frameIdentifier); + this._sidebarRPC.call( + hostToSidebarEvents.DESTROY_FRAME, + data.frameIdentifier + ); } }); } @@ -239,25 +245,29 @@ export default class Sidebar { sidebarTrigger(document.body, () => this.open()); features.init(this._sidebarRPC); - this._sidebarRPC.on('openSidebar', () => this.open()); - this._sidebarRPC.on('closeSidebar', () => this.close()); + this._sidebarRPC.on(sidebarToHostEvents.OPEN_SIDEBAR, () => this.open()); + this._sidebarRPC.on(sidebarToHostEvents.CLOSE_SIDEBAR, () => this.close()); // Sidebar listens to the `openNotebook` event coming from the sidebar's // iframe and re-publishes it via the emitter to the Notebook - this._sidebarRPC.on('openNotebook', (/** @type {string} */ groupId) => { - this.hide(); - this._emitter.publish('openNotebook', groupId); - }); + this._sidebarRPC.on( + sidebarToHostEvents.OPEN_NOTEBOOK, + (/** @type {string} */ groupId) => { + this.hide(); + this._emitter.publish('openNotebook', groupId); + } + ); this._emitter.subscribe('closeNotebook', () => { this.show(); }); + /** @type {Array<[import('../shared/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], + [sidebarToHostEvents.LOGIN_REQUESTED, this.onLoginRequest], + [sidebarToHostEvents.LOGOUT_REQUESTED, this.onLogoutRequest], + [sidebarToHostEvents.SIGNUP_REQUESTED, this.onSignupRequest], + [sidebarToHostEvents.PROFILE_REQUESTED, this.onProfileRequest], + [sidebarToHostEvents.HELP_REQUESTED, this.onHelpRequest], ]; eventHandlers.forEach(([event, handler]) => { if (handler) { @@ -439,7 +449,7 @@ export default class Sidebar { } open() { - this._sidebarRPC.call('sidebarOpened'); + this._sidebarRPC.call(hostToSidebarEvents.SIDEBAR_OPENED); this._emitter.publish('sidebarOpened'); if (this.iframeContainer) { @@ -478,7 +488,10 @@ export default class Sidebar { * @param {boolean} shouldShowHighlights */ setAllVisibleHighlights(shouldShowHighlights) { - this._sidebarRPC.call('setVisibleHighlights', shouldShowHighlights); + this._sidebarRPC.call( + hostToSidebarEvents.SET_VISIBLE_HIGHLIGHTS, + shouldShowHighlights + ); } /** 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/sidebar-test.js b/src/annotator/test/sidebar-test.js index 47fbec3563b..59317f1fbc8 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; @@ -366,7 +363,7 @@ describe('Sidebar', () => { 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,7 +430,7 @@ describe('Sidebar', () => { const onLogoutRequest = sandbox.stub(); createSidebar({ services: [{ onLogoutRequest }] }); - emitEvent(events.LOGOUT_REQUESTED); + emitEvent('logoutRequested'); assert.called(onLogoutRequest); })); @@ -443,7 +440,7 @@ describe('Sidebar', () => { const onSignupRequest = sandbox.stub(); createSidebar({ services: [{ onSignupRequest }] }); - emitEvent(events.SIGNUP_REQUESTED); + emitEvent('signupRequested'); assert.called(onSignupRequest); })); @@ -453,7 +450,7 @@ describe('Sidebar', () => { const onProfileRequest = sandbox.stub(); createSidebar({ services: [{ onProfileRequest }] }); - emitEvent(events.PROFILE_REQUESTED); + emitEvent('profileRequested'); assert.called(onProfileRequest); })); @@ -463,7 +460,7 @@ describe('Sidebar', () => { 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 index fad0aa4b12e..bd940cfcfaa 100644 --- a/src/shared/bridge-events.js +++ b/src/shared/bridge-events.js @@ -1,10 +1,123 @@ /** - * This module defines the set of global events that are dispatched - * across the bridge between the sidebar and annotator + * This module defines the set of global events that are dispatched across the + * the bridge/s between the sidebar-host and sidebar-guest/s. */ -export default { - // Events that the sidebar sends to the annotator - // ---------------------------------------------- + +/** + * Events that the host sends to the sidebar + * + * @typedef {'destroyFrame'|'setVisibleHighlights'|'sidebarOpened'} HostToSidebarEvent + * @type {Record<'DESTROY_FRAME'|'SET_VISIBLE_HIGHLIGHTS'|'SIDEBAR_OPENED', HostToSidebarEvent>} + */ +export const hostToSidebarEvents = { + /** + * The host is asking the sidebar to delete a frame. + */ + DESTROY_FRAME: 'destroyFrame', + + /** + * The host is asking the sidebar to set the annotation highlights on/off. + */ + SET_VISIBLE_HIGHLIGHTS: 'setVisibleHighlights', + + /** + * The host informs the sidebar that the sidebar has been opened. + */ + SIDEBAR_OPENED: 'sidebarOpened', +}; + +/** + * Events that the guest sends to the sidebar + * + * @typedef {'beforeCreateAnnotation'|'closeSidebar'|'focusAnnotations'|'openSidebar'|'showAnnotations'|'sync'|'toggleAnnotationSelection'} GuestToSidebarEvent + * @type {Record<'BEFORE_CREATE_ANNOTATION'|'CLOSE_SIDEBAR'|'FOCUS_ANNOTATIONS'|'OPEN_SIDEBAR'|'SHOW_ANNOTATIONS'|'SYNC'|'TOGGLE_ANNOTATION_SELECTION', GuestToSidebarEvent>} + */ +export const guestToSidebarEvents = { + /** + * The guest is asking the sidebar to create an annotation. + */ + BEFORE_CREATE_ANNOTATION: 'beforeCreateAnnotation', + + /** + * The guest is asking the sidebar to relay the message to open the sidebar. + */ + CLOSE_SIDEBAR: 'closeSidebar', + + /** + * The guest is asking the sidebar to focus on certain annotations. + */ + FOCUS_ANNOTATIONS: 'focusAnnotations', + + /** + * The guest is asking the sidebar to relay the message to open the sidebar. + */ + OPEN_SIDEBAR: 'openSidebar', + + /** + * The guest is asking the sidebar to display some annotations. + */ + SHOW_ANNOTATIONS: 'showAnnotations', + + /** + * The guest notifies the sidebar to synchronize about the anchoring status of annotations. + */ + SYNC: 'sync', + + /** + * The guest is asking the sidebar to toggle some annotations. + */ + TOGGLE_ANNOTATION_SELECTION: 'toggleAnnotationSelection', +}; + +/** + * Events that the sidebar sends to the guest/s + * + * @typedef {'deleteAnnotation'|'focusAnnotations'|'getDocumentInfo'|'loadAnnotations'|'scrollToAnnotation'|'setVisibleHighlights'} SidebarToGuestEvent + * @type {Record<'DELETE_ANNOTATION'|'FOCUS_ANNOTATIONS'|'GET_DOCUMENT_INFO'|'LOAD_ANNOTATIONS'|'SCROLL_TO_ANNOTATION'|'SET_VISIBLE_HIGHLIGHTS', SidebarToGuestEvent>} + */ +export const sidebarToGuestEvents = { + /** + * The sidebar is asking the guest/s to delete an annotation. + */ + DELETE_ANNOTATION: 'deleteAnnotation', + + /** + * The sidebar is asking the guest/s to focus on certain annotations. + */ + FOCUS_ANNOTATIONS: 'focusAnnotations', + + /** + * The sidebar is asking the guest/s to get the document metadata. + */ + GET_DOCUMENT_INFO: 'getDocumentInfo', + + /** + * The sidebar is asking the guest/s to load annotations. + */ + LOAD_ANNOTATIONS: 'loadAnnotations', + + /** + * The sidebar is asking the guest/s to scroll to certain annotation. + */ + SCROLL_TO_ANNOTATION: 'scrollToAnnotation', + + /** + * The sidebar relays to the guest/s to set the annotation highlights on/off. + */ + SET_VISIBLE_HIGHLIGHTS: 'setVisibleHighlights', +}; + +/** + * Events that the sidebar sends to the host + * + * @typedef {'closeSidebar'|'featureFlagsUpdated'|'helpRequested'|'loginRequested'|'logoutRequested'|'openNotebook'|'openSidebar'|'profileRequested'|'publicAnnotationCountChanged'|'signupRequested'} SidebarToHostEvent + * @type {Record<'CLOSE_SIDEBAR'|'FEATURE_FLAGS_UPDATED'|'HELP_REQUESTED'|'LOGIN_REQUESTED'|'LOGOUT_REQUESTED'|'OPEN_NOTEBOOK'|'OPEN_SIDEBAR'|'PROFILE_REQUESTED'|'PUBLIC_ANNOTATION_COUNT_CHANGED'|'SIGNUP_REQUESTED', SidebarToHostEvent>} + */ +export const sidebarToHostEvents = { + /** + * The sidebar relays to the host to open the sidebar. + */ + CLOSE_SIDEBAR: 'closeSidebar', /** * The updated feature flags for the user @@ -12,37 +125,51 @@ export default { FEATURE_FLAGS_UPDATED: 'featureFlagsUpdated', /** - * The sidebar is asking the annotator to open the partner site help page. + * The sidebar is asking the host 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. + /** + * 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. */ 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. + /** + * 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. */ LOGOUT_REQUESTED: 'logoutRequested', /** - * The sidebar is asking the annotator to open the partner site profile page. + * The sidebar is asking the host to open the notebook. + */ + OPEN_NOTEBOOK: 'openNotebook', + + /** + * The sidebar is asking the host to open the sidebar (side-effect of + * creating an annotation). + */ + OPEN_SIDEBAR: 'openSidebar', + + /** + * The sidebar is asking the host to open the partner site profile page. */ PROFILE_REQUESTED: 'profileRequested', /** - * The set of annotations was updated. + * The sidebar inform the host to update the number of annotations in the partner site. */ PUBLIC_ANNOTATION_COUNT_CHANGED: 'publicAnnotationCountChanged', /** - * The sidebar is asking the annotator to do a partner site sign-up. + * The sidebar is asking the host to do a partner site sign-up. */ SIGNUP_REQUESTED: 'signupRequested', - - // Events that the annotator sends to the sidebar - // ---------------------------------------------- }; + +/** + * @typedef {HostToSidebarEvent|GuestToSidebarEvent|SidebarToGuestEvent|SidebarToHostEvent} BridgeEvent + */ diff --git a/src/shared/bridge.js b/src/shared/bridge.js index c777de42836..42f22ddc203 100644 --- a/src/shared/bridge.js +++ b/src/shared/bridge.js @@ -1,6 +1,9 @@ import { PortRPC } from './port-rpc'; -/** @typedef {import('../types/annotator').Destroyable} Destroyable */ +/** + * @typedef {import('./bridge-events').BridgeEvent} BridgeEvent + * @typedef {import('../types/annotator').Destroyable} Destroyable + */ /** * The Bridge service sets up a channel between frames and provides an events @@ -66,7 +69,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 {BridgeEvent} 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 +135,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'|BridgeEvent} 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..ac02a7f266c 100644 --- a/src/sidebar/components/HypothesisApp.js +++ b/src/sidebar/components/HypothesisApp.js @@ -1,7 +1,7 @@ import classnames from 'classnames'; import { useEffect, useMemo } from 'preact/hooks'; -import bridgeEvents from '../../shared/bridge-events'; +import { sidebarToHostEvents } from '../../shared/bridge-events'; import { confirm } from '../../shared/prompts'; import { serviceConfig } from '../config/service-config'; import { useStoreProxy } from '../store/use-store'; @@ -98,7 +98,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(sidebarToHostEvents.LOGIN_REQUESTED); return; } @@ -116,7 +116,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(sidebarToHostEvents.SIGNUP_REQUESTED); return; } window.open(store.getLink('signup')); @@ -156,7 +156,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) { store.discardAllDrafts(); if (serviceConfig(settings)) { - frameSync.notifyHost(bridgeEvents.LOGOUT_REQUESTED); + frameSync.notifyHost(sidebarToHostEvents.LOGOUT_REQUESTED); return; } diff --git a/src/sidebar/components/TopBar.js b/src/sidebar/components/TopBar.js index fb7ad296010..ce80d852941 100644 --- a/src/sidebar/components/TopBar.js +++ b/src/sidebar/components/TopBar.js @@ -1,6 +1,6 @@ import { IconButton, LinkButton } from '@hypothesis/frontend-shared'; -import bridgeEvents from '../../shared/bridge-events'; +import { sidebarToHostEvents } from '../../shared/bridge-events'; import { serviceConfig } from '../config/service-config'; import { useStoreProxy } from '../store/use-store'; import { isThirdPartyService } from '../helpers/is-third-party-service'; @@ -71,7 +71,7 @@ function TopBar({ const requestHelp = () => { const service = serviceConfig(settings); if (service && service.onHelpRequestProvided) { - frameSync.notifyHost(bridgeEvents.HELP_REQUESTED); + frameSync.notifyHost(sidebarToHostEvents.HELP_REQUESTED); } else { store.toggleSidebarPanel('help'); } diff --git a/src/sidebar/components/UserMenu.js b/src/sidebar/components/UserMenu.js index 77b574f5766..674bc78f1d6 100644 --- a/src/sidebar/components/UserMenu.js +++ b/src/sidebar/components/UserMenu.js @@ -1,11 +1,11 @@ import { SvgIcon } from '@hypothesis/frontend-shared'; import { useState } from 'preact/hooks'; -import bridgeEvents from '../../shared/bridge-events'; +import { sidebarToHostEvents } 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'; @@ -57,7 +57,10 @@ function UserMenu({ auth, frameSync, onLogout, settings }) { !isThirdParty || serviceSupports('onLogoutRequestProvided'); const onSelectNotebook = () => { - frameSync.notifyHost('openNotebook', store.focusedGroupId()); + frameSync.notifyHost( + sidebarToHostEvents.OPEN_NOTEBOOK, + store.focusedGroupId() + ); }; // Temporary access to the Notebook without feature flag: @@ -70,7 +73,7 @@ function UserMenu({ auth, frameSync, onLogout, settings }) { }; const onProfileSelected = () => - isThirdParty && frameSync.notifyHost(bridgeEvents.PROFILE_REQUESTED); + isThirdParty && frameSync.notifyHost(sidebarToHostEvents.PROFILE_REQUESTED); // 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..c7c7c6375ba 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', () => { @@ -228,10 +226,7 @@ describe('HypothesisApp', () => { it('sends SIGNUP_REQUESTED 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..1782192377d 100644 --- a/src/sidebar/components/test/TopBar-test.js +++ b/src/sidebar/components/test/TopBar-test.js @@ -1,9 +1,6 @@ import { mount } from 'enzyme'; -import bridgeEvents from '../../../shared/bridge-events'; -import TopBar from '../TopBar'; -import { $imports } from '../TopBar'; - +import TopBar, { $imports } from '../TopBar'; import { checkAccessibility } from '../../../test-util/accessibility'; import mockImportedComponents from '../../../test-util/mock-imported-components'; @@ -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..c847e2d17e6 100644 --- a/src/sidebar/services/features.js +++ b/src/sidebar/services/features.js @@ -1,4 +1,4 @@ -import bridgeEvents from '../../shared/bridge-events'; +import { sidebarToHostEvents } from '../../shared/bridge-events'; import { watch } from '../util/watch'; /** @@ -27,7 +27,7 @@ export class FeaturesService { const currentFlags = () => this._store.profile().features; const sendFeatureFlags = () => { this._frameSync.notifyHost( - bridgeEvents.FEATURE_FLAGS_UPDATED, + sidebarToHostEvents.FEATURE_FLAGS_UPDATED, currentFlags() || {} ); }; diff --git a/src/sidebar/services/frame-sync.js b/src/sidebar/services/frame-sync.js index a240c3e9abd..c7a0b4ed1cb 100644 --- a/src/sidebar/services/frame-sync.js +++ b/src/sidebar/services/frame-sync.js @@ -1,7 +1,12 @@ import debounce from 'lodash.debounce'; -import bridgeEvents from '../../shared/bridge-events'; import { Bridge } from '../../shared/bridge'; +import { + hostToSidebarEvents, + guestToSidebarEvents, + sidebarToHostEvents, + sidebarToGuestEvents, +} from '../../shared/bridge-events'; import { isReply, isPublic } from '../helpers/annotation-metadata'; import { watch } from '../util/watch'; @@ -105,13 +110,19 @@ export class FrameSyncService { // when they are added or removed in the sidebar, but not re-anchoring // annotations if their selectors are updated. if (added.length > 0) { - this._guestRPC.call('loadAnnotations', added.map(formatAnnot)); + this._guestRPC.call( + sidebarToGuestEvents.LOAD_ANNOTATIONS, + added.map(formatAnnot) + ); added.forEach(annot => { inFrame.add(annot.$tag); }); } deleted.forEach(annot => { - this._guestRPC.call('deleteAnnotation', formatAnnot(annot)); + this._guestRPC.call( + sidebarToGuestEvents.DELETE_ANNOTATION, + formatAnnot(annot) + ); inFrame.delete(annot.$tag); }); @@ -119,7 +130,7 @@ export class FrameSyncService { if (frames.every(frame => frame.isAnnotationFetchComplete)) { if (publicAnns === 0 || publicAnns !== prevPublicAnns) { this._hostRPC.call( - bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, + sidebarToHostEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, publicAnns ); prevPublicAnns = publicAnns; @@ -136,23 +147,29 @@ export class FrameSyncService { */ this._setupSyncFromGuests = () => { // A new annotation, note or highlight was created in the frame - this._guestRPC.on('beforeCreateAnnotation', event => { - const annot = Object.assign({}, event.msg, { $tag: event.tag }); - // If user is not logged in, we can't really create a meaningful highlight - // or annotation. Instead, we need to open the sidebar, show an error, - // and delete the (unsaved) annotation so it gets un-selected in the - // target document - if (!store.isLoggedIn()) { - this._hostRPC.call('openSidebar'); - store.openSidebarPanel('loginPrompt'); - this._guestRPC.call('deleteAnnotation', formatAnnot(annot)); - return; - } - inFrame.add(event.tag); + this._guestRPC.on( + guestToSidebarEvents.BEFORE_CREATE_ANNOTATION, + event => { + const annot = Object.assign({}, event.msg, { $tag: event.tag }); + // If user is not logged in, we can't really create a meaningful highlight + // or annotation. Instead, we need to open the sidebar, show an error, + // and delete the (unsaved) annotation so it gets un-selected in the + // target document + if (!store.isLoggedIn()) { + this._hostRPC.call(sidebarToHostEvents.OPEN_SIDEBAR); + store.openSidebarPanel('loginPrompt'); + this._guestRPC.call( + sidebarToGuestEvents.DELETE_ANNOTATION, + formatAnnot(annot) + ); + return; + } + inFrame.add(event.tag); - // Create the new annotation in the sidebar. - annotationsService.create(annot); - }); + // Create the new annotation in the sidebar. + annotationsService.create(annot); + } + ); // Map of annotation tag to anchoring status // ('anchored'|'orphan'|'timeout'). @@ -168,7 +185,7 @@ export class FrameSyncService { }, 10); // Anchoring an annotation in the frame completed - this._guestRPC.on('sync', events_ => { + this._guestRPC.on(guestToSidebarEvents.SYNC, events_ => { events_.forEach(event => { inFrame.add(event.tag); anchoringStatusUpdates[event.tag] = event.msg.$orphan @@ -178,25 +195,28 @@ export class FrameSyncService { }); }); - this._guestRPC.on('showAnnotations', tags => { + this._guestRPC.on(guestToSidebarEvents.SHOW_ANNOTATIONS, tags => { store.selectAnnotations(store.findIDsForTags(tags)); store.selectTab('annotation'); }); - this._guestRPC.on('focusAnnotations', tags => { + this._guestRPC.on(guestToSidebarEvents.FOCUS_ANNOTATIONS, tags => { store.focusAnnotations(tags || []); }); - this._guestRPC.on('toggleAnnotationSelection', tags => { - store.toggleSelectedAnnotations(store.findIDsForTags(tags)); - }); + this._guestRPC.on( + guestToSidebarEvents.TOGGLE_ANNOTATION_SELECTION, + tags => { + store.toggleSelectedAnnotations(store.findIDsForTags(tags)); + } + ); - this._guestRPC.on('openSidebar', () => { - this._hostRPC.call('openSidebar'); + this._guestRPC.on(guestToSidebarEvents.OPEN_SIDEBAR, () => { + this._hostRPC.call(sidebarToHostEvents.OPEN_SIDEBAR); }); - this._guestRPC.on('closeSidebar', () => { - this._hostRPC.call('closeSidebar'); + this._guestRPC.on(guestToSidebarEvents.CLOSE_SIDEBAR, () => { + this._hostRPC.call(sidebarToHostEvents.CLOSE_SIDEBAR); }); }; } @@ -212,7 +232,7 @@ export class FrameSyncService { * @param {PortRPC} channel */ const addFrame = channel => { - channel.call('getDocumentInfo', (err, info) => { + channel.call(sidebarToGuestEvents.GET_DOCUMENT_INFO, (err, info) => { if (err) { channel.destroy(); return; @@ -249,14 +269,14 @@ export class FrameSyncService { this._setupSyncToGuests(); this._setupSyncFromGuests(); - this._hostRPC.on('sidebarOpened', () => { + this._hostRPC.on(hostToSidebarEvents.SIDEBAR_OPENED, () => { this._store.setSidebarOpened(true); }); // Listen for notifications of a guest being unloaded. This message is routed // via the host frame rather than coming directly from the unloaded guest // to work around https://bugs.webkit.org/show_bug.cgi?id=231167. - this._hostRPC.on('destroyFrame', frameIdentifier => { + this._hostRPC.on(hostToSidebarEvents.DESTROY_FRAME, frameIdentifier => { const frame = this._store.frames().find(f => f.id === frameIdentifier); if (frame) { this._store.destroyFrame(frame); @@ -265,8 +285,8 @@ export class FrameSyncService { // When user toggles the highlight visibility control in the sidebar container, // update the visibility in all the guest frames. - this._hostRPC.on('setVisibleHighlights', state => { - this._guestRPC.call('setVisibleHighlights', state); + this._hostRPC.on(hostToSidebarEvents.SET_VISIBLE_HIGHLIGHTS, state => { + this._guestRPC.call(sidebarToGuestEvents.SET_VISIBLE_HIGHLIGHTS, state); }); // Create channel for sidebar <-> host communication and send port to host. @@ -282,7 +302,7 @@ export class FrameSyncService { /** * Send an RPC message to the host frame. * - * @param {string} method + * @param {import('../../shared/bridge-events').SidebarToHostEvent} method * @param {any[]} args */ notifyHost(method, ...args) { @@ -299,7 +319,7 @@ export class FrameSyncService { */ focusAnnotations(tags) { this._store.focusAnnotations(tags); - this._guestRPC.call('focusAnnotations', tags); + this._guestRPC.call(sidebarToGuestEvents.FOCUS_ANNOTATIONS, tags); } /** @@ -308,6 +328,6 @@ export class FrameSyncService { * @param {string} tag */ scrollToAnnotation(tag) { - this._guestRPC.call('scrollToAnnotation', tag); + this._guestRPC.call(sidebarToGuestEvents.SCROLL_TO_ANNOTATION, tag); } } 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 ); });