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

Show buckets in the bucket-bar for VitalSource

Background information

Currently, the bucket-bar should display anchor positions only from a
_single_ guest frame. This is because there is no merging mechanism for
anchor positions from multiple guest frames.

Before this PR

* The host frame listened for `anchorsChanged` events from
_all_ the guest frames.

* Only _one_ guest frame sent this event. We referred to this guest
  frame as having the 'main' annotatable content.  The 'main'
  annotatable guest frame was identified by not having a
  `subFrameIdentifier` (a configuration option added to a guest frame
  when the Hypothesis client was injected). Hence, the 'main'
  annotatable guest frame was always the frame where the Hypothesis
  loaded initially (in contrast with the injection mechanism).

In this PR

We have reversed the logic of how the `anchorChanged` RPC
events are send and received:

* _Every_ guest frame sends `anchorsChanged` RPC events to the host
  frame.

* The host frame chooses to listen to only _one_ guest frame for the
  `anchorsChanged` RPC events: the guest frame that connects first with
  the host.

For non-VitalSource case, the first guest frame that connects to the
host frame is the one where the Hypothesis client was initially loaded
(doesn't contain `subFrameIdentifier` option).

For the VitalSource, because of #4176, there is no guest frame in the
host frame. This allows the guest frame in the book content, where the
Hypothesis client was injected, to send anchor positions, and positions
to be reflected in the host's bucket-bar.
parent 259d695e
...@@ -197,13 +197,10 @@ export class Guest { ...@@ -197,13 +197,10 @@ export class Guest {
this._sidebarRPC = new PortRPC(); this._sidebarRPC = new PortRPC();
this._connectSidebar(); this._connectSidebar();
this._bucketBarClient = this._bucketBarClient = new BucketBarClient({
this._frameIdentifier === null
? new BucketBarClient({
contentContainer: this._integration.contentContainer(), contentContainer: this._integration.contentContainer(),
hostRPC: this._hostRPC, hostRPC: this._hostRPC,
}) });
: null;
this._sideBySideActive = false; this._sideBySideActive = false;
...@@ -448,7 +445,7 @@ export class Guest { ...@@ -448,7 +445,7 @@ export class Guest {
this._selectionObserver.disconnect(); this._selectionObserver.disconnect();
this._adder.destroy(); this._adder.destroy();
this._bucketBarClient?.destroy(); this._bucketBarClient.destroy();
removeAllHighlights(this.element); removeAllHighlights(this.element);
...@@ -631,7 +628,7 @@ export class Guest { ...@@ -631,7 +628,7 @@ export class Guest {
_updateAnchors(anchors, notify) { _updateAnchors(anchors, notify) {
this.anchors = anchors; this.anchors = anchors;
if (notify) { if (notify) {
this._bucketBarClient?.update(this.anchors); this._bucketBarClient.update(this.anchors);
} }
} }
......
...@@ -301,15 +301,20 @@ export class Sidebar { ...@@ -301,15 +301,20 @@ export class Sidebar {
); );
// The listener will do nothing if the sidebar doesn't have a bucket bar // The listener will do nothing if the sidebar doesn't have a bucket bar
// (clean theme), but it is still actively listening. // (clean theme)
const bucketBar = this.bucketBar;
// Currently, we ignore `anchorsChanged` for all the guests except the first connected guest.
if (bucketBar) {
guestRPC.on( guestRPC.on(
'anchorsChanged', 'anchorsChanged',
/** @param {AnchorPosition[]} positions */ /** @param {AnchorPosition[]} positions */
positions => { positions => {
// Currently, only one guest frame sends anchor positions to the bucket bar if (this._guestRPC.indexOf(guestRPC) === 0) {
this.bucketBar?.update(positions); bucketBar.update(positions);
}
} }
); );
}
guestRPC.on('frameDestroyed', () => { guestRPC.on('frameDestroyed', () => {
guestRPC.destroy(); guestRPC.destroy();
......
...@@ -1285,7 +1285,7 @@ describe('Guest', () => { ...@@ -1285,7 +1285,7 @@ describe('Guest', () => {
assert.calledWith(sidebarRPC().connect, port1); assert.calledWith(sidebarRPC().connect, port1);
}); });
it('configures the BucketBarClient if guest is the main annotatable frame', () => { it('configures the BucketBarClient', () => {
const contentContainer = document.createElement('div'); const contentContainer = document.createElement('div');
fakeIntegration.contentContainer.returns(contentContainer); fakeIntegration.contentContainer.returns(contentContainer);
...@@ -1297,12 +1297,6 @@ describe('Guest', () => { ...@@ -1297,12 +1297,6 @@ describe('Guest', () => {
}); });
}); });
it('does not configure the BucketBarClient if guest is not the main annotatable frame', () => {
createGuest({ subFrameIdentifier: 'frame-id' });
assert.notCalled(FakeBucketBarClient);
});
it('sends document metadata and URIs to sidebar', async () => { it('sends document metadata and URIs to sidebar', async () => {
createGuest(); createGuest();
await delay(0); await delay(0);
......
...@@ -494,6 +494,17 @@ describe('Sidebar', () => { ...@@ -494,6 +494,17 @@ describe('Sidebar', () => {
assert.calledOnce(sidebar.bucketBar.update); assert.calledOnce(sidebar.bucketBar.update);
assert.calledWith(sidebar.bucketBar.update, anchorPositions); assert.calledWith(sidebar.bucketBar.update, anchorPositions);
sidebar.bucketBar.update.resetHistory();
// Second connected Guest does register a listener for the
// `anchorsChanged` RPC event but it is inactive.
connectGuest(sidebar);
const anchorChangedCallback = fakePortRPCs[2].on
.getCalls()
.find(({ args }) => args[0] === 'anchorsChanged').args[1];
anchorChangedCallback(anchorPositions);
assert.notCalled(sidebar.bucketBar.update);
}); });
}); });
......
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