Don't close the sidebar for pointer events within the bounds of the s…

…idebar

Fix an issue where clicking in transparent areas of the bucket bar could close
the sidebar when side-by-side mode is disabled. Such clicks are allowed to
bubble up to `document.body` because the user may be trying to click on a
scrollbar thumb or other element in this area. However we don't want these
clicks to close the sidebar like clicking elsewhere on the document does, as
this can be annoying if the user was trying to click on eg. a sidebar bucket but
missed.

In fixing this, the separate mouse and touch event listeners have been replaced
with a pointer event listener, as this is now supported in all of our target
browsers.

src/annotator/guest.ts

@@ -214,6 +214,11 @@ export class Guest extends TinyEmitter implements Annotator, Destroyable {
   /** Channel for guest-sidebar communication. */
   private _sidebarRPC: PortRPC<SidebarToGuestEvent, GuestToSidebarEvent>;
 
+  /**
+   * The most recently received sidebar layout information from the host frame.
+   */
+  private _sidebarLayout: SidebarLayout | null;
+
   private _bucketBarClient: BucketBarClient;
 
   private _listeners: ListenerCollection;
@@ -306,6 +311,7 @@ export class Guest extends TinyEmitter implements Annotator, Destroyable {
     this._connectHost(hostFrame);
 
     this._sidebarRPC = new PortRPC();
+    this._sidebarLayout = null;
     this._connectSidebar();
 
     this._bucketBarClient = new BucketBarClient({
@@ -330,15 +336,27 @@ export class Guest extends TinyEmitter implements Annotator, Destroyable {
     // them from propagating out of their shadow roots, and hence clicking on
     // elements in the sidebar's vertical toolbar or adder won't close the
     // sidebar.
-    const maybeCloseSidebar = (element: Element) => {
+    const maybeCloseSidebar = (event: PointerEvent) => {
       // Don't hide the sidebar if event was disabled because the sidebar
       // doesn't overlap the content.
       if (this._integration.sideBySideActive()) {
         return;
       }
 
       // Don't hide the sidebar if the event comes from an element that contains a highlight
-      if (annotationsAt(element).length) {
+      if (annotationsAt(event.target as Element).length) {
+        return;
+      }
+
+      // If the click is within the bounds of the sidebar, ignore it. We don't
+      // want to close the sidebar if the user clicks eg. in transparent areas
+      // of the toolbar / bucket bar along the edge. Clicks within the sidebar
+      // iframe won't be received by the guest frame(s) at all.
+      if (
+        frameFillsAncestor(window, this._hostFrame) &&
+        this._sidebarLayout?.expanded &&
+        window.innerWidth - event.clientX < this._sidebarLayout.width
+      ) {
         return;
       }
 
@@ -354,15 +372,7 @@ export class Guest extends TinyEmitter implements Annotator, Destroyable {
       }
     });
 
-    this._listeners.add(this.element, 'mousedown', ({ target }) => {
-      maybeCloseSidebar(target as Element);
-    });
-
-    // Allow taps on the document to hide the sidebar as well as clicks.
-    // On iOS < 13 (2019), elements like h2 or div don't emit 'click' events.
-    this._listeners.add(this.element, 'touchstart', ({ target }) => {
-      maybeCloseSidebar(target as Element);
-    });
+    this._listeners.add(this.element, 'pointerdown', maybeCloseSidebar);
 
     this._listeners.add(this.element, 'mouseover', ({ target }) => {
       const tags = annotationsAt(target as Element);
@@ -848,6 +858,7 @@ export class Guest extends TinyEmitter implements Annotator, Destroyable {
    * @param sidebarLayout
    */
   fitSideBySide(sidebarLayout: SidebarLayout) {
+    this._sidebarLayout = sidebarLayout;
     this._integration.fitSideBySide(sidebarLayout);
   }
 

src/annotator/test/guest-test.js

@@ -624,23 +624,37 @@ describe('Guest', () => {
       fakeSidebarFrame?.remove();
     });
 
-    it('hides sidebar on user "mousedown" or "touchstart" events in the document', () => {
-      for (let event of ['mousedown', 'touchstart']) {
-        rootElement.dispatchEvent(new Event(event));
+    context('clicks/taps on the document', () => {
+      const simulateClick = (element = rootElement, clientX = 0) =>
+        element.dispatchEvent(
+          new PointerEvent('pointerdown', { bubbles: true, clientX })
+        );
+
+      it('hides sidebar', () => {
+        simulateClick();
         assert.calledWith(sidebarRPC().call, 'closeSidebar');
-        sidebarRPC().call.resetHistory();
-      }
-    });
+      });
+
+      it('does not hide sidebar if target is a highlight', () => {
+        simulateClick(fakeHighlight);
+        assert.notCalled(sidebarRPC().call);
+      });
 
-    it('does not hide sidebar if side-by-side mode is active', () => {
-      for (let event of ['mousedown', 'touchstart']) {
+      it('does not hide sidebar if side-by-side mode is active', () => {
         fakeIntegration.sideBySideActive.returns(true);
+        simulateClick();
+        assert.notCalled(sidebarRPC().call);
+      });
+
+      it('does not hide sidebar if event is within the bounds of the sidebar', () => {
+        createGuest();
+        emitHostEvent('sidebarLayoutChanged', { expanded: true, width: 300 });
 
-        rootElement.dispatchEvent(new Event(event));
+        // Simulate click on the left edge of the sidebar.
+        simulateClick(rootElement, window.innerWidth - 295);
 
         assert.notCalled(sidebarRPC().call);
-        sidebarRPC().call.resetHistory();
-      }
+      });
     });
 
     it('does not reposition the adder if hidden when the window is resized', () => {