Revert "Added close button to the notebook"

This reverts commit e19983b0.

src/annotator/notebook.js

@@ -3,9 +3,6 @@ import { createSidebarConfig } from './config/sidebar';
 import { createShadowRoot } from './util/shadow-root';
 import { render } from 'preact';
 
-// FIXME: use the button from the frontend shared package once this is stable.
-import Button from '../sidebar/components/Button';
-
 /**
  * Create the iframe that will load the notebook application.
  *
@@ -36,9 +33,7 @@ export default class Notebook extends Delegator {
     super(element, config);
     this.frame = null;
 
-    /** @type {null|string} */
     this._groupId = null;
-    /** @type {null|string} */
     this._prevGroupId = null;
 
     /**
@@ -61,7 +56,8 @@ export default class Notebook extends Delegator {
       this._groupId = groupId;
       this.open();
     });
-    // Defensive programming: if the sidebar has opened, get out of the way
+    this.subscribe('closeNotebook', () => this.close());
+    // If the sidebar has opened, get out of the way
     this.subscribe('sidebarOpened', () => this.close());
   }
 
@@ -110,25 +106,6 @@ export default class Notebook extends Delegator {
     this.container.className = 'notebook-outer';
     shadowRoot.appendChild(this.container);
 
-    render(
-      <div className="Notebook__controller-bar">
-        <Button
-          icon="cancel"
-          className="Notebook__close-button"
-          buttonText="Close"
-          title="Close the Notebook"
-          onClick={event => {
-            // Guest 'component' captures all click events in the host page and opens the sidebar.
-            // We stop the propagation of the event to prevent the sidebar to be opened.
-            event.stopPropagation();
-            this.close();
-            this.publish('closeNotebook');
-          }}
-        />
-      </div>,
-      this.container
-    );
-
     return this.container;
   }
 }

src/annotator/sidebar.js

@@ -213,12 +213,13 @@ export default class Sidebar extends Guest {
 
     // Re-publish the crossframe event so that anything extending Delegator
     // can subscribe to it (without need for crossframe)
-    this.crossframe.on('openNotebook', (/** @type {string} */ groupId) => {
-      this.hide();
+    this.crossframe.on('openNotebook', groupId => {
+      this.close();
       this.publish('openNotebook', [groupId]);
     });
-    this.subscribe('closeNotebook', () => {
-      this.show();
+    this.crossframe.on('closeNotebook', () => {
+      this.open();
+      this.publish('closeNotebook');
     });
 
     const eventHandlers = [
@@ -446,22 +447,4 @@ export default class Sidebar extends Guest {
   setAllVisibleHighlights(shouldShowHighlights) {
     this.crossframe.call('setVisibleHighlights', shouldShowHighlights);
   }
-
-  /**
-   * Shows the sidebar's controls
-   */
-  show() {
-    if (this.frame) {
-      this.frame.classList.remove('is-hidden');
-    }
-  }
-
-  /**
-   * Hides the sidebar's controls
-   */
-  hide() {
-    if (this.frame) {
-      this.frame.classList.add('is-hidden');
-    }
-  }
 }

src/annotator/test/notebook-test.js

@@ -114,27 +114,22 @@ describe('Notebook', () => {
     });
   });
 
-  describe('responding to user input', () => {
-    it('closes the notebook when close button clicked', () => {
+  describe('responding to events', () => {
+    it('opens on `openNotebook`', () => {
       const notebook = createNotebook();
 
-      notebook.open();
+      notebook.publish('openNotebook');
 
-      const button = notebook.container.getElementsByClassName(
-        'Notebook__close-button'
-      )[0];
-      button.click();
-      assert.equal(notebook.container.style.display, 'none');
-    });
+      assert.equal(notebook.container.style.display, '');
     });
 
-  describe('responding to events', () => {
-    it('opens on `openNotebook`', () => {
+    it('closes on `closeNotebook`', () => {
       const notebook = createNotebook();
 
-      notebook.publish('openNotebook');
+      notebook.open();
+      notebook.publish('closeNotebook');
 
-      assert.equal(notebook.container.style.display, '');
+      assert.equal(notebook.container.style.display, 'none');
     });
 
     it('closes on "sidebarOpened"', () => {

src/annotator/test/sidebar-test.js

@@ -251,29 +251,29 @@ describe('Sidebar', () => {
         assert.called(target);
       }));
 
-    describe('on "openNotebook" crossframe event', () => {
-      it('hides the sidebar', () => {
+    describe('on "openNotebook" event', () => {
+      it('hides itself and republishes the event', () => {
         const sidebar = createSidebar();
-        sinon.stub(sidebar, 'hide').callThrough();
         sinon.stub(sidebar, 'publish');
+        sinon.stub(sidebar, 'close');
         emitEvent('openNotebook', 'mygroup');
         assert.calledWith(
           sidebar.publish,
           'openNotebook',
           sinon.match(['mygroup'])
         );
-        assert.calledOnce(sidebar.hide);
-        assert.notEqual(sidebar.frame.style.visibility, 'hidden');
+        assert.calledOnce(sidebar.close);
       });
     });
 
-    describe('on "closeNotebook" internal event', () => {
-      it('shows the sidebar', () => {
+    describe('on "closeNotebook" event', () => {
+      it('opens itself and republishes the event', () => {
         const sidebar = createSidebar();
-        sinon.stub(sidebar, 'show').callThrough();
-        sidebar.publish('closeNotebook');
-        assert.calledOnce(sidebar.show);
-        assert.equal(sidebar.frame.style.visibility, '');
+        sinon.stub(sidebar, 'publish');
+        sinon.stub(sidebar, 'open');
+        emitEvent('closeNotebook');
+        assert.calledWith(sidebar.publish, 'closeNotebook');
+        assert.calledOnce(sidebar.open);
       });
     });
 

src/styles/annotator/annotator.scss

@@ -47,11 +47,6 @@
   user-select: none;
   -webkit-tap-highlight-color: rgba(255, 255, 255, 0);
 
-  &.is-hidden {
-    visibility: hidden;
-    transition: visibility var.$annotator-timing--sidebar-collapse-transition;
-  }
-
   &.annotator-collapsed {
     margin-left: 0;
 

src/styles/annotator/notebook.scss

 @use '../variables' as var;
 @use '../mixins/molecules';
-@use "../mixins/buttons";
 
 .notebook-outer {
   box-sizing: border-box;
   position: fixed;
-  // This large zIndex is used to bring the notebook to the front, so it is not
-  // hidden by other elements from the host page. It is the same value used by
-  // the sidebar
-  z-index: 2147483647;
   top: 0;
   left: 0;
   width: 100vw;
   height: 100vh;
   padding: var.$layout-space;
+  // Leave explicitly the right amount of room for closed-sidebar affordances
+  padding-right: var.$annotator-toolbar-width + 5px;
 
   &.is-open {
     // TBD: Actual opacity/overlay we'd like to use
@@ -27,30 +24,4 @@
   padding: 0;
   width: 100%;
   height: 100%;
-
-  // FIXME: these properties produce a visual break between the Notebook__controller-bar and the iframe.
-  // A better approach would be if the Notebook__controller-bar and iframe could be children of notebook-inner.
-  border: none;
-  border-top-left-radius: 0;
-  border-top-right-radius: 0;
-}
-
-// This container element has the purpose of pushing children to the right side.
-.Notebook__controller-bar {
-  font-size: var.$font-size--large;
-  display: flex;
-  justify-content: flex-end;
-  background-color: var.$grey-2;
-  padding: var.$layout-space--xsmall;
-
-  // FIXME: these properties emulates as if the Notebook__controller-bar would be part of the iframe.
-  border-top-left-radius: var.$border-radius;
-  border-top-right-radius: var.$border-radius;
-}
-
-.Notebook__close-button {
-  @include buttons.button--labeled(
-    $background-color: var.$grey-2,
-    $active-background-color: var.$grey-3
-  );
 }

src/styles/sidebar/components/HypothesisApp.scss

@@ -24,9 +24,9 @@
   // FIXME: Temporary affordance for the Notebook view to override some
   // layout spacing that assumes the presence of the top bar
   &--notebook {
-    padding: 0 var.$layout-space;
+    padding: var.$layout-space;
     @include responsive.respond-to(tablets desktops) {
-      padding: 0 var.$layout-space--xlarge;
+      padding: var.$layout-space--xlarge;
     }
   }
 }