Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types for bridge events #3829

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@
"scripts": {
"build": "cross-env NODE_ENV=production gulp build",
"lint": "eslint .",
"checkformatting": "prettier --check '**/*.{js,scss}'",
"format": "prettier --list-different --write '**/*.{js,scss}'",
"checkformatting": "prettier --check '**/*.{js,scss,d.ts}'",
"format": "prettier --list-different --write '**/*.{js,scss,d.ts}'",
"test": "gulp test",
"typecheck": "tsc --build tsconfig.json",
"report-coverage": "codecov -f coverage/coverage-final.json",
Expand Down
5 changes: 2 additions & 3 deletions src/annotator/annotation-counts.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import events from '../shared/bridge-events';

const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count';

/**
Expand All @@ -11,8 +9,9 @@ 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('publicAnnotationCountChanged', updateAnnotationCountElems);

function updateAnnotationCountElems(newCount) {
const elems = rootEl.querySelectorAll('[' + ANNOTATION_COUNT_ATTR + ']');
Expand Down
3 changes: 1 addition & 2 deletions src/annotator/features.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import events from '../shared/bridge-events';
import warnOnce from '../shared/warn-once';

let _features = {};
Expand All @@ -12,7 +11,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('featureFlagsUpdated', _set);
},

reset: function () {
Expand Down
12 changes: 6 additions & 6 deletions src/annotator/sidebar.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Hammer from 'hammerjs';

import { Bridge } from '../shared/bridge';
import events from '../shared/bridge-events';
import { ListenerCollection } from '../shared/listener-collection';

import { annotationCounts } from './annotation-counts';
Expand Down Expand Up @@ -252,12 +251,13 @@ export default class Sidebar {
this.show();
});

/** @type {Array<[import('../types/bridge-events').BridgeEvent, 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],
['loginRequested', this.onLoginRequest],
['logoutRequested', this.onLogoutRequest],
['signupRequested', this.onSignupRequest],
['profileRequested', this.onProfileRequest],
['helpRequested', this.onHelpRequest],
];
eventHandlers.forEach(([event, handler]) => {
if (handler) {
Expand Down
3 changes: 1 addition & 2 deletions src/annotator/test/features-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import events from '../../shared/bridge-events';
import { features, $imports } from '../features';

describe('features - annotation layer', () => {
Expand All @@ -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;
}
},
Expand Down
25 changes: 11 additions & 14 deletions src/annotator/test/sidebar-test.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -366,7 +363,7 @@ describe('Sidebar', () => {
const onLoginRequest = sandbox.stub();
createSidebar({ services: [{ onLoginRequest }] });

emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');

assert.called(onLoginRequest);
});
Expand All @@ -386,7 +383,7 @@ describe('Sidebar', () => {
],
});

emitEvent(events.LOGIN_REQUESTED);
emitEvent('loginRequested');

assert.called(firstOnLogin);
assert.notCalled(secondOnLogin);
Expand All @@ -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');
});
});

Expand All @@ -433,7 +430,7 @@ describe('Sidebar', () => {
const onLogoutRequest = sandbox.stub();
createSidebar({ services: [{ onLogoutRequest }] });

emitEvent(events.LOGOUT_REQUESTED);
emitEvent('logoutRequested');

assert.called(onLogoutRequest);
}));
Expand All @@ -443,7 +440,7 @@ describe('Sidebar', () => {
const onSignupRequest = sandbox.stub();
createSidebar({ services: [{ onSignupRequest }] });

emitEvent(events.SIGNUP_REQUESTED);
emitEvent('signupRequested');

assert.called(onSignupRequest);
}));
Expand All @@ -453,7 +450,7 @@ describe('Sidebar', () => {
const onProfileRequest = sandbox.stub();
createSidebar({ services: [{ onProfileRequest }] });

emitEvent(events.PROFILE_REQUESTED);
emitEvent('profileRequested');

assert.called(onProfileRequest);
}));
Expand All @@ -463,7 +460,7 @@ describe('Sidebar', () => {
const onHelpRequest = sandbox.stub();
createSidebar({ services: [{ onHelpRequest }] });

emitEvent(events.HELP_REQUESTED);
emitEvent('helpRequested');

assert.called(onHelpRequest);
}));
Expand Down
48 changes: 0 additions & 48 deletions src/shared/bridge-events.js

This file was deleted.

4 changes: 2 additions & 2 deletions src/shared/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class Bridge {
* Make a method call on all channels, collect the results and pass them to a
* callback when all results are collected.
*
* @param {string} method - Name of remote method to call.
* @param {import('../types/bridge-events').BridgeEvent} method - Name of remote method to call.
* @param {any[]} args - Arguments to method. Final argument is an optional
* callback with this type: `(error: string|Error|null, ...result: any[]) => void`.
* This callback, if any, will be triggered once a response (via `postMessage`)
Expand Down Expand Up @@ -132,7 +132,7 @@ export class Bridge {
* Register a listener to be invoked when any connected channel sends a
* message to this `Bridge`.
*
* @param {string} method
* @param {import('../types/bridge-events').BridgeEvent} method
* @param {(...args: any[]) => void} listener -- Final argument is an optional
* callback of the type: `(error: string|Error|null, ...result: any[]) => void`.
* This callback must be invoked in order to respond (via `postMessage`)
Expand Down
9 changes: 4 additions & 5 deletions src/sidebar/components/HypothesisApp.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import classnames from 'classnames';
import { useEffect, useMemo } from 'preact/hooks';

import bridgeEvents from '../../shared/bridge-events';
import { confirm } from '../../shared/prompts';
import { serviceConfig } from '../config/service-config';
import { useStoreProxy } from '../store/use-store';
import { parseAccountID } from '../helpers/account-id';
import { shouldAutoDisplayTutorial } from '../helpers/session';
import { applyTheme } from '../helpers/theme';
import { withServices } from '../service-context';
import { useStoreProxy } from '../store/use-store';

import AnnotationView from './AnnotationView';
import SidebarView from './SidebarView';
Expand Down Expand Up @@ -98,7 +97,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) {
const login = async () => {
if (serviceConfig(settings)) {
// Let the host page handle the login request
frameSync.notifyHost(bridgeEvents.LOGIN_REQUESTED);
frameSync.notifyHost('loginRequested');
return;
}

Expand All @@ -116,7 +115,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) {
const signUp = () => {
if (serviceConfig(settings)) {
// Let the host page handle the signup request
frameSync.notifyHost(bridgeEvents.SIGNUP_REQUESTED);
frameSync.notifyHost('signupRequested');
return;
}
window.open(store.getLink('signup'));
Expand Down Expand Up @@ -156,7 +155,7 @@ function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) {
store.discardAllDrafts();

if (serviceConfig(settings)) {
frameSync.notifyHost(bridgeEvents.LOGOUT_REQUESTED);
frameSync.notifyHost('logoutRequested');
return;
}

Expand Down
7 changes: 3 additions & 4 deletions src/sidebar/components/TopBar.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { IconButton, LinkButton } from '@hypothesis/frontend-shared';

import bridgeEvents from '../../shared/bridge-events';
import { serviceConfig } from '../config/service-config';
import { useStoreProxy } from '../store/use-store';
import { isThirdPartyService } from '../helpers/is-third-party-service';
import { withServices } from '../service-context';
import { applyTheme } from '../helpers/theme';
import { withServices } from '../service-context';
import { useStoreProxy } from '../store/use-store';

import GroupList from './GroupList';
import SearchInput from './SearchInput';
Expand Down Expand Up @@ -71,7 +70,7 @@ function TopBar({
const requestHelp = () => {
const service = serviceConfig(settings);
if (service && service.onHelpRequestProvided) {
frameSync.notifyHost(bridgeEvents.HELP_REQUESTED);
frameSync.notifyHost('helpRequested');
} else {
store.toggleSidebarPanel('help');
}
Expand Down
3 changes: 1 addition & 2 deletions src/sidebar/components/UserMenu.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { SvgIcon } from '@hypothesis/frontend-shared';
import { useState } from 'preact/hooks';

import bridgeEvents from '../../shared/bridge-events';
import { serviceConfig } from '../config/service-config';
import { isThirdPartyUser } from '../helpers/account-id';
import { useStoreProxy } from '../store/use-store';
Expand Down Expand Up @@ -70,7 +69,7 @@ function UserMenu({ auth, frameSync, onLogout, settings }) {
};

const onProfileSelected = () =>
isThirdParty && frameSync.notifyHost(bridgeEvents.PROFILE_REQUESTED);
isThirdParty && frameSync.notifyHost('profileRequested');

// Generate dynamic props for the profile <MenuItem> component
const profileItemProps = (() => {
Expand Down
14 changes: 3 additions & 11 deletions src/sidebar/components/test/HypothesisApp-test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { mount } from 'enzyme';

import bridgeEvents from '../../../shared/bridge-events';
import mockImportedComponents from '../../../test-util/mock-imported-components';

import HypothesisApp, { $imports } from '../HypothesisApp';

describe('HypothesisApp', () => {
Expand Down Expand Up @@ -228,10 +226,7 @@ describe('HypothesisApp', () => {
it('sends SIGNUP_REQUESTED event', () => {
const wrapper = createComponent();
clickSignUp(wrapper);
assert.calledWith(
fakeFrameSync.notifyHost,
bridgeEvents.SIGNUP_REQUESTED
);
assert.calledWith(fakeFrameSync.notifyHost, 'signupRequested');
});

it('does not open a URL directly', () => {
Expand Down Expand Up @@ -304,7 +299,7 @@ describe('HypothesisApp', () => {

assert.equal(fakeFrameSync.notifyHost.callCount, 1);
assert.isTrue(
fakeFrameSync.notifyHost.calledWithExactly(bridgeEvents.LOGIN_REQUESTED)
fakeFrameSync.notifyHost.calledWithExactly('loginRequested')
);
});
});
Expand Down Expand Up @@ -412,10 +407,7 @@ describe('HypothesisApp', () => {
await clickLogOut(wrapper);

assert.calledOnce(fakeFrameSync.notifyHost);
assert.calledWithExactly(
fakeFrameSync.notifyHost,
bridgeEvents.LOGOUT_REQUESTED
);
assert.calledWithExactly(fakeFrameSync.notifyHost, 'logoutRequested');
});

it('does not send LOGOUT_REQUESTED if the user cancels the prompt', async () => {
Expand Down
10 changes: 2 additions & 8 deletions src/sidebar/components/test/TopBar-test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { mount } from 'enzyme';

import bridgeEvents from '../../../shared/bridge-events';
import TopBar from '../TopBar';
import { $imports } from '../TopBar';

import TopBar, { $imports } from '../TopBar';
import { checkAccessibility } from '../../../test-util/accessibility';
import mockImportedComponents from '../../../test-util/mock-imported-components';

Expand Down Expand Up @@ -123,10 +120,7 @@ describe('TopBar', () => {
helpButton.props().onClick();

assert.equal(fakeStore.toggleSidebarPanel.callCount, 0);
assert.calledWith(
fakeFrameSync.notifyHost,
bridgeEvents.HELP_REQUESTED
);
assert.calledWith(fakeFrameSync.notifyHost, 'helpRequested');
});
});
});
Expand Down
Loading