Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Robert Knight <robertknight@gmail.com>
  • Loading branch information
esanzgar and robertknight committed Oct 18, 2021
1 parent 1748ca4 commit 38cc9d7
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 25 deletions.
9 changes: 2 additions & 7 deletions src/annotator/annotation-sync.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions src/annotator/test/sidebar-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }] });
Expand Down Expand Up @@ -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 }] });
Expand All @@ -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 }] });
Expand All @@ -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 }] });
Expand Down
10 changes: 5 additions & 5 deletions src/sidebar/components/test/HypothesisApp-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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({});
Expand Down Expand Up @@ -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);

Expand Down
18 changes: 9 additions & 9 deletions src/types/bridge-events.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
*/
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'

Expand All @@ -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'

Expand All @@ -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'

Expand All @@ -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'

Expand All @@ -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'

Expand All @@ -100,7 +100,7 @@ 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'

Expand Down Expand Up @@ -134,8 +134,8 @@ 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). It also relays this message (from the guest) to the host.
*/
| 'openSidebar'

Expand Down

0 comments on commit 38cc9d7

Please sign in to comment.