Support rendering bucket bar inside a custom container

Revise how the bucket bar rendering and bucket position calculations work to
remove the assumptions about where the bucket bar will be rendered relative to
the viewport. Then use this new flexibility to support a `bucketBarContainer`
config option which allows the host page to specify a selector for an existing
DOM element where the bucket bar will be rendered. The initial use case is the
video player where we want to render the bucket bar alongside the transcript
rather than the sidebar. In future this can support other use cases where the
document content is in some scrollable container that doesn't fill the viewport.

 - Render bucket bar into its own shadow root, so its styles work when the
   bucket bar is rendered into a custom container.

 - Add `container` argument to `computeBuckets` helper, and compute bucket
   positions relative to this container instead of the viewport.

 - Remove the hard-coded knowledge in `computeBuckets` about the vertical
   toolbar above the bucket bar, and instead change the positioning of the
   default bucket bar container so that it is below the vertical toolbar at the
   edge of the sidebar.

 - Move the gray background behind the bucket bar out of the `Buckets` component
   and into a separate element rendered by the toolbar. This is needed so that
   the background is still displayed even if the bucket bar is rendered into a
   custom container.

   In the process of adding this separate element, tweak the layout so
   that the right edge of the buckets aligns with the right edge of
   the toolbar icons.

 - Add a `bucketBarContainer` config option which specifies a selector for an
   element to render the bucket bar into.

src/annotator/bucket-bar.tsx

@@ -3,6 +3,7 @@ import { render } from 'preact';
 import type { AnchorPosition, Destroyable } from '../types/annotator';
 import Buckets from './components/Buckets';
 import { computeBuckets } from './util/buckets';
+import { createShadowRoot } from './util/shadow-root';
 
 export type BucketBarOptions = {
   onFocusAnnotations: (tags: string[]) => void;
@@ -14,11 +15,13 @@ export type BucketBarOptions = {
 };
 
 /**
- * Controller for the "bucket bar" shown alongside the sidebar indicating where
- * annotations are in the document.
+ * Controller for the "bucket bar" showing where annotations are in the document.
+ *
+ * This is usually positioned along the edge of the sidebar but can be
+ * rendered elsewhere for certain content viewers.
  */
 export class BucketBar implements Destroyable {
-  private _bucketsContainer: HTMLDivElement;
+  private _bucketsContainer: HTMLElement;
   private _onFocusAnnotations: BucketBarOptions['onFocusAnnotations'];
   private _onScrollToClosestOffScreenAnchor: BucketBarOptions['onScrollToClosestOffScreenAnchor'];
   private _onSelectAnnotations: BucketBarOptions['onSelectAnnotations'];
@@ -31,7 +34,11 @@ export class BucketBar implements Destroyable {
       onSelectAnnotations,
     }: BucketBarOptions
   ) {
-    this._bucketsContainer = document.createElement('div');
+    this._bucketsContainer = document.createElement('hypothesis-bucket-bar');
+    this._bucketsContainer.style.display = 'block';
+    this._bucketsContainer.style.width = '100%';
+
+    createShadowRoot(this._bucketsContainer);
     container.appendChild(this._bucketsContainer);
 
     this._onFocusAnnotations = onFocusAnnotations;
@@ -48,7 +55,7 @@ export class BucketBar implements Destroyable {
   }
 
   update(positions: AnchorPosition[]) {
-    const buckets = computeBuckets(positions);
+    const buckets = computeBuckets(positions, this._bucketsContainer);
     render(
       <Buckets
         above={buckets.above}
@@ -62,7 +69,7 @@ export class BucketBar implements Destroyable {
           this._onSelectAnnotations(tags, toogle)
         }
       />,
-      this._bucketsContainer
+      this._bucketsContainer.shadowRoot!
     );
   }
 }

src/annotator/components/Buckets.tsx

 import { PointerButton } from '@hypothesis/frontend-shared';
-import classnames from 'classnames';
 
 import type { Bucket } from '../util/buckets';
 
@@ -51,17 +50,7 @@ export default function Buckets({
   const showDownNavigation = below.tags.size > 0;
 
   return (
-    <ul
-      className={classnames(
-        // 2020-11-20: Making bucket bar one pixel wider (23px vs 22px) is an
-        // interim and pragmatic solution for an apparent glitch on
-        // Safari and Chrome. Adding one pixel resolves this issue:
-        // https://github.com/hypothesis/client/pull/2750
-        'absolute w-[23px] left-[-22px] h-full',
-        // The background is set to low opacity when the sidebar is collapsed.
-        'bg-grey-2 sidebar-collapsed:bg-black/[.08]'
-      )}
-    >
+    <ul className="relative">
       {showUpNavigation && (
         <li
           className="absolute right-0 pointer-events-auto mt-[-11px]"

src/annotator/config/index.ts

@@ -41,6 +41,7 @@ function configurationKeys(context: Context): string[] {
       'appType',
       'annotations',
       'branding',
+      'bucketBarContainer',
       'enableExperimentalNewNoteButton',
       'externalContainerSelector',
       'focus',
@@ -106,6 +107,11 @@ const configDefinitions: ConfigDefinitionMap = {
     allowInBrowserExt: false,
     getValue: getHostPageSetting,
   },
+  bucketBarContainer: {
+    defaultValue: null,
+    allowInBrowserExt: false,
+    getValue: getHostPageSetting,
+  },
   // URL of the client's boot script. Used when injecting the client into
   // child iframes.
   clientUrl: {

src/annotator/config/test/index-test.js

@@ -77,6 +77,7 @@ describe('annotator/config/index', () => {
             appType: 'fakeValue',
             annotations: 'fakeValue',
             branding: null,
+            bucketBarContainer: null,
             clientUrl: 'fakeValue',
             contentInfoBanner: null,
             enableExperimentalNewNoteButton: null,
@@ -110,6 +111,7 @@ describe('annotator/config/index', () => {
             appType: 'fakeValue',
             annotations: 'fakeValue',
             branding: 'fakeValue',
+            bucketBarContainer: 'fakeValue',
             clientUrl: 'fakeValue',
             contentInfoBanner: 'fakeValue',
             enableExperimentalNewNoteButton: 'fakeValue',
@@ -167,6 +169,7 @@ describe('annotator/config/index', () => {
         appType: null,
         annotations: null,
         branding: null,
+        bucketBarContainer: null,
         clientUrl: null,
         contentInfoBanner: null,
         enableExperimentalNewNoteButton: null,
@@ -232,6 +235,7 @@ describe('annotator/config/index', () => {
           'appType',
           'annotations',
           'branding',
+          'bucketBarContainer',
           'enableExperimentalNewNoteButton',
           'externalContainerSelector',
           'focus',

src/annotator/sidebar.tsx

+import classnames from 'classnames';
 import * as Hammer from 'hammerjs';
 import { render } from 'preact';
 
@@ -43,6 +44,9 @@ export type SidebarConfig = { sidebarAppUrl: string } & Record<string, unknown>;
  * Client configuration used by the sidebar container ({@link Sidebar}).
  */
 export type SidebarContainerConfig = {
+  /** CSS selector for the container of the bucket bar. */
+  bucketBarContainer?: string;
+
   /**
    * Details of the annotation service the client should connect to.
    * This includes callbacks provided by the host page to handle certain actions
@@ -168,7 +172,46 @@ export class Sidebar implements Destroyable {
       if (config.theme === 'clean') {
         this.iframeContainer.classList.add('theme-clean');
       } else {
-        this.bucketBar = new BucketBar(this.iframeContainer, {
+        let bucketBarContainer: HTMLElement | undefined;
+        if (config.bucketBarContainer) {
+          bucketBarContainer = document.querySelector(
+            config.bucketBarContainer
+          ) as HTMLElement | undefined;
+          if (!bucketBarContainer) {
+            console.warn(
+              `Custom bucket bar container "${config.bucketBarContainer}" not found`
+            );
+          }
+        }
+
+        // Create the background for the bucket bar and toolbar. This also
+        // serves as the default container for the bucket bar.
+        const sidebarEdge = document.createElement('div');
+        sidebarEdge.setAttribute('data-testid', 'sidebar-edge');
+        sidebarEdge.className = classnames(
+          // Position the background along the left edge of the sidebar.
+          //
+          // `width` is 1px more than `left` to avoid a gap on iOS.
+          // See https://github.com/hypothesis/client/pull/2750.
+          'absolute top-0 bottom-0 w-[23px] left-[-22px]',
+
+          // Make the bucket bar fill the container, with large padding on top
+          // so that buckets are below the toolbar, and small padding on the
+          // right to align the right edge of the buckets with the right edge
+          // of toolbar icons.
+          'flex flex-column pt-[110px] pr-[5px]',
+
+          // Use a grey background, with lower opacity with the sidebar is
+          // collapsed, so the page content behind it can be read.
+          'bg-grey-2 sidebar-collapsed:bg-black/[.08]'
+        );
+        this.iframeContainer.append(sidebarEdge);
+
+        if (!bucketBarContainer) {
+          bucketBarContainer = sidebarEdge;
+        }
+
+        this.bucketBar = new BucketBar(bucketBarContainer, {
           onFocusAnnotations: tags =>
             this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', tags)),
           onScrollToClosestOffScreenAnchor: (tags, direction) =>

src/annotator/test/bucket-bar-test.js

@@ -50,7 +50,9 @@ describe('BucketBar', () => {
   it('should render the bucket bar with no buckets when constructed', () => {
     const bucketBar = createBucketBar();
     assert.calledWith(fakeComputeBuckets, []);
-    assert.ok(bucketBar._bucketsContainer.querySelector('.FakeBuckets'));
+    assert.ok(
+      bucketBar._bucketsContainer.shadowRoot.querySelector('.FakeBuckets')
+    );
   });
 
   it('passes "onFocusAnnotations" to the Bucket component', () => {

src/annotator/test/sidebar-test.js

@@ -1053,9 +1053,12 @@ describe('Sidebar', () => {
   });
 
   describe('bucket bar', () => {
-    it('displays the bucket bar by default', () => {
+    it('displays the bucket bar alongside the sidebar by default', () => {
       const sidebar = createSidebar();
       assert.isNotNull(sidebar.bucketBar);
+      assert.calledOnce(FakeBucketBar);
+      const container = FakeBucketBar.args[0][0];
+      assert.equal(container.getAttribute('data-testid'), 'sidebar-edge');
     });
 
     it('does not display the bucket bar if using the "clean" theme', () => {
@@ -1070,6 +1073,37 @@ describe('Sidebar', () => {
       assert.isNull(sidebar.bucketBar);
     });
 
+    it('creates bucket bar in specified container if `bucketBarContainer` config is supplied', () => {
+      const bucketBarContainer = document.createElement('div');
+      bucketBarContainer.id = 'bucket-bar-container';
+      document.body.append(bucketBarContainer);
+
+      try {
+        const sidebar = createSidebar({
+          bucketBarContainer: '#bucket-bar-container',
+        });
+        assert.ok(sidebar.bucketBar);
+        assert.calledWith(FakeBucketBar, bucketBarContainer, sinon.match.any);
+      } finally {
+        bucketBarContainer.remove();
+      }
+    });
+
+    it('warns if `bucketBarContainer` config is supplied but invalid', () => {
+      sinon.stub(console, 'warn');
+      try {
+        createSidebar({
+          bucketBarContainer: '#invalid-selector',
+        });
+        assert.calledWith(
+          console.warn,
+          `Custom bucket bar container "#invalid-selector" not found`
+        );
+      } finally {
+        console.warn.restore();
+      }
+    });
+
     it('calls the "hoverAnnotations" RPC method', () => {
       const sidebar = createSidebar();
       connectGuest(sidebar);

src/annotator/util/buckets.ts

@@ -50,12 +50,6 @@ export type WorkingBucket = {
   bottom: number;
 };
 
-// Only anchors with top offsets between `BUCKET_TOP_THRESHOLD` and
-// `window.innerHeight - BUCKET_BOTTOM_THRESHOLD` are considered "on-screen"
-// and will be bucketed. This is to account for bucket-bar tool buttons (top
-// and the height of the bottom navigation bucket (bottom)
-export const BUCKET_TOP_THRESHOLD = 137;
-export const BUCKET_BOTTOM_THRESHOLD = 22;
 // Generated buckets of annotation anchor highlights should be spaced by
 // at least this amount, in pixels
 const BUCKET_GAP_SIZE = 60;
@@ -70,6 +64,11 @@ export function findClosestOffscreenAnchor(
   anchors: Anchor[],
   direction: 'up' | 'down'
 ): Anchor | null {
+  // Thresholds for how far away an anchor has to be from the top/bottom of
+  // the viewport to be considered off-screen.
+  const BUCKET_TOP_THRESHOLD = 137;
+  const BUCKET_BOTTOM_THRESHOLD = 22;
+
   let closestAnchor = null;
   let closestTop = 0;
 
@@ -143,7 +142,22 @@ export function computeAnchorPositions(anchors: Anchor[]): AnchorPosition[] {
   return positions;
 }
 
-export function computeBuckets(anchorPositions: AnchorPosition[]): BucketSet {
+/**
+ * Gap between the top/bottom of the container and the top/bottom of buckets.
+ */
+export const BUCKET_BAR_VERTICAL_MARGIN = 30;
+
+/**
+ * Group anchors into buckets and determine a suitable vertical position
+ * for them within {@link container}.
+ *
+ * @param anchorPositions - Positions of anchors relative to viewport
+ * @param container - Container into which buckets will be rendered
+ */
+export function computeBuckets(
+  anchorPositions: AnchorPosition[],
+  container: Element
+): BucketSet {
   const aboveTags = new Set<string>();
   const belowTags = new Set<string>();
   const buckets: Bucket[] = [];
@@ -165,22 +179,33 @@ export function computeBuckets(anchorPositions: AnchorPosition[]): BucketSet {
     };
   }
 
+  const containerRect = container.getBoundingClientRect();
+  const vMargin = BUCKET_BAR_VERTICAL_MARGIN;
+
+  // Compute positions of buckets relative to bucket bar instead of viewport.
+  const relativePositions = anchorPositions.map(aPos => ({
+    tag: aPos.tag,
+    top: aPos.top - containerRect.top,
+    bottom: aPos.bottom - containerRect.top,
+  }));
+
   // Build buckets from position information
-  anchorPositions.forEach(aPos => {
+  for (const aPos of relativePositions) {
     const center = (aPos.top + aPos.bottom) / 2;
-    if (center < BUCKET_TOP_THRESHOLD) {
+
+    if (center < vMargin) {
       aboveTags.add(aPos.tag);
-      return;
-    } else if (center > window.innerHeight - BUCKET_BOTTOM_THRESHOLD) {
+      continue;
+    } else if (center > containerRect.height - vMargin) {
       belowTags.add(aPos.tag);
-      return;
+      continue;
     }
 
     if (!currentBucket) {
       // We've encountered our first on-screen anchor position:
       // We'll need a bucket!
       currentBucket = newBucket(aPos);
-      return;
+      continue;
     }
     // We want to contain overlapping highlights and those near each other
     // within a shared bucket
@@ -212,7 +237,7 @@ export function computeBuckets(anchorPositions: AnchorPosition[]): BucketSet {
       currentBucket.bottom = updatedBottom;
       currentBucket.position = currentBucket.top + updatedHeight / 2;
     }
-  });
+  }
 
   if (currentBucket) {
     buckets.push(currentBucket);
@@ -221,13 +246,13 @@ export function computeBuckets(anchorPositions: AnchorPosition[]): BucketSet {
   // Add an upper "navigation" bucket with offscreen-above anchors
   const above = {
     tags: aboveTags,
-    position: BUCKET_TOP_THRESHOLD,
+    position: vMargin,
   };
 
   // Add a lower "navigation" bucket with offscreen-below anchors
   const below = {
     tags: belowTags,
-    position: window.innerHeight - BUCKET_BOTTOM_THRESHOLD,
+    position: containerRect.height - vMargin,
   };
 
   return {

src/annotator/util/test/buckets-test.js

@@ -3,8 +3,7 @@ import {
   computeAnchorPositions,
   computeBuckets,
   $imports,
-  BUCKET_TOP_THRESHOLD,
-  BUCKET_BOTTOM_THRESHOLD,
+  BUCKET_BAR_VERTICAL_MARGIN,
 } from '../buckets';
 
 const FAKE_WINDOW_HEIGHT = 410;
@@ -14,6 +13,8 @@ describe('annotator/util/buckets', () => {
   let fakeGetBoundingClientRect;
   let stubbedInnerHeight;
 
+  let bucketBarContainer;
+
   beforeEach(() => {
     // In a normal `Anchor` object, `highlights` would be an array of
     // DOM elements. Here, `highlights[0]` is the vertical offset (top) of the
@@ -48,6 +49,17 @@ describe('annotator/util/buckets', () => {
       };
     });
 
+    bucketBarContainer = document.createElement('div');
+    bucketBarContainer.style.position = 'fixed';
+
+    // Set the vertical position and width of the bucket bar to roughly match
+    // what it will be when positioned along the edge of the sidebar.
+    const topOffset = 100;
+    bucketBarContainer.style.top = `${topOffset}px`;
+    bucketBarContainer.style.height = `${FAKE_WINDOW_HEIGHT - topOffset}px`;
+    bucketBarContainer.style.width = '25px';
+    document.body.append(bucketBarContainer);
+
     $imports.$mock({
       '../highlighter': {
         getBoundingClientRect: fakeGetBoundingClientRect,
@@ -57,6 +69,7 @@ describe('annotator/util/buckets', () => {
 
   afterEach(() => {
     $imports.$restore();
+    bucketBarContainer.remove();
     stubbedInnerHeight.restore();
   });
 
@@ -203,42 +216,56 @@ describe('annotator/util/buckets', () => {
       ];
     });
 
-    it('puts anchors whose center is above the screen into the `above` bucket', () => {
-      const thresholdPos = BUCKET_TOP_THRESHOLD;
-      const bucketSet = computeBuckets([
-        ...fakeAnchorPositions,
-        {
-          tag: 'just-on-screen',
-          top: thresholdPos - 10,
-          bottom: thresholdPos + 11,
-        },
-        {
-          tag: 'just-off-screen',
-          top: thresholdPos - 11,
-          bottom: thresholdPos + 10,
-        },
-      ]);
+    it('puts anchors whose center is above the container into the `above` bucket', () => {
+      const thresholdPos =
+        bucketBarContainer.getBoundingClientRect().top +
+        BUCKET_BAR_VERTICAL_MARGIN;
+
+      const bucketSet = computeBuckets(
+        [
+          ...fakeAnchorPositions,
+          {
+            tag: 'just-on-screen',
+            top: thresholdPos - 10,
+            bottom: thresholdPos + 11,
+          },
+          {
+            tag: 'just-off-screen',
+            top: thresholdPos - 11,
+            bottom: thresholdPos + 10,
+          },
+        ],
+        bucketBarContainer
+      );
+
       assert.deepEqual(
         [...bucketSet.above.tags],
         ['t0', 't1', 'just-off-screen']
       );
     });
 
-    it('puts anchors whose center is below the screen into the `below` bucket', () => {
-      const thresholdPos = FAKE_WINDOW_HEIGHT - BUCKET_BOTTOM_THRESHOLD;
-      const bucketSet = computeBuckets([
-        ...fakeAnchorPositions,
-        {
-          tag: 'just-on-screen',
-          top: thresholdPos - 11,
-          bottom: thresholdPos + 10,
-        },
-        {
-          tag: 'just-off-screen',
-          top: thresholdPos - 10,
-          bottom: thresholdPos + 11,
-        },
-      ]);
+    it('puts anchors whose center is below the container into the `below` bucket', () => {
+      const thresholdPos =
+        bucketBarContainer.getBoundingClientRect().bottom -
+        BUCKET_BAR_VERTICAL_MARGIN;
+
+      const bucketSet = computeBuckets(
+        [
+          ...fakeAnchorPositions,
+          {
+            tag: 'just-on-screen',
+            top: thresholdPos - 11,
+            bottom: thresholdPos + 10,
+          },
+          {
+            tag: 'just-off-screen',
+            top: thresholdPos - 10,
+            bottom: thresholdPos + 11,
+          },
+        ],
+        bucketBarContainer
+      );
+
       assert.deepEqual(
         [...bucketSet.below.tags],
         ['t4', 't5', 'just-off-screen']
@@ -246,7 +273,7 @@ describe('annotator/util/buckets', () => {
     });
 
     it('puts on-screen anchors into a buckets', () => {
-      const bucketSet = computeBuckets(fakeAnchorPositions);
+      const bucketSet = computeBuckets(fakeAnchorPositions, bucketBarContainer);
       assert.deepEqual([...bucketSet.buckets[0].tags], ['t2', 't3']);
     });
 
@@ -254,7 +281,8 @@ describe('annotator/util/buckets', () => {
       fakeAnchorPositions[2].bottom = 216;
       fakeAnchorPositions[3].top = 301; // more than 60px from 216
 
-      const bucketSet = computeBuckets(fakeAnchorPositions);
+      const bucketSet = computeBuckets(fakeAnchorPositions, bucketBarContainer);
+
       assert.deepEqual([...bucketSet.buckets[0].tags], ['t2']);
       assert.deepEqual([...bucketSet.buckets[1].tags], ['t3']);
     });
@@ -262,7 +290,7 @@ describe('annotator/util/buckets', () => {
     it('puts overlapping anchors into a shared bucket', () => {
       fakeAnchorPositions[2].bottom = 401;
       fakeAnchorPositions[3].bottom = 385;
-      const bucketSet = computeBuckets(fakeAnchorPositions);
+      const bucketSet = computeBuckets(fakeAnchorPositions, bucketBarContainer);
       assert.deepEqual([...bucketSet.buckets[0].tags], ['t2', 't3']);
     });
 
@@ -271,8 +299,14 @@ describe('annotator/util/buckets', () => {
       fakeAnchorPositions[2].bottom = 250;
       fakeAnchorPositions[3].top = 225;
       fakeAnchorPositions[3].bottom = 300;
-      const bucketSet = computeBuckets(fakeAnchorPositions);
-      assert.equal(bucketSet.buckets[0].position, 250);
+
+      const bucketSet = computeBuckets(fakeAnchorPositions, bucketBarContainer);
+
+      const expected =
+        (fakeAnchorPositions[2].top + fakeAnchorPositions[3].bottom) / 2 -
+        bucketBarContainer.getBoundingClientRect().top;
+
+      assert.equal(bucketSet.buckets[0].position, expected);
     });
 
     it('returns only above- and below-screen anchors if none are on-screen', () => {
@@ -281,7 +315,9 @@ describe('annotator/util/buckets', () => {
         fakeAnchorPositions[index].top += 1000;
         fakeAnchorPositions[index].bottom += 1000;
       });
-      const bucketSet = computeBuckets(fakeAnchorPositions);
+
+      const bucketSet = computeBuckets(fakeAnchorPositions, bucketBarContainer);
+
       assert.equal(bucketSet.buckets.length, 0);
       // Above-screen
       assert.deepEqual([...bucketSet.above.tags], ['t0', 't1']);