Add PDF side-by-side mode

src/annotator/pdf-sidebar.js

 import Sidebar from './sidebar';
 
+/**
+ * @typedef {import('../types/annotator').HypothesisWindow} HypothesisWindow
+ * @typedef {import('./sidebar').LayoutState} LayoutState
+ */
+
 const defaultConfig = {
   TextSelection: {},
   PDF: {},
@@ -12,8 +17,103 @@ const defaultConfig = {
   },
 };
 
+// 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
+const MIN_PDF_WIDTH = 680;
+
 export default class PdfSidebar extends Sidebar {
   constructor(element, config) {
     super(element, { ...defaultConfig, ...config });
+
+    this._lastSidebarLayoutState = {
+      expanded: false,
+      width: 0,
+      height: 0,
+    };
+
+    this.window = /** @type {HypothesisWindow} */ (window);
+    this.pdfViewer = this.window.PDFViewerApplication?.pdfViewer;
+    this.pdfContainer = this.window.PDFViewerApplication?.appConfig?.appContainer;
+
+    // Prefer to lay out the sidebar and the PDF side-by-side (with the sidebar
+    // not overlapping the PDF) when space allows
+    this.sideBySide = !!(
+      config.enableExperimentalPDFSideBySide &&
+      this.pdfViewer &&
+      this.pdfContainer
+    );
+
+    // Is the current state of the layout side-by-side?
+    this.sideBySideActive = false;
+
+    if (this.sideBySide) {
+      this.subscribe('sidebarLayoutChanged', state =>
+        this.fitSideBySide(state)
+      );
+      this.window.addEventListener('resize', () => this.fitSideBySide());
+    }
+  }
+
+  /**
+   * Set the PDF.js container element to the designated `width` and
+   * activate side-by-side mode.
+   *
+   * @param {number} width - in pixels
+   */
+  activateSideBySide(width) {
+    this.sideBySideActive = true;
+    this.closeSidebarOnDocumentClick = false;
+    this.pdfContainer.style.width = width + 'px';
+    this.pdfContainer.classList.add('hypothesis-side-by-side');
+  }
+
+  /**
+   * Deactivate side-by-side mode and allow PDF.js pages to render at
+   * whatever width the current full-page viewport allows.
+   */
+  deactivateSideBySide() {
+    this.sideBySideActive = false;
+    this.closeSidebarOnDocumentClick = true;
+    this.pdfContainer.style.width = 'auto';
+    this.pdfContainer.classList.remove('hypothesis-side-by-side');
+  }
+
+  /**
+   * Attempt to make the PDF viewer and the sidebar fit side-by-side without
+   * overlap if there is enough room in the viewport to do so reasonably.
+   * Resize the PDF viewer container element to leave the right amount of room
+   * for the sidebar, and prompt PDF.js to re-render the PDF pages to scale
+   * within that resized container.
+   *
+   * @param {LayoutState} [sidebarLayoutState]
+   */
+  fitSideBySide(sidebarLayoutState) {
+    if (!sidebarLayoutState) {
+      sidebarLayoutState = /** @type {LayoutState} */ (this
+        ._lastSidebarLayoutState);
+    }
+    const maximumWidthToFit = this.window.innerWidth - sidebarLayoutState.width;
+    if (sidebarLayoutState.expanded && maximumWidthToFit >= MIN_PDF_WIDTH) {
+      this.activateSideBySide(maximumWidthToFit);
+    } else {
+      this.deactivateSideBySide();
+    }
+
+    // The following logic is pulled from PDF.js `webViewerResize`
+    const currentScaleValue = this.pdfViewer.currentScaleValue;
+    if (
+      currentScaleValue === 'auto' ||
+      currentScaleValue === 'page-fit' ||
+      currentScaleValue === 'page-width'
+    ) {
+      // NB: There is logic within the setter for `currentScaleValue`
+      // Setting this scale value will prompt PDF.js to recalculate viewport
+      this.pdfViewer.currentScaleValue = currentScaleValue;
+    }
+    // This will cause PDF pages to re-render if their scaling has changed
+    this.pdfViewer.update();
+
+    this._lastSidebarLayoutState = sidebarLayoutState;
   }
 }

src/annotator/sidebar.js

@@ -7,6 +7,13 @@ import features from './features';
 
 import { ToolbarController } from './toolbar';
 
+/**
+ * @typedef LayoutState
+ * @prop {boolean} expanded
+ * @prop {number} width
+ * @prop {number} height
+ */
+
 // Minimum width to which the frame can be resized.
 const MIN_RESIZE = 280;
 
@@ -202,11 +209,11 @@ export default class Sidebar extends Host {
       expanded = frameVisibleWidth > toolbarWidth;
     }
 
-    const layoutState = {
+    const layoutState = /** @type LayoutState */ ({
       expanded,
       width: expanded ? frameVisibleWidth : toolbarWidth,
       height: rect.height,
-    };
+    });
 
     if (this.onLayoutChange) {
       this.onLayoutChange(layoutState);

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

+import $ from 'jquery';
+
+import PdfSidebar from '../pdf-sidebar';
+import { $imports } from '../pdf-sidebar';
+
+describe('PdfSidebar', () => {
+  const sandbox = sinon.createSandbox();
+  let CrossFrame;
+  let fakeCrossFrame;
+
+  let fakePDFViewerApplication;
+  let fakePDFContainer;
+  let fakePDFViewerUpdate;
+  const sidebarConfig = { pluginClasses: {} };
+
+  const createPdfSidebar = config => {
+    config = { ...sidebarConfig, ...config };
+
+    const element = document.createElement('div');
+    return new PdfSidebar(element, config);
+  };
+
+  beforeEach(() => {
+    sandbox.stub(PdfSidebar.prototype, '_setupGestures');
+    fakePDFContainer = document.createElement('div');
+    fakePDFViewerUpdate = sinon.stub();
+
+    fakePDFViewerApplication = {
+      appConfig: {
+        appContainer: fakePDFContainer,
+      },
+      pdfViewer: {
+        currentScaleValue: 'auto',
+        update: fakePDFViewerUpdate,
+      },
+    };
+    // See https://github.com/sinonjs/sinon/issues/1537
+    // Can't stub an undefined property in a sandbox
+    window.PDFViewerApplication = fakePDFViewerApplication;
+
+    // From `Sidebar.js` tests
+    fakeCrossFrame = {};
+    fakeCrossFrame.onConnect = sandbox.stub().returns(fakeCrossFrame);
+    fakeCrossFrame.on = sandbox.stub().returns(fakeCrossFrame);
+    fakeCrossFrame.call = sandbox.spy();
+    fakeCrossFrame.destroy = sandbox.stub();
+
+    const fakeBucketBar = {};
+    fakeBucketBar.element = $('<div></div>');
+    fakeBucketBar.destroy = sandbox.stub();
+
+    CrossFrame = sandbox.stub();
+    CrossFrame.returns(fakeCrossFrame);
+
+    const BucketBar = sandbox.stub();
+    BucketBar.returns(fakeBucketBar);
+
+    sidebarConfig.pluginClasses.CrossFrame = CrossFrame;
+    sidebarConfig.pluginClasses.BucketBar = BucketBar;
+  });
+
+  afterEach(() => {
+    delete window.PDFViewerApplication;
+    sandbox.restore();
+    $imports.$restore();
+  });
+
+  context('side-by-side mode configured', () => {
+    it('enables side-by-side mode if config and PDF js are present', () => {
+      const sidebar = createPdfSidebar({
+        enableExperimentalPDFSideBySide: true,
+      });
+      assert.isTrue(sidebar.sideBySide);
+    });
+
+    describe('when window is resized', () => {
+      it('attempts to lay out side-by-side', () => {
+        sandbox.stub(window, 'innerWidth').value(1300);
+        const sidebar = createPdfSidebar({
+          enableExperimentalPDFSideBySide: true,
+        });
+
+        window.dispatchEvent(new Event('resize'));
+
+        // PDFSidebar.fitSideBySide is invoked with no argument, so
+        // `sidebar.lastSidebarLayoutState` is used. By default, the sidebar
+        // is not `expanded`, so side-by-side will not activate here (it only
+        // activates if sidebar is `expanded` in its layout state)
+        assert.isFalse(sidebar.sideBySideActive);
+        // However, the PDF container is always updated on a resize
+        assert.calledOnce(fakePDFViewerUpdate);
+      });
+
+      it('resizes and activates side-by-side mode', () => {
+        sandbox.stub(window, 'innerWidth').value(1300);
+        const sidebar = createPdfSidebar({
+          enableExperimentalPDFSideBySide: true,
+        });
+        sidebar._lastSidebarLayoutState = {
+          expanded: true,
+          width: 428,
+          height: 800,
+        };
+
+        window.dispatchEvent(new Event('resize'));
+
+        // Since `sidebar._lastSidebarLayoutState` has `expanded: true`,
+        // side-by-side mode can be activated if there is enough room...
+        assert.isTrue(sidebar.sideBySideActive);
+        assert.calledOnce(fakePDFViewerUpdate);
+        assert.equal(fakePDFContainer.style.width, 1300 - 428 + 'px');
+      });
+
+      it('does not activate side-by-side mode if there is not enough room', () => {
+        sandbox.stub(window, 'innerWidth').value(800);
+        const sidebar = createPdfSidebar({
+          enableExperimentalPDFSideBySide: true,
+        });
+        sidebar._lastSidebarLayoutState = {
+          expanded: true,
+          width: 428,
+          height: 800,
+        };
+
+        window.dispatchEvent(new Event('resize'));
+
+        // Since `sidebar._lastSidebarLayoutState` has `expanded: true`,
+        // side-by-side mode can be activated if there is enough room...
+        assert.isFalse(sidebar.sideBySideActive);
+        assert.calledOnce(fakePDFViewerUpdate);
+        assert.equal(fakePDFContainer.style.width, 'auto');
+      });
+    });
+
+    describe('when sidebar layout state changes', () => {
+      it('resizes and activates side-by-side mode when sidebar expanded', () => {
+        sandbox.stub(window, 'innerWidth').value(1350);
+        const sidebar = createPdfSidebar({
+          enableExperimentalPDFSideBySide: true,
+        });
+
+        sidebar.publish('sidebarLayoutChanged', [
+          { expanded: true, width: 428, height: 728 },
+        ]);
+
+        assert.isTrue(sidebar.sideBySideActive);
+        assert.calledOnce(fakePDFViewerUpdate);
+        assert.equal(fakePDFContainer.style.width, 1350 - 428 + 'px');
+      });
+
+      /**
+       * For each of the relative zoom modes supported by PDF.js, PDFSidebar
+       * should re-set the `currentScale` value, which will prompt PDF.js
+       * to re-calculate the zoom/viewport. Then, `pdfViewer.update()` will
+       * re-render the PDF pages as needed for the dirtied viewport/scaling.
+       * These tests are primarily for test coverage of these zoom modes.
+       */
+      ['auto', 'page-fit', 'page-width'].forEach(zoomMode => {
+        it('activates side-by-side mode for each relative zoom mode', () => {
+          fakePDFViewerApplication.pdfViewer.currentScaleValue = zoomMode;
+          sandbox.stub(window, 'innerWidth').value(1350);
+          const sidebar = createPdfSidebar({
+            enableExperimentalPDFSideBySide: true,
+          });
+
+          sidebar.publish('sidebarLayoutChanged', [
+            { expanded: true, width: 428, height: 728 },
+          ]);
+
+          assert.isTrue(sidebar.sideBySideActive);
+          assert.calledOnce(fakePDFViewerUpdate);
+          assert.equal(fakePDFContainer.style.width, 1350 - 428 + 'px');
+        });
+      });
+
+      it('deactivates side-by-side mode when sidebar collapsed', () => {
+        sandbox.stub(window, 'innerWidth').value(1350);
+        const sidebar = createPdfSidebar({
+          enableExperimentalPDFSideBySide: true,
+        });
+
+        sidebar.publish('sidebarLayoutChanged', [
+          { expanded: false, width: 428, height: 728 },
+        ]);
+
+        assert.isFalse(sidebar.sideBySideActive);
+        assert.equal(fakePDFContainer.style.width, 'auto');
+      });
+
+      it('does not activate side-by-side mode if there is not enough room', () => {
+        sandbox.stub(window, 'innerWidth').value(800);
+        const sidebar = createPdfSidebar({
+          enableExperimentalPDFSideBySide: true,
+        });
+
+        sidebar.publish('sidebarLayoutChanged', [
+          { expanded: true, width: 428, height: 728 },
+        ]);
+
+        assert.isFalse(sidebar.sideBySideActive);
+        assert.calledOnce(fakePDFViewerUpdate);
+        assert.equal(fakePDFContainer.style.width, 'auto');
+      });
+    });
+  });
+
+  context('side-by-side mode not configured', () => {
+    it('does not enable side-by-side mode', () => {
+      const sidebar = createPdfSidebar({});
+
+      assert.isFalse(sidebar.sideBySide);
+    });
+
+    it('does not attempt to resize PDF container on window resize', () => {
+      const sidebar = createPdfSidebar({});
+
+      window.dispatchEvent(new Event('resize'));
+
+      assert.isFalse(sidebar.sideBySideActive);
+      assert.notCalled(fakePDFViewerUpdate);
+    });
+
+    it('does not attempt to resize PDF container on sidebar layout change', () => {
+      const sidebar = createPdfSidebar({});
+
+      sidebar.publish('sidebarLayoutChanged', [
+        { expanded: true, width: 428, height: 728 },
+      ]);
+
+      assert.isFalse(sidebar.sideBySideActive);
+      assert.notCalled(fakePDFViewerUpdate);
+    });
+  });
+});

src/styles/annotator/annotator.scss

@@ -78,6 +78,7 @@ $sidebar-collapse-transition-time: 150ms;
     z-index: 2;
   }
 
+  // FIXME:  Use variables for sizing here
   .annotator-frame-button {
     @include focus.outline-on-keyboard-focus;
 

src/styles/annotator/pdfjs-overrides.scss

@@ -15,3 +15,15 @@
 .textLayer .highlight.selected {
   background-color: rgba(0, 100, 0, 0.5);
 }
+
+// Make sure sidebar bucket bar/toolbar don't obscure PDF JS tools menu
+// This gives enough room for `.annotator-frame-button` which uses a hard-coded
+// width of 30px
+#toolbarViewer {
+  margin-right: 30px;
+}
+
+// The affordance is not needed in side-by-side mode
+.hypothesis-side-by-side #toolbarViewer {
+  margin-right: 0;
+}