Cleanup and reorganise code in sidebar

- Remove `disableShadowSidebar` flag
- Simplify code structure
- Remove iframe from externalContainer on `destroy`.

Closes #2908

src/annotator/sidebar.js

@@ -25,7 +25,7 @@ import BucketBar from './bucket-bar';
  * @prop {(event: any) => void} listener
  */
 
-// Minimum width to which the frame can be resized.
+// Minimum width to which the iframeContainer can be resized.
 export const MIN_RESIZE = 280;
 
 /**
@@ -63,52 +63,54 @@ export default class Sidebar extends Guest {
   constructor(element, config) {
     super(element, config);
 
-    let externalFrame;
-    let frame;
-    let hypothesisSidebar; // refers to <hypothesis-sidebar> element
-
-    if (config.externalContainerSelector) {
-      externalFrame =
-        document.querySelector(config.externalContainerSelector) || element;
+    this.iframe = createSidebarIframe(config);
+
+    const {
+      externalContainerSelector,
+      theme,
+      openSidebar,
+      annotations,
+      query,
+      group,
+    } = config;
+
+    if (externalContainerSelector) {
+      this.externalFrame =
+        /** @type {HTMLElement} */
+        (document.querySelector(externalContainerSelector)) ?? element;
+      this.externalFrame.appendChild(this.iframe);
     } else {
-      frame = document.createElement('div');
-      frame.style.display = 'none';
-      frame.className = 'annotator-frame';
+      this.iframeContainer = document.createElement('div');
+      this.iframeContainer.style.display = 'none';
+      this.iframeContainer.className = 'annotator-frame';
 
-      if (config.theme === 'clean') {
-        frame.classList.add('annotator-frame--theme-clean');
+      if (theme === 'clean') {
+        this.iframeContainer.classList.add('annotator-frame--theme-clean');
       } else {
-        this.bucketBar = new BucketBar(frame, this, config.BucketBar);
+        this.bucketBar = new BucketBar(
+          this.iframeContainer,
+          this,
+          config.BucketBar
+        );
       }
 
-      // Undocumented switch to enable/disable the wrapping of the sidebar inside a shadow DOM
-      // 2021-01-22: remove this switch after the 2021-02-05
-      if (config.disableShadowSidebar) {
-        element.appendChild(frame);
-      } else {
-        // Wrap up the 'frame' element into a shadow DOM so it is not affected by host CSS styles
-        hypothesisSidebar = document.createElement('hypothesis-sidebar');
-        const shadowDom = createShadowRoot(hypothesisSidebar);
-        shadowDom.appendChild(frame);
+      this.iframeContainer.appendChild(this.iframe);
 
-        element.appendChild(hypothesisSidebar);
-      }
-    }
+      // Wrap up the 'iframeContainer' element into a shadow DOM so it is not affected by host CSS styles
+      this.hypothesisSidebar = document.createElement('hypothesis-sidebar');
+      const shadowDom = createShadowRoot(this.hypothesisSidebar);
+      shadowDom.appendChild(this.iframeContainer);
 
-    const sidebarFrame = createSidebarIframe(config);
-    (frame || externalFrame).appendChild(sidebarFrame);
+      element.appendChild(this.hypothesisSidebar);
+    }
 
     /** @type {RegisteredListener[]} */
     this.registeredListeners = [];
 
-    this.externalFrame = externalFrame;
-    this.frame = frame;
-    this.hypothesisSidebar = hypothesisSidebar;
-
     this.subscribe('panelReady', () => {
       // Show the UI
-      if (this.frame) {
-        this.frame.style.display = '';
+      if (this.iframeContainer) {
+        this.iframeContainer.style.display = '';
       }
     });
 
@@ -117,16 +119,11 @@ export default class Sidebar extends Guest {
       // the sidebar so that the text editor can be focused as
       // soon as the annotation card appears
       if (!annotation.$highlight) {
-        /** @type {Window} */ (sidebarFrame.contentWindow).focus();
+        /** @type {Window} */ (this.iframe.contentWindow).focus();
       }
     });
 
-    if (
-      config.openSidebar ||
-      config.annotations ||
-      config.query ||
-      config.group
-    ) {
+    if (openSidebar || annotations || query || group) {
       this.subscribe('panelReady', () => this.show());
     }
 
@@ -137,11 +134,11 @@ export default class Sidebar extends Guest {
       setSidebarOpen: open => (open ? this.show() : this.hide()),
       setHighlightsVisible: show => this.setAllVisibleHighlights(show),
     });
-    this.toolbar.useMinimalControls = config.theme === 'clean';
+    this.toolbar.useMinimalControls = theme === 'clean';
 
-    if (this.frame) {
+    if (this.iframeContainer) {
       // If using our own container frame for the sidebar, add the toolbar to it.
-      this.frame.prepend(toolbarContainer);
+      this.iframeContainer.prepend(toolbarContainer);
       this.toolbarWidth = this.toolbar.getWidth();
     } else {
       // If using a host-page provided container for the sidebar, the toolbar is
@@ -181,8 +178,11 @@ export default class Sidebar extends Guest {
   destroy() {
     this._unregisterEvents();
     this._hammerManager?.destroy();
-    this.frame?.remove();
-    this.hypothesisSidebar?.remove();
+    if (this.hypothesisSidebar) {
+      this.hypothesisSidebar.remove();
+    } else {
+      this.iframe.remove();
+    }
     super.destroy();
   }
 
@@ -270,13 +270,13 @@ export default class Sidebar extends Guest {
 
       if (
         this._gestureState.final !== this._gestureState.initial &&
-        this.frame
+        this.iframeContainer
       ) {
         const margin = /** @type {number} */ (this._gestureState.final);
         const width = -margin;
-        this.frame.style.marginLeft = `${margin}px`;
+        this.iframeContainer.style.marginLeft = `${margin}px`;
         if (width >= MIN_RESIZE) {
-          this.frame.style.width = `${width}px`;
+          this.iframeContainer.style.width = `${width}px`;
         }
         this._notifyOfLayoutChange();
       }
@@ -300,8 +300,9 @@ export default class Sidebar extends Guest {
     // The sidebar iframe is hidden or shown by adjusting the left margin of
     // its container.
 
-    const toolbarWidth = (this.frame && this.toolbar.getWidth()) || 0;
-    const frame = this.frame || this.externalFrame;
+    const toolbarWidth = (this.iframeContainer && this.toolbar.getWidth()) || 0;
+    const frame = /** @type {HTMLElement} */ (this.iframeContainer ??
+      this.externalFrame);
     const rect = frame.getBoundingClientRect();
     const computedStyle = window.getComputedStyle(frame);
     const width = parseInt(computedStyle.width);
@@ -353,7 +354,7 @@ export default class Sidebar extends Guest {
   }
 
   _onPan(event) {
-    const frame = this.frame;
+    const frame = this.iframeContainer;
     if (!frame) {
       return;
     }
@@ -409,10 +410,10 @@ export default class Sidebar extends Guest {
     this.crossframe.call('sidebarOpened');
     this.publish('sidebarOpened');
 
-    if (this.frame) {
-      const width = this.frame.getBoundingClientRect().width;
-      this.frame.style.marginLeft = `${-1 * width}px`;
-      this.frame.classList.remove('annotator-collapsed');
+    if (this.iframeContainer) {
+      const width = this.iframeContainer.getBoundingClientRect().width;
+      this.iframeContainer.style.marginLeft = `${-1 * width}px`;
+      this.iframeContainer.classList.remove('annotator-collapsed');
     }
 
     this.toolbar.sidebarOpen = true;
@@ -425,9 +426,9 @@ export default class Sidebar extends Guest {
   }
 
   hide() {
-    if (this.frame) {
-      this.frame.style.marginLeft = '';
-      this.frame.classList.add('annotator-collapsed');
+    if (this.iframeContainer) {
+      this.iframeContainer.style.marginLeft = '';
+      this.iframeContainer.classList.add('annotator-collapsed');
     }
 
     this.toolbar.sidebarOpen = false;

src/annotator/test/sidebar-test.js

@@ -117,25 +117,27 @@ describe('Sidebar', () => {
 
     it('starts hidden', () => {
       const sidebar = createSidebar();
-      assert.equal(sidebar.frame.style.display, 'none');
+      assert.equal(sidebar.iframeContainer.style.display, 'none');
     });
 
     it('applies a style if theme is configured as "clean"', () => {
       const sidebar = createSidebar({ theme: 'clean' });
       assert.isTrue(
-        sidebar.frame.classList.contains('annotator-frame--theme-clean')
+        sidebar.iframeContainer.classList.contains(
+          'annotator-frame--theme-clean'
+        )
       );
     });
 
     it('becomes visible when the "panelReady" event fires', () => {
       const sidebar = createSidebar();
       sidebar.publish('panelReady');
-      assert.equal(sidebar.frame.style.display, '');
+      assert.equal(sidebar.iframeContainer.style.display, '');
     });
   });
 
   function getConfigString(sidebar) {
-    return sidebar.frame.querySelector('iframe').src;
+    return sidebar.iframe.src;
   }
 
   function configFragment(config) {
@@ -156,7 +158,7 @@ describe('Sidebar', () => {
 
   context('when a new annotation is created', () => {
     function stubIframeWindow(sidebar) {
-      const iframe = sidebar.frame.querySelector('iframe');
+      const iframe = sidebar.iframe;
       const fakeIframeWindow = { focus: sinon.stub() };
       sinon.stub(iframe, 'contentWindow').get(() => fakeIframeWindow);
       return iframe;
@@ -397,9 +399,9 @@ describe('Sidebar', () => {
         sidebar._onPan({ type: 'panstart' });
 
         assert.isTrue(
-          sidebar.frame.classList.contains('annotator-no-transition')
+          sidebar.iframeContainer.classList.contains('annotator-no-transition')
         );
-        assert.equal(sidebar.frame.style.pointerEvents, 'none');
+        assert.equal(sidebar.iframeContainer.style.pointerEvents, 'none');
       });
 
       it('captures the left margin as the gesture initial state', () => {
@@ -416,9 +418,9 @@ describe('Sidebar', () => {
         sidebar._gestureState = { final: 0 };
         sidebar._onPan({ type: 'panend' });
         assert.isFalse(
-          sidebar.frame.classList.contains('annotator-no-transition')
+          sidebar.iframeContainer.classList.contains('annotator-no-transition')
         );
-        assert.equal(sidebar.frame.style.pointerEvents, '');
+        assert.equal(sidebar.iframeContainer.style.pointerEvents, '');
       });
 
       it('calls `show` if the widget is fully visible', () => {
@@ -500,7 +502,7 @@ describe('Sidebar', () => {
       sidebar.destroy();
       assert.called(fakeCrossFrame.destroy);
       assert.notExists(sidebarContainer.querySelector('hypothesis-sidebar'));
-      assert.equal(sidebar.frame.parentElement, null);
+      assert.equal(sidebar.iframeContainer.parentElement, null);
     });
 
     it('the (non-shadow DOMed) sidebar is destroyed and the frame is detached', () => {
@@ -509,7 +511,7 @@ describe('Sidebar', () => {
       sidebar.destroy();
       assert.called(fakeCrossFrame.destroy);
       assert.notExists(sidebarContainer.querySelector('.annotator-frame'));
-      assert.equal(sidebar.frame.parentElement, null);
+      assert.equal(sidebar.iframeContainer.parentElement, null);
     });
   });
 
@@ -651,7 +653,7 @@ describe('Sidebar', () => {
         // remove info about call that happens on creation of sidebar
         layoutChangeHandlerSpy.reset();
 
-        frame = sidebar.frame;
+        frame = sidebar.iframeContainer;
         Object.assign(frame.style, {
           display: 'block',
           width: DEFAULT_WIDTH + 'px',
@@ -759,6 +761,21 @@ describe('Sidebar', () => {
           width: 0,
         });
       });
+
+      it('removes the iframe from the container when destroyed', () => {
+        sidebar.show();
+        assert.exists(sidebar.iframe.parentElement);
+        sidebar.destroy();
+        assert.notExists(sidebar.iframe.parentElement);
+      });
+
+      it('ignores pan events', () => {
+        sandbox
+          .stub(window, 'getComputedStyle')
+          .returns({ marginLeft: '100px' });
+        sidebar._onPan({ type: 'panstart' });
+        assert.isNull(sidebar._gestureState.initial);
+      });
     });
   });
 
@@ -779,7 +796,7 @@ describe('Sidebar', () => {
     });
 
     it('uses the configured external container as the frame', () => {
-      assert.equal(sidebar.frame, undefined);
+      assert.equal(sidebar.iframeContainer, undefined);
       assert.isDefined(sidebar.externalFrame);
       assert.equal(sidebar.externalFrame, externalFrame);
       assert.equal(externalFrame.childNodes.length, 1);
@@ -803,16 +820,5 @@ describe('Sidebar', () => {
       });
       assert.isNull(sidebar.bucketBar);
     });
-
-    it('disables shadow DOM if `disableShadowSidebar` flag is set', () => {
-      createSidebar({
-        disableShadowSidebar: true,
-      });
-      const sidebarContainer = containers[0];
-      assert.notExists(sidebarContainer.querySelector('hypothesis-sidebar'));
-      const sidebar = sidebarContainer.querySelector('.annotator-frame');
-      assert.exists(sidebar);
-      assert.notExists(sidebar.shadowRoot);
-    });
   });
 });