Re-enforce the use of enums for Bridge 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/annotation-counts-test.js

@@ -1,3 +1,4 @@
+import { sidebarToHostEvents } from '../../shared/bridge-events';
 import { annotationCounts } from '../annotation-counts';
 
 describe('annotationCounts', () => {
@@ -57,7 +58,7 @@ describe('annotationCounts', () => {
       const newCount = 10;
       annotationCounts(document.body, fakeCrossFrame);
 
-      emitEvent('publicAnnotationCountChanged', newCount);
+      emitEvent(sidebarToHostEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, newCount);
 
       assert.equal(countEl1.textContent, newCount);
       assert.equal(countEl2.textContent, newCount);

src/annotator/test/annotation-sync-test.js

@@ -1,6 +1,9 @@
-import { EventBus } from '../util/emitter';
-
+import {
+  guestToSidebarEvents,
+  sidebarToGuestEvents,
+} from '../../shared/bridge-events';
 import { AnnotationSync } from '../annotation-sync';
+import { EventBus } from '../util/emitter';
 
 describe('AnnotationSync', () => {
   let createAnnotationSync;
@@ -44,12 +47,12 @@ describe('AnnotationSync', () => {
         emitter.subscribe('annotationDeleted', eventStub);
         createAnnotationSync();
 
-        publish('deleteAnnotation', { msg: ann }, () => {});
+        publish(sidebarToGuestEvents.DELETE_ANNOTATION, { msg: ann }, () => {});
 
         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);
@@ -58,7 +61,7 @@ describe('AnnotationSync', () => {
         };
         createAnnotationSync();
 
-        publish('deleteAnnotation', { msg: ann }, callback);
+        publish(sidebarToGuestEvents.DELETE_ANNOTATION, { msg: ann }, callback);
       });
 
       it('deletes any existing annotation from its cache before publishing event to the annotator', done => {
@@ -70,15 +73,15 @@ describe('AnnotationSync', () => {
           done();
         });
 
-        publish('deleteAnnotation', { msg: ann }, () => {});
+        publish(sidebarToGuestEvents.DELETE_ANNOTATION, { msg: ann }, () => {});
       });
 
       it('deletes any existing annotation from its cache', () => {
         const ann = { id: 1, $tag: 'tag1' };
         const annSync = createAnnotationSync();
         annSync.cache.tag1 = ann;
 
-        publish('deleteAnnotation', { msg: ann }, () => {});
+        publish(sidebarToGuestEvents.DELETE_ANNOTATION, { msg: ann }, () => {});
 
         assert.isUndefined(annSync.cache.tag1);
       });
@@ -100,7 +103,7 @@ describe('AnnotationSync', () => {
         emitter.subscribe('annotationsLoaded', loadedStub);
         createAnnotationSync();
 
-        publish('loadAnnotations', bodies, () => {});
+        publish(sidebarToGuestEvents.LOAD_ANNOTATIONS, bodies, () => {});
 
         assert.calledWith(loadedStub, annotations);
       });
@@ -118,10 +121,14 @@ describe('AnnotationSync', () => {
         emitter.publish('beforeAnnotationCreated', ann);
 
         assert.called(fakeBridge.call);
-        assert.calledWith(fakeBridge.call, 'beforeCreateAnnotation', {
-          msg: ann,
-          tag: ann.$tag,
-        });
+        assert.calledWith(
+          fakeBridge.call,
+          guestToSidebarEvents.BEFORE_CREATE_ANNOTATION,
+          {
+            msg: ann,
+            tag: ann.$tag,
+          }
+        );
       });
 
       it('assigns a non-empty tag to the annotation', () => {
@@ -162,7 +169,9 @@ describe('AnnotationSync', () => {
 
       annotationSync.sync([ann]);
 
-      assert.calledWith(fakeBridge.call, 'sync', [{ msg: ann, tag: ann.$tag }]);
+      assert.calledWith(fakeBridge.call, guestToSidebarEvents.SYNC, [
+        { msg: ann, tag: ann.$tag },
+      ]);
     });
   });
 
@@ -173,8 +182,8 @@ describe('AnnotationSync', () => {
       annotationSync.destroy();
       const cb = sinon.stub();
 
-      publish('loadAnnotations', [ann], cb);
-      publish('deleteAnnotation', ann, cb);
+      publish(sidebarToGuestEvents.LOAD_ANNOTATIONS, [ann], cb);
+      publish(sidebarToGuestEvents.DELETE_ANNOTATION, ann, cb);
 
       assert.calledTwice(cb);
       assert.calledWith(cb.firstCall, null);