From 94fb864283f86f7afc17c59e7690502bf010bea9 Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Thu, 14 Oct 2021 11:39:10 +0100
Subject: [PATCH] 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.
---
 src/annotator/sidebar.js                     |  4 +++-
 src/annotator/test/sidebar-test.js           |  9 +++++++++
 src/sidebar/services/frame-sync.js           |  5 +++++
 src/sidebar/services/test/frame-sync-test.js | 10 ++++++++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js
index 5eb4fa86c9d..b39f377990a 100644
--- a/src/annotator/sidebar.js
+++ b/src/annotator/sidebar.js
@@ -121,7 +121,6 @@ export default class Sidebar {
       // When a new non-highlight annotation is created, focus
       // the sidebar so that the text editor can be focused as
       // soon as the annotation card appears
-      this.setHighlightsVisible(true);
       if (!annotation.$highlight) {
         /** @type {Window} */ (this.iframe.contentWindow).focus();
       }
@@ -245,6 +244,9 @@ export default class Sidebar {
     sidebarTrigger(document.body, () => this.open());
     features.init(this._sidebarRPC);
 
+    this._sidebarRPC.on('showHighlights', () =>
+      this.setHighlightsVisible(true)
+    );
     this._sidebarRPC.on('openSidebar', () => this.open());
     this._sidebarRPC.on('closeSidebar', () => this.close());
 
diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js
index 51eb68cf7ab..9b92c3a9d81 100644
--- a/src/annotator/test/sidebar-test.js
+++ b/src/annotator/test/sidebar-test.js
@@ -323,6 +323,15 @@ describe('Sidebar', () => {
       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', () =>
       it('opens the frame', () => {
         const target = sandbox.stub(Sidebar.prototype, 'open');
diff --git a/src/sidebar/services/frame-sync.js b/src/sidebar/services/frame-sync.js
index e4ec330d262..ca0bdddcb3e 100644
--- a/src/sidebar/services/frame-sync.js
+++ b/src/sidebar/services/frame-sync.js
@@ -153,6 +153,11 @@ export class FrameSyncService {
         }
         inFrame.add(event.tag);
 
+        // 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.
         annotationsService.create(annot);
       });
diff --git a/src/sidebar/services/test/frame-sync-test.js b/src/sidebar/services/test/frame-sync-test.js
index 84e734f13f0..315fbce88b8 100644
--- a/src/sidebar/services/test/frame-sync-test.js
+++ b/src/sidebar/services/test/frame-sync-test.js
@@ -310,6 +310,16 @@ describe('FrameSyncService', () => {
   });
 
   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', () => {
       it('creates the annotation in the sidebar', () => {
         frameSync.connect();