Re-enforce the use of enums for Bridge events

In contrast to #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

@@ -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 + ']');

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))
     );
   }

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 () {

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);
   }
 
   /**

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
+    );
   }
 
   /**

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;
         }
       },

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,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');
       });
     });
 
@@ -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);
       }));