Skip to content

C
coopwire-hypothesis
Commit f484236b authored by Robert Knight's avatar Robert Knight
Browse files
Options

Replace `panelReady` event for triggering initial sidebar display 

The initial un-hiding and opening of the sidebar was previously
triggered by a `panelReady` event emitted by the Guest in the host frame
when it connects.

As part of eliminating the need for the host frame to also be a guest,
change the initial sidebar open to use the `Sidebar.ready` promise
instead, which is resolved when the sidebar applicaiton sends a
`hypothesisSidebarReady` notice to the `Sidebar` app.

Part of https://github.com/hypothesis/client/issues/3798
parent bd69b16e
Hide whitespace changes
Inline Side-by-side
Showing with 77 additions and 83 deletions
src/annotator/guest.js
View file @ f484236b
......@@ -166,7 +166,6 @@ export default class Guest {
// Set up listeners for messages coming from the sidebar to this guest.
this._bridge = new Bridge();
this._bridge.onConnect(() => {
this._emitter.publish('panelReady');
this.setVisibleHighlights(config.showHighlights === 'always');
});
this._connectSidebarEvents();
......
src/annotator/sidebar.js
View file @ f484236b
......@@ -110,13 +110,6 @@ export default class Sidebar {
this._listeners = new ListenerCollection();
this._emitter.subscribe('panelReady', () => {
// Show the UI
if (this.iframeContainer) {
this.iframeContainer.style.display = '';
}
});
this._emitter.subscribe('beforeAnnotationCreated', annotation => {
// When a new non-highlight annotation is created, focus
// the sidebar so that the text editor can be focused as
......@@ -126,15 +119,6 @@ export default class Sidebar {
}
});
if (
config.openSidebar ||
config.annotations ||
config.query ||
config.group
) {
this._emitter.subscribe('panelReady', () => this.open());
}
// Set up the toolbar on the left edge of the sidebar.
const toolbarContainer = document.createElement('div');
this.toolbar = new ToolbarController(toolbarContainer, {
......@@ -210,6 +194,22 @@ export default class Sidebar {
});
});
this.ready.then(() => {
// Show the UI
if (this.iframeContainer) {
this.iframeContainer.style.display = '';
}
if (
config.openSidebar ||
config.annotations ||
config.query ||
config.group
) {
this.open();
}
});
// Notify sidebar when a guest is unloaded. This message is routed via
// the host frame because in Safari guest frames are unable to send messages
// directly to the sidebar during a window's 'unload' event.
......
src/annotator/test/guest-test.js
View file @ f484236b
......@@ -154,14 +154,6 @@ describe('Guest', () => {
assert.deepEqual(FakeAnnotationSync.lastCall.args[0], eventBus);
});
it('publishes the "panelReady" event when a connection is established', () => {
const handler = sandbox.stub();
const guest = createGuest();
guest._emitter.subscribe('panelReady', handler);
fakeBridge.onConnect.yield();
assert.called(handler);
});
describe('event subscription', () => {
let emitter;
let guest;
......
src/annotator/test/sidebar-test.js
View file @ f484236b
......@@ -8,6 +8,25 @@ const DEFAULT_WIDTH = 350;
const DEFAULT_HEIGHT = 600;
const EXTERNAL_CONTAINER_SELECTOR = 'test-external-container';
/**
* Simulate the sidebar application notifying the host frame that it has loaded
* and is ready to communicate with the host and guest frames.
*/
function sendSidebarReadyMessage() {
const channel = new MessageChannel();
const event = new MessageEvent(
'message',
{
data: { type: 'hypothesisSidebarReady' },
},
[channel.port1]
);
window.dispatchEvent(event);
return event;
}
describe('Sidebar', () => {
const sandbox = sinon.createSandbox();
let fakeGuest;
......@@ -141,9 +160,10 @@ describe('Sidebar', () => {
);
});
it('becomes visible when the "panelReady" event fires', () => {
it('becomes visible when the sidebar application has loaded', async () => {
const sidebar = createSidebar();
sidebar._emitter.publish('panelReady');
sendSidebarReadyMessage();
await sidebar.ready;
assert.equal(sidebar.iframeContainer.style.display, '');
});
});
......@@ -158,21 +178,6 @@ describe('Sidebar', () => {
});
});
function sendSidebarReadyMessage() {
const channel = new MessageChannel();
const event = new MessageEvent(
'message',
{
data: { type: 'hypothesisSidebarReady' },
},
[channel.port1]
);
window.dispatchEvent(event);
return event;
}
it('creates Bridge for host <-> sidebar RPC calls when `hypothesisSidebarReady` message is received', () => {
createSidebar();
const event = sendSidebarReadyMessage();
......@@ -532,47 +537,39 @@ describe('Sidebar', () => {
}));
});
describe('panelReady event', () => {
it('opens the sidebar when a direct-linked annotation is present.', () => {
const sidebar = createSidebar({
annotations: 'ann-id',
});
const open = sandbox.stub(sidebar, 'open');
sidebar._emitter.publish('panelReady');
assert.calledOnce(open);
});
it('opens the sidebar when a direct-linked group is present.', () => {
const sidebar = createSidebar({
group: 'group-id',
});
const open = sandbox.stub(sidebar, 'open');
sidebar._emitter.publish('panelReady');
assert.calledOnce(open);
});
it('opens the sidebar when a direct-linked query is present.', () => {
const sidebar = createSidebar({
query: 'tag:foo',
});
const open = sandbox.stub(sidebar, 'open');
sidebar._emitter.publish('panelReady');
assert.calledOnce(open);
});
it('opens the sidebar when openSidebar is set to true.', () => {
const sidebar = createSidebar({
openSidebar: true,
describe('when the sidebar application has loaded', () => {
[
{
test: 'a direct-linked annotation is present',
config: { annotations: 'ann-id' },
},
{
test: 'a direct-linked group is present',
config: { group: 'group-id' },
},
{
test: 'a direct-linked query is present',
config: { query: 'tag:foo' },
},
{
test: '`openSidebar` is set to true',
config: { openSidebar: true },
},
].forEach(({ test, config }) => {
it(`opens the sidebar when ${test}`, async () => {
const sidebar = createSidebar(config);
const open = sandbox.stub(sidebar, 'open');
sendSidebarReadyMessage();
await sidebar.ready;
assert.calledOnce(open);
});
const open = sandbox.stub(sidebar, 'open');
sidebar._emitter.publish('panelReady');
assert.calledOnce(open);
});
it('does not open the sidebar if not configured to.', () => {
it('does not open the sidebar if not configured to', async () => {
const sidebar = createSidebar();
const open = sandbox.stub(sidebar, 'open');
sidebar._emitter.publish('panelReady');
sendSidebarReadyMessage();
await sidebar.ready;
assert.notCalled(open);
});
});
......@@ -653,19 +650,25 @@ describe('Sidebar', () => {
});
describe('window resize events', () => {
it('hides the sidebar if window width is < MIN_RESIZE', () => {
const sidebar = createSidebar({ openSidebar: true });
sidebar._emitter.publish('panelReady');
let sidebar;
beforeEach(async () => {
// Configure the sidebar to open on load and wait for the initial open to
// complete.
sidebar = createSidebar({ openSidebar: true });
sendSidebarReadyMessage();
await sidebar.ready;
});
it('hides the sidebar if window width is < MIN_RESIZE', () => {
window.innerWidth = MIN_RESIZE - 1;
window.dispatchEvent(new Event('resize'));
assert.equal(fakeToolbar.sidebarOpen, false);
});
it('invokes the "open" method when window is resized', () => {
// Calling the 'open' methods adjust the marginLeft at different screen sizes
const sidebar = createSidebar({ openSidebar: true });
sidebar._emitter.publish('panelReady');
sinon.stub(sidebar, 'open');
// Make the window very small
......
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