Change `BucketBar` to support only a single content container element

In current usage of the `BucketBar` we only ever have a single scrollable
container to observe. Simplify the `BucketBar` API to pass in this element directly
rather than a selector and add associated tests.

src/annotator/bucket-bar.js

@@ -5,7 +5,8 @@ import { anchorBuckets } from './util/buckets';
 
 /**
  * @typedef BucketBarOptions
- * @prop {string[]} [scrollables] - Selectors for the scrollable elements on the page
+ * @prop {Element} [contentContainer] - The scrollable container element for the
+ *   highlights that the bucket bar's buckets will point at
  */
 
 export default class BucketBar {
@@ -14,8 +15,8 @@ export default class BucketBar {
    * @param {Pick<import('./guest').default, 'anchors'|'selectAnnotations'>} guest
    * @param {BucketBarOptions} [options]
    */
-  constructor(container, guest, options = {}) {
-    this.options = options;
+  constructor(container, guest, { contentContainer = document.body } = {}) {
+    this._contentContainer = contentContainer;
     this.element = document.createElement('div');
 
     this.guest = guest;
@@ -25,12 +26,7 @@ export default class BucketBar {
 
     window.addEventListener('resize', this.updateFunc);
     window.addEventListener('scroll', this.updateFunc);
-    this.options.scrollables?.forEach(scrollable => {
-      const scrollableElement = /** @type {HTMLElement | null} */ (document.querySelector(
-        scrollable
-      ));
-      scrollableElement?.addEventListener('scroll', this.updateFunc);
-    });
+    contentContainer.addEventListener('scroll', this.updateFunc);
 
     // Immediately render the buckets for the current anchors.
     this._update();
@@ -39,12 +35,7 @@ export default class BucketBar {
   destroy() {
     window.removeEventListener('resize', this.updateFunc);
     window.removeEventListener('scroll', this.updateFunc);
-    this.options.scrollables?.forEach(scrollable => {
-      const scrollableElement = /** @type {HTMLElement | null} */ (document.querySelector(
-        scrollable
-      ));
-      scrollableElement?.removeEventListener('scroll', this.updateFunc);
-    });
+    this._contentContainer.removeEventListener('scroll', this.updateFunc);
   }
 
   update() {

src/annotator/pdf-sidebar.js

@@ -6,12 +6,6 @@ import Sidebar from './sidebar';
  * @typedef {import('./sidebar').LayoutState} LayoutState
  */
 
-const defaultConfig = {
-  BucketBar: {
-    scrollables: ['#viewerContainer'],
-  },
-};
-
 // The viewport and controls for PDF.js start breaking down below about 670px
 // of available space, so only render PDF and sidebar side-by-side if there
 // is enough room. Otherwise, allow sidebar to overlap PDF
@@ -24,7 +18,9 @@ export default class PdfSidebar extends Sidebar {
    * @param {Record<string, any>} [config]
    */
   constructor(element, guest, config = {}) {
-    super(element, guest, { ...defaultConfig, ...config });
+    const contentContainer = document.querySelector('#viewerContainer');
+
+    super(element, guest, { contentContainer, ...config });
 
     this._lastSidebarLayoutState = {
       expanded: false,

src/annotator/sidebar.js

@@ -89,11 +89,9 @@ export default class Sidebar extends Delegator {
       if (config.theme === 'clean') {
         this.iframeContainer.classList.add('annotator-frame--theme-clean');
       } else {
-        const bucketBar = new BucketBar(
-          this.iframeContainer,
-          guest,
-          config.BucketBar
-        );
+        const bucketBar = new BucketBar(this.iframeContainer, guest, {
+          contentContainer: config.contentContainer || document.body,
+        });
         guest.subscribe('anchorsChanged', () => bucketBar.update());
         this.bucketBar = bucketBar;
       }

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

@@ -80,30 +80,37 @@ describe('BucketBar', () => {
       assert.calledWith(fakeAnnotator.selectAnnotations, fakeAnnotations, true);
     });
 
-    context('when scrollables provided', () => {
-      let scrollableEls = [];
+    context('when `contentContainer` is specified', () => {
+      let container;
 
       beforeEach(() => {
-        const scrollableEls1 = document.createElement('div');
-        scrollableEls1.className = 'scrollable-1';
-        const scrollableEls2 = document.createElement('div');
-        scrollableEls2.className = 'scrollable-2';
-        scrollableEls.push(scrollableEls1, scrollableEls2);
-        document.body.appendChild(scrollableEls1);
-        document.body.appendChild(scrollableEls2);
+        container = document.createElement('div');
+        container.className = 'scrollable-1';
+        document.body.appendChild(container);
       });
+
       afterEach(() => {
-        scrollableEls.forEach(el => el.remove());
+        container.remove();
       });
+
       it('should update buckets when any scrollable scrolls', () => {
-        createBucketBar({
-          scrollables: ['.scrollable-1', '.scrollable-2'],
+        createBucketBar({ contentContainer: container });
+        fakeBucketUtil.anchorBuckets.resetHistory();
+
+        container.dispatchEvent(new Event('scroll'));
+
+        assert.calledOnce(fakeBucketUtil.anchorBuckets);
+      });
     });
+
+    context('when no `contentContainer` is specified', () => {
+      it('should update buckets when body is scrolled', () => {
+        createBucketBar();
         fakeBucketUtil.anchorBuckets.resetHistory();
-        scrollableEls[0].dispatchEvent(new Event('scroll'));
+
+        document.body.dispatchEvent(new Event('scroll'));
+
         assert.calledOnce(fakeBucketUtil.anchorBuckets);
-        scrollableEls[1].dispatchEvent(new Event('scroll'));
-        assert.calledTwice(fakeBucketUtil.anchorBuckets);
       });
     });
 
@@ -114,4 +121,18 @@ describe('BucketBar', () => {
       assert.notCalled(window.requestAnimationFrame);
     });
   });
+
+  it('should stop listening for scroll events when destroyed', () => {
+    const container = document.createElement('div');
+    const bucketBar = createBucketBar({ contentContainer: container });
+    fakeBucketUtil.anchorBuckets.resetHistory();
+
+    bucketBar.destroy();
+
+    container.dispatchEvent(new Event('scroll'));
+    window.dispatchEvent(new Event('resize'));
+    window.dispatchEvent(new Event('scroll'));
+
+    assert.notCalled(fakeBucketUtil.anchorBuckets);
+  });
 });

src/annotator/test/pdf-sidebar-test.js

@@ -59,6 +59,17 @@ describe('PdfSidebar', () => {
     unmockSidebar();
   });
 
+  it(`sets the content container for use by the BucketBar`, () => {
+    const container = document.createElement('div');
+    container.id = 'viewerContainer';
+    document.body.appendChild(container);
+
+    const sidebar = createPdfSidebar();
+    assert.equal(sidebar.options.contentContainer, container);
+
+    container.remove();
+  });
+
   context('side-by-side mode configured', () => {
     describe('when window is resized', () => {
       it('attempts to lay out side-by-side', () => {

src/annotator/test/sidebar-test.js

@@ -17,8 +17,10 @@ describe('Sidebar', () => {
   let containers;
   let sidebars;
 
-  let FakeToolbarController;
+  let FakeBucketBar;
   let fakeBucketBar;
+
+  let FakeToolbarController;
   let fakeToolbar;
 
   before(() => {
@@ -93,7 +95,7 @@ describe('Sidebar', () => {
       destroy: sinon.stub(),
       update: sinon.stub(),
     };
-    const BucketBar = sandbox.stub().returns(fakeBucketBar);
+    FakeBucketBar = sandbox.stub().returns(fakeBucketBar);
 
     sidebars = [];
 
@@ -101,7 +103,7 @@ describe('Sidebar', () => {
       './toolbar': {
         ToolbarController: FakeToolbarController,
       },
-      './bucket-bar': { default: BucketBar },
+      './bucket-bar': { default: FakeBucketBar },
     });
   });
 
@@ -824,7 +826,7 @@ describe('Sidebar', () => {
     });
   });
 
-  describe('bucket bar state', () => {
+  describe('bucket bar', () => {
     it('displays the bucket bar by default', () => {
       const sidebar = createSidebar();
       assert.isNotNull(sidebar.bucketBar);
@@ -842,6 +844,27 @@ describe('Sidebar', () => {
       assert.isNull(sidebar.bucketBar);
     });
 
+    it('configures bucket bar to observe `contentContainer` scrolling if specified', () => {
+      const contentContainer = document.createElement('div');
+      const sidebar = createSidebar({ contentContainer });
+      assert.calledWith(
+        FakeBucketBar,
+        sidebar.iframeContainer,
+        fakeGuest,
+        sinon.match({ contentContainer })
+      );
+    });
+
+    it('configures bucket bar to observe body scrolling if no `contentContainer` is specified', () => {
+      const sidebar = createSidebar();
+      assert.calledWith(
+        FakeBucketBar,
+        sidebar.iframeContainer,
+        fakeGuest,
+        sinon.match({ contentContainer: document.body })
+      );
+    });
+
     it('updates the bucket bar when an `anchorsChanged` event is received', () => {
       const sidebar = createSidebar();