Commit 0b757c64 authored by Eduardo Sanz García's avatar Eduardo Sanz García Committed by Eduardo

Apply suggestions from code review

Co-authored-by: 's avatarRobert Knight <robertknight@gmail.com>
parent ebc5c0e2
/** /**
* @typedef {import('../shared/bridge').Bridge<GuestToSidebarEvent,SidebarToGuestEvent>} SidebarBridge
* @typedef {import('../types/annotator').AnnotationData} AnnotationData * @typedef {import('../types/annotator').AnnotationData} AnnotationData
* @typedef {import('../types/annotator').Destroyable} Destroyable * @typedef {import('../types/annotator').Destroyable} Destroyable
* @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent * @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
...@@ -6,12 +7,6 @@ ...@@ -6,12 +7,6 @@
* @typedef {import('./util/emitter').EventBus} EventBus * @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 * Message sent between the sidebar and annotator about an annotation that has
* changed. * changed.
...@@ -33,7 +28,7 @@ ...@@ -33,7 +28,7 @@
export class AnnotationSync { export class AnnotationSync {
/** /**
* @param {EventBus} eventBus - Event bus for communicating with the annotator code (eg. the Guest) * @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) { constructor(eventBus, bridge) {
this._emitter = eventBus.createEmitter(); this._emitter = eventBus.createEmitter();
......
...@@ -332,7 +332,7 @@ describe('Sidebar', () => { ...@@ -332,7 +332,7 @@ describe('Sidebar', () => {
}); });
}); });
describe('on "loginRequest" event', () => { describe('on "loginRequested" event', () => {
it('calls the onLoginRequest callback function if one was provided', () => { it('calls the onLoginRequest callback function if one was provided', () => {
const onLoginRequest = sandbox.stub(); const onLoginRequest = sandbox.stub();
createSidebar({ services: [{ onLoginRequest }] }); createSidebar({ services: [{ onLoginRequest }] });
...@@ -399,7 +399,7 @@ describe('Sidebar', () => { ...@@ -399,7 +399,7 @@ describe('Sidebar', () => {
}); });
}); });
describe('on LOGOUT_REQUESTED event', () => describe('on "logoutRequested" event', () =>
it('calls the onLogoutRequest callback function', () => { it('calls the onLogoutRequest callback function', () => {
const onLogoutRequest = sandbox.stub(); const onLogoutRequest = sandbox.stub();
createSidebar({ services: [{ onLogoutRequest }] }); createSidebar({ services: [{ onLogoutRequest }] });
...@@ -409,7 +409,7 @@ describe('Sidebar', () => { ...@@ -409,7 +409,7 @@ describe('Sidebar', () => {
assert.called(onLogoutRequest); assert.called(onLogoutRequest);
})); }));
describe('on "signupRequest" event', () => describe('on "signupRequested" event', () =>
it('calls the onSignupRequest callback function', () => { it('calls the onSignupRequest callback function', () => {
const onSignupRequest = sandbox.stub(); const onSignupRequest = sandbox.stub();
createSidebar({ services: [{ onSignupRequest }] }); createSidebar({ services: [{ onSignupRequest }] });
...@@ -419,7 +419,7 @@ describe('Sidebar', () => { ...@@ -419,7 +419,7 @@ describe('Sidebar', () => {
assert.called(onSignupRequest); assert.called(onSignupRequest);
})); }));
describe('on "profileRequest" event', () => describe('on "profileRequested" event', () =>
it('calls the onProfileRequest callback function', () => { it('calls the onProfileRequest callback function', () => {
const onProfileRequest = sandbox.stub(); const onProfileRequest = sandbox.stub();
createSidebar({ services: [{ onProfileRequest }] }); createSidebar({ services: [{ onProfileRequest }] });
......
...@@ -223,7 +223,7 @@ describe('HypothesisApp', () => { ...@@ -223,7 +223,7 @@ describe('HypothesisApp', () => {
fakeServiceConfig.returns({}); fakeServiceConfig.returns({});
}); });
it('sends "signupRequest" event', () => { it('sends "signupRequested" event', () => {
const wrapper = createComponent(); const wrapper = createComponent();
clickSignUp(wrapper); clickSignUp(wrapper);
assert.calledWith(fakeFrameSync.notifyHost, 'signupRequested'); assert.calledWith(fakeFrameSync.notifyHost, 'signupRequested');
...@@ -287,9 +287,9 @@ describe('HypothesisApp', () => { ...@@ -287,9 +287,9 @@ describe('HypothesisApp', () => {
assert.called(fakeToastMessenger.error); 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 // 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 // (so that the partner site we're embedded in can do its own login
// thing). // thing).
fakeServiceConfig.returns({}); fakeServiceConfig.returns({});
...@@ -402,7 +402,7 @@ describe('HypothesisApp', () => { ...@@ -402,7 +402,7 @@ describe('HypothesisApp', () => {
addCommonLogoutTests(); addCommonLogoutTests();
it('sends LOGOUT_REQUESTED', async () => { it('sends "logoutRequested"', async () => {
const wrapper = createComponent(); const wrapper = createComponent();
await clickLogOut(wrapper); await clickLogOut(wrapper);
...@@ -410,7 +410,7 @@ describe('HypothesisApp', () => { ...@@ -410,7 +410,7 @@ describe('HypothesisApp', () => {
assert.calledWithExactly(fakeFrameSync.notifyHost, 'logoutRequested'); 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); fakeStore.countDrafts.returns(1);
fakeConfirm.returns(false); fakeConfirm.returns(false);
......
/** /**
* This module defines the set of global events that are dispatched across the * This module defines the events that are sent between frames with different
* the bridge(s) between the sidebar-host and sidebar-guest(s). * roles in the client (guest, host, sidebar).
*/ */
/** /**
...@@ -8,12 +8,12 @@ ...@@ -8,12 +8,12 @@
*/ */
export type HostToSidebarEvent = 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' | 'destroyFrame'
/** /**
* The host is asking the sidebar to set the annotation highlights on/off. * Highlights have been toggled on/off.
*/ */
| 'setVisibleHighlights' | 'setVisibleHighlights'
...@@ -32,7 +32,7 @@ export type GuestToSidebarEvent = ...@@ -32,7 +32,7 @@ export type GuestToSidebarEvent =
| 'beforeCreateAnnotation' | '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' | 'closeSidebar'
...@@ -42,7 +42,7 @@ export type GuestToSidebarEvent = ...@@ -42,7 +42,7 @@ export type GuestToSidebarEvent =
| 'focusAnnotations' | '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' | 'openSidebar'
...@@ -52,7 +52,7 @@ export type GuestToSidebarEvent = ...@@ -52,7 +52,7 @@ export type GuestToSidebarEvent =
| 'showAnnotations' | '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' | 'sync'
...@@ -76,7 +76,7 @@ export type SidebarToGuestEvent = ...@@ -76,7 +76,7 @@ export type SidebarToGuestEvent =
| 'focusAnnotations' | 'focusAnnotations'
/** /**
* The sidebar is asking the guest(s) to get the document metadata. * The sidebar is asking the guest(s) for the URL and other metadata about the document.
*/ */
| 'getDocumentInfo' | 'getDocumentInfo'
...@@ -100,17 +100,18 @@ export type SidebarToGuestEvent = ...@@ -100,17 +100,18 @@ export type SidebarToGuestEvent =
*/ */
export type SidebarToHostEvent = export type SidebarToHostEvent =
/** /**
* The sidebar relays to the host to open the sidebar. * The sidebar relays to the host to close the sidebar.
*/ */
| 'closeSidebar' | 'closeSidebar'
/** /**
* The updated feature flags for the user * The sidebar informs the host to update the Hypothesis configuration to enable/disable additional features
*/ */
| 'featureFlagsUpdated' | 'featureFlagsUpdated'
/** /**
* The sidebar is asking the host to open the partner site help page. * 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' | 'helpRequested'
...@@ -118,6 +119,7 @@ export type SidebarToHostEvent = ...@@ -118,6 +119,7 @@ export type SidebarToHostEvent =
* The sidebar is asking the host to do a partner site log in * 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 * (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. * 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' | 'loginRequested'
...@@ -125,6 +127,7 @@ export type SidebarToHostEvent = ...@@ -125,6 +127,7 @@ export type SidebarToHostEvent =
* The sidebar is asking the host to do a partner site log out. * 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 * This is used when the client is embedded in a partner site and a log out
* button in the client is clicked. * button in the client is clicked.
* https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onlogoutrequest
*/ */
| 'logoutRequested' | 'logoutRequested'
...@@ -134,23 +137,26 @@ export type SidebarToHostEvent = ...@@ -134,23 +137,26 @@ export type SidebarToHostEvent =
| 'openNotebook' | 'openNotebook'
/** /**
* The sidebar is asking the host to open the sidebar (side-effect of * The sidebar is asking the host to open the sidebar (side-effect of creating
* creating an annotation). * an annotation).
*/ */
| 'openSidebar' | 'openSidebar'
/** /**
* The sidebar is asking the host to open the partner site profile page. * 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' | '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' | 'publicAnnotationCountChanged'
/** /**
* The sidebar is asking the host to do a partner site sign-up. * 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'; | 'signupRequested';
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment