Refactor side-by-side state in Guest

In issue discussions, tests and various parts of the code there are
mentions of side-by-side mode being active or not. The `Guest` class
didn't store this state directly but instead had a
`closeSidebarOnDocumentClick` state which is always the inverse of
"is side-by-side active?".

Make the code easier to follow by storing the "is side-by-side active?" state
directly. This will also avoid a misnamed variable if in future the
state has other effects besides changing whether clicking on the
document closes the sidebar.

 - Replace `closeSidebarOnDocumentClick` with a private
   `_sideBySideActive` property, exposed via a read-only
   `sideBySideActive` getter

 - Refactor tests to only use the public API of Guest (calling
   `Guest#fitSideBySide` or reading `sideBySideActive`)

src/annotator/guest.js

@@ -162,8 +162,7 @@ export default class Guest {
     });
     this.crossframe.onConnect(() => this._setupInitialState(config));
 
-    // Whether clicks on non-highlighted text should close the sidebar
-    this.closeSidebarOnDocumentClick = true;
+    this._sideBySideActive = false;
     this._connectAnnotationSync();
     this._connectAnnotationUISync(this.crossframe);
 
@@ -179,7 +178,7 @@ export default class Guest {
     // the document content.
     /** @param {Element} element */
     const maybeCloseSidebar = element => {
-      if (!this.closeSidebarOnDocumentClick) {
+      if (this._sideBySideActive) {
         // Don't hide the sidebar if event was disabled because the sidebar
         // doesn't overlap the content.
         return;
@@ -605,7 +604,17 @@ export default class Guest {
    * @param {SidebarLayout} sidebarLayout
    */
   fitSideBySide(sidebarLayout) {
-    const active = this.integration.fitSideBySide(sidebarLayout);
-    this.closeSidebarOnDocumentClick = !active;
+    this._sideBySideActive = this.integration.fitSideBySide(sidebarLayout);
+  }
+
+  /**
+   * Return true if side-by-side mode is currently active.
+   *
+   * Side-by-side mode is activated or de-activated when `fitSideBySide` is called
+   * depending on whether the sidebar is expanded and whether there is room for
+   * the content alongside the sidebar.
+   */
+  get sideBySideActive() {
+    return this._sideBySideActive;
   }
 }

src/annotator/test/guest-test.js

@@ -446,10 +446,14 @@ describe('Guest', () => {
       }
     });
 
-    it('does not hide sidebar if configured not to close sidebar', () => {
+    it('does not hide sidebar if side-by-side mode is active', () => {
       for (let event of ['mousedown', 'touchstart']) {
-        guest.closeSidebarOnDocumentClick = false;
+        // Activate side-by-side mode
+        fakeHTMLIntegration.fitSideBySide.returns(true);
+        guest.fitSideBySide({ expanded: true, width: 100 });
+
         rootElement.dispatchEvent(new Event(event));
+
         assert.notCalled(guest.crossframe.call);
         guest.crossframe.call.resetHistory();
       }
@@ -460,6 +464,7 @@ describe('Guest', () => {
       sandbox.stub(guest, '_onSelection'); // Calling _onSelect makes the adder to reposition
 
       window.dispatchEvent(new Event('resize'));
+
       assert.called(guest._repositionAdder);
       assert.notCalled(guest._onSelection);
     });
@@ -470,7 +475,9 @@ describe('Guest', () => {
 
       guest._isAdderVisible = true;
       sandbox.stub(window, 'getSelection').returns({ getRangeAt: () => true });
+
       window.dispatchEvent(new Event('resize'));
+
       assert.called(guest._onSelection);
     });
   });
@@ -1025,11 +1032,11 @@ describe('Guest', () => {
           const layout = { expanded: true, width: 100 };
 
           guest.fitSideBySide(layout);
-          assert.isTrue(guest.closeSidebarOnDocumentClick);
+          assert.isFalse(guest.sideBySideActive);
 
           getIntegration().fitSideBySide.returns(true);
           guest.fitSideBySide(layout);
-          assert.isFalse(guest.closeSidebarOnDocumentClick);
+          assert.isTrue(guest.sideBySideActive);
         });
       });
     });