Commit 565b543d authored by Robert Knight's avatar Robert Knight

Make showing highlights when creating an annotation work for all guests

Change the logic for making highlights visible when creating a new
annotation, so that it works if the annotation was created in a non-host
frame.

This is done by moving the logic from the `beforeAnnotationCreated`
handler in the Sidebar class, which was only called for annotations in
the host frame, to the `beforeCreateAnnotation` handler in
FrameSyncService, which is called for all frames. The sidebar app will
then send a request to show highlights to the host frame, which will
update the sidebar's controls and then relay the request to guest
frames.
parent 7d5ff320
...@@ -239,6 +239,9 @@ export default class Sidebar { ...@@ -239,6 +239,9 @@ export default class Sidebar {
sidebarTrigger(document.body, () => this.open()); sidebarTrigger(document.body, () => this.open());
features.init(this._sidebarRPC); features.init(this._sidebarRPC);
this._sidebarRPC.on('showHighlights', () =>
this.setHighlightsVisible(true)
);
this._sidebarRPC.on('openSidebar', () => this.open()); this._sidebarRPC.on('openSidebar', () => this.open());
this._sidebarRPC.on('closeSidebar', () => this.close()); this._sidebarRPC.on('closeSidebar', () => this.close());
......
...@@ -294,6 +294,15 @@ describe('Sidebar', () => { ...@@ -294,6 +294,15 @@ describe('Sidebar', () => {
return result; return result;
}; };
describe('on "showHighlights" event', () => {
it('makes all highlights visible', () => {
createSidebar();
assert.isFalse(fakeToolbar.highlightsVisible);
emitEvent('showHighlights');
assert.isTrue(fakeToolbar.highlightsVisible);
});
});
describe('on "open" event', () => describe('on "open" event', () =>
it('opens the frame', () => { it('opens the frame', () => {
const target = sandbox.stub(Sidebar.prototype, 'open'); const target = sandbox.stub(Sidebar.prototype, 'open');
......
...@@ -167,6 +167,11 @@ export class FrameSyncService { ...@@ -167,6 +167,11 @@ export class FrameSyncService {
this._hostRPC.call('openSidebar'); this._hostRPC.call('openSidebar');
} }
// Ensure that the highlight for the newly-created annotation is visible.
// Currently we only support a single, shared visibility state for all highlights
// in all frames, so this will make all existing highlights visible too.
this._hostRPC.call('showHighlights');
// Create the new annotation in the sidebar. // Create the new annotation in the sidebar.
annotationsService.create(annot); annotationsService.create(annot);
}); });
......
...@@ -310,6 +310,16 @@ describe('FrameSyncService', () => { ...@@ -310,6 +310,16 @@ describe('FrameSyncService', () => {
}); });
context('when a new annotation is created in the frame', () => { context('when a new annotation is created in the frame', () => {
it('makes the new highlight visible in the frame', () => {
frameSync.connect();
fakeStore.isLoggedIn.returns(true);
const ann = { target: [] };
guestBridge().emit('beforeCreateAnnotation', { tag: 't1', msg: ann });
assert.calledWith(hostBridge().call, 'showHighlights');
});
context('when an authenticated user is present', () => { context('when an authenticated user is present', () => {
it('creates the annotation in the sidebar', () => { it('creates the annotation in the sidebar', () => {
frameSync.connect(); frameSync.connect();
...@@ -344,7 +354,7 @@ describe('FrameSyncService', () => { ...@@ -344,7 +354,7 @@ describe('FrameSyncService', () => {
guestBridge().emit('beforeCreateAnnotation', { tag: 't1', msg: ann }); guestBridge().emit('beforeCreateAnnotation', { tag: 't1', msg: ann });
assert.notCalled(hostBridge().call); assert.neverCalledWith(hostBridge().call, 'openSidebar');
}); });
}); });
......
...@@ -15,7 +15,7 @@ export type HostToSidebarEvent = ...@@ -15,7 +15,7 @@ export type HostToSidebarEvent =
/** /**
* Highlights have been toggled on/off. * Highlights have been toggled on/off.
*/ */
| 'setVisibleHighlights' | 'setHighlightsVisible'
/** /**
* The host informs the sidebar that the sidebar has been opened. * The host informs the sidebar that the sidebar has been opened.
...@@ -93,7 +93,7 @@ export type SidebarToGuestEvent = ...@@ -93,7 +93,7 @@ export type SidebarToGuestEvent =
/** /**
* The sidebar relays to the guest(s) to set the annotation highlights on/off. * The sidebar relays to the guest(s) to set the annotation highlights on/off.
*/ */
| 'setVisibleHighlights'; | 'setHighlightsVisible';
/** /**
* Events that the sidebar sends to the host * Events that the sidebar sends to the host
...@@ -142,6 +142,11 @@ export type SidebarToHostEvent = ...@@ -142,6 +142,11 @@ export type SidebarToHostEvent =
*/ */
| 'openSidebar' | 'openSidebar'
/**
* The sidebar requests the host to enable the "Show highlights" control.
*/
| 'showHighlights'
/** /**
* 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 * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onprofilerequest
......
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