Apply suggestions from code review

Co-authored-by: Robert Knight <[email protected]>

Author: esanzgar

src/annotator/annotation-sync.js

@@ -1,17 +1,12 @@
 /**
+ * @typedef {import('../shared/bridge').Bridge<GuestToSidebarEvent,SidebarToGuestEvent>} SidebarBridge
  * @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<T,U>} Bridge
- */
-
 /**
  * Message sent between the sidebar and annotator about an annotation that has
  * changed.
@@ -33,7 +28,7 @@
 export class AnnotationSync {
   /**
    * @param {EventBus} eventBus - Event bus for communicating with the annotator code (eg. the Guest)
-   * @param {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>} bridge - Channel for communicating with the sidebar
+   * @param {SidebarBridge} bridge - Channel for communicating with the sidebar
    */
   constructor(eventBus, bridge) {
     this._emitter = eventBus.createEmitter();

src/annotator/test/sidebar-test.js

@@ -358,7 +358,7 @@ describe('Sidebar', () => {
       });
     });
 
-    describe('on "loginRequest" event', () => {
+    describe('on "loginRequested" event', () => {
       it('calls the onLoginRequest callback function if one was provided', () => {
         const onLoginRequest = sandbox.stub();
         createSidebar({ services: [{ onLoginRequest }] });
@@ -425,7 +425,7 @@ describe('Sidebar', () => {
       });
     });
 
-    describe('on LOGOUT_REQUESTED event', () =>
+    describe('on "logoutRequested" event', () =>
       it('calls the onLogoutRequest callback function', () => {
         const onLogoutRequest = sandbox.stub();
         createSidebar({ services: [{ onLogoutRequest }] });
@@ -435,7 +435,7 @@ describe('Sidebar', () => {
         assert.called(onLogoutRequest);
       }));
 
-    describe('on "signupRequest" event', () =>
+    describe('on "signupRequested" event', () =>
       it('calls the onSignupRequest callback function', () => {
         const onSignupRequest = sandbox.stub();
         createSidebar({ services: [{ onSignupRequest }] });
@@ -445,7 +445,7 @@ describe('Sidebar', () => {
         assert.called(onSignupRequest);
       }));
 
-    describe('on "profileRequest" event', () =>
+    describe('on "profileRequested" event', () =>
       it('calls the onProfileRequest callback function', () => {
         const onProfileRequest = sandbox.stub();
         createSidebar({ services: [{ onProfileRequest }] });

src/sidebar/components/test/HypothesisApp-test.js

@@ -223,7 +223,7 @@ describe('HypothesisApp', () => {
         fakeServiceConfig.returns({});
       });
 
-      it('sends "signupRequest" event', () => {
+      it('sends "signupRequested" event', () => {
         const wrapper = createComponent();
         clickSignUp(wrapper);
         assert.calledWith(fakeFrameSync.notifyHost, 'signupRequested');
@@ -287,9 +287,9 @@ describe('HypothesisApp', () => {
       assert.called(fakeToastMessenger.error);
     });
 
-    it('sends LOGIN_REQUESTED event to host page if using a third-party service', async () => {
+    it('sends "loginRequested" event to host page if using a third-party service', async () => {
       // If the client is using a third-party annotation service then clicking
-      // on a login button should send the LOGIN_REQUESTED event over the bridge
+      // on a login button should send the "loginRequested" event over the bridge
       // (so that the partner site we're embedded in can do its own login
       // thing).
       fakeServiceConfig.returns({});
@@ -402,15 +402,15 @@ describe('HypothesisApp', () => {
 
       addCommonLogoutTests();
 
-      it('sends LOGOUT_REQUESTED', async () => {
+      it('sends "logoutRequested"', async () => {
         const wrapper = createComponent();
         await clickLogOut(wrapper);
 
         assert.calledOnce(fakeFrameSync.notifyHost);
         assert.calledWithExactly(fakeFrameSync.notifyHost, 'logoutRequested');
       });
 
-      it('does not send LOGOUT_REQUESTED if the user cancels the prompt', async () => {
+      it('does not send "logoutRequested" if the user cancels the prompt', async () => {
         fakeStore.countDrafts.returns(1);
         fakeConfirm.returns(false);
 

src/types/bridge-events.d.ts

@@ -1,19 +1,19 @@
 /**
- * This module defines the set of global events that are dispatched across the
- * the bridge(s) between the sidebar-host and sidebar-guest(s).
+ * This module defines the events that are sent between frames with different
+ * roles in the client (guest, host, sidebar).
  */
 
 /**
  * Events that the host sends to the sidebar
  */
 export type HostToSidebarEvent =
   /**
-   * The host is asking the sidebar to delete a frame.
+   * The host informs the sidebar that a guest frame has been destroyed
    */
   | 'destroyFrame'
 
   /**
-   * The host is asking the sidebar to set the annotation highlights on/off.
+   * Highlights have been toggled on/off.
    */
   | 'setVisibleHighlights'
 
@@ -32,7 +32,7 @@ export type GuestToSidebarEvent =
   | 'beforeCreateAnnotation'
 
   /**
-   * The guest is asking the sidebar to relay the message to open the sidebar.
+   * The guest is asking the sidebar to relay the message to the host to close the sidebar.
    */
   | 'closeSidebar'
 
@@ -42,7 +42,7 @@ export type GuestToSidebarEvent =
   | 'focusAnnotations'
 
   /**
-   * The guest is asking the sidebar to relay the message to open the sidebar.
+   * The guest is asking the sidebar to relay the message to the host to open the sidebar.
    */
   | 'openSidebar'
 
@@ -52,7 +52,7 @@ export type GuestToSidebarEvent =
   | 'showAnnotations'
 
   /**
-   * The guest notifies the sidebar to synchronize about the anchoring status of annotations.
+   * The guest informs the sidebar whether annotations were successfully anchored
    */
   | 'sync'
 
@@ -76,7 +76,7 @@ export type SidebarToGuestEvent =
   | 'focusAnnotations'
 
   /**
-   * The sidebar is asking the guest(s) to get the document metadata.
+   * The sidebar is asking the guest(s) the URL and other metadata about the document.
    */
   | 'getDocumentInfo'
 
@@ -100,31 +100,34 @@ export type SidebarToGuestEvent =
  */
 export type SidebarToHostEvent =
   /**
-   * The sidebar relays to the host to open the sidebar.
+   * The sidebar relays to the host to close the sidebar.
    */
   | 'closeSidebar'
 
   /**
-   * The updated feature flags for the user
+   * The sidebar informs the host to update the Hypothesis configuration to enable/disable additional features
    */
   | 'featureFlagsUpdated'
 
   /**
    * The sidebar is asking the host to open the partner site help page.
+   * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onhelprequest
    */
   | '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.
+   * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onloginrequest
    */
   | '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.
+   * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onlogoutrequest
    */
   | 'logoutRequested'
 
@@ -134,23 +137,26 @@ export type SidebarToHostEvent =
   | 'openNotebook'
 
   /**
-   * The sidebar is asking the host to open the sidebar (side-effect of
-   * creating an annotation).
+   * 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.
+   * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onprofilerequest
    */
   | 'profileRequested'
 
   /**
-   * The sidebar inform the host to update the number of annotations in the partner site.
+   * The sidebar informs the host to update the annotation counter in the partner site.
+   * https://h.readthedocs.io/projects/client/en/latest/publishers/host-page-integration/#cmdoption-arg-data-hypothesis-annotation-count
    */
   | 'publicAnnotationCountChanged'
 
   /**
    * The sidebar is asking the host to do a partner site sign-up.
+   * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onsignuprequest
    */
   | 'signupRequested';