Update Notebook components to use shared button components

src/annotator/components/NotebookModal.js

@@ -3,8 +3,7 @@ import classnames from 'classnames';
 
 import { createSidebarConfig } from '../config/sidebar';
 
-// FIXME: use the button from the frontend shared package once this is stable.
-import Button from '../../sidebar/components/Button';
+import { LabeledButton } from '../../shared/components/buttons';
 
 /**
  * @typedef NotebookIframeProps
@@ -102,13 +101,16 @@ export default function NotebookModal({ eventBus, config }) {
   return (
     <div className={classnames('Notebook__outer', { 'is-hidden': isHidden })}>
       <div className="Notebook__inner">
-        <Button
+        <div className="Notebook__close-button-container">
+          <LabeledButton
             icon="cancel"
-          className="Notebook__close-button"
-          buttonText="Close"
             title="Close the Notebook"
             onClick={onClose}
-        />
+            variant="dark"
+          >
+            Close
+          </LabeledButton>
+        </div>
         {groupId !== null && (
           <NotebookIframe key={iframeKey} config={config} groupId={groupId} />
         )}

src/annotator/components/test/NotebookModal-test.js

@@ -109,7 +109,7 @@ describe('NotebookModal', () => {
     assert.isFalse(outer.hasClass('is-hidden'));
 
     act(() => {
-      wrapper.find('Button').prop('onClick')();
+      wrapper.find('LabeledButton').prop('onClick')();
     });
     wrapper.update();
 
@@ -125,7 +125,7 @@ describe('NotebookModal', () => {
     });
     assert.equal(document.body.style.overflow, 'hidden');
     act(() => {
-      wrapper.find('Button').prop('onClick')();
+      wrapper.find('LabeledButton').prop('onClick')();
     });
     assert.notEqual(document.body.style.overflow, 'hidden');
   });

src/sidebar/components/PaginationNavigation.js

 import { pageNumberOptions } from '../util/pagination';
 
-import Button from './Button';
+import { LabeledButton } from '../../shared/components/buttons';
 
 /**
  * @typedef PaginationNavigationProps
@@ -35,13 +35,15 @@ function PaginationNavigation({ currentPage, onChangePage, totalPages }) {
     <div className="PaginationNavigation">
       <div className="PaginationNavigation__relative PaginationNavigation__prev">
         {hasPreviousPage && (
-          <Button
-            className="PaginationNavigation__button"
+          <LabeledButton
+            className="PaginationPageButton"
             icon="arrow-left"
-            buttonText="prev"
             title="Go to previous page"
             onClick={e => changePageTo(currentPage - 1, e.target)}
-          />
+            variant="dark"
+          >
+            prev
+          </LabeledButton>
         )}
       </div>
       <ul className="PaginationNavigation__pages">
@@ -50,28 +52,32 @@ function PaginationNavigation({ currentPage, onChangePage, totalPages }) {
             {page === null ? (
               <div className="PaginationNavigation__gap">...</div>
             ) : (
-              <Button
+              <LabeledButton
+                className="PaginationPageButton"
                 key={`page-${idx}`}
-                buttonText={page.toString()}
                 title={`Go to page ${page}`}
-                className="PaginationNavigation__page-button"
-                isPressed={page === currentPage}
+                pressed={page === currentPage}
                 onClick={e => changePageTo(page, e.target)}
-              />
+                variant="dark"
+              >
+                {page.toString()}
+              </LabeledButton>
             )}
           </li>
         ))}
       </ul>
       <div className="PaginationNavigation__relative PaginationNavigation__next">
         {hasNextPage && (
-          <Button
-            className="PaginationNavigation__button PaginationNavigation__button-right"
+          <LabeledButton
+            className="PaginationPageButton"
             icon="arrow-right"
-            buttonText="next"
             iconPosition="right"
             title="Go to next page"
             onClick={e => changePageTo(currentPage + 1, e.target)}
-          />
+            variant="dark"
+          >
+            next
+          </LabeledButton>
         )}
       </div>
     </div>

src/styles/annotator/annotator.scss

@@ -10,6 +10,9 @@
 // -----------------
 @use '@hypothesis/frontend-shared/styles';
 
+// Shared button styles (TEMPORARY)
+@use '../shared';
+
 // Annotator-specific components.
 @use './components/AdderToolbar';
 @use './components/Buckets';

src/styles/annotator/notebook.scss

@@ -39,11 +39,10 @@
   border: none;
 }
 
-.Notebook__close-button {
+.Notebook__close-button-container {
   position: absolute;
   right: 0;
   font-size: var.$font-size--large;
-  @include buttons.button--primary;
   margin: var.$layout-space--xsmall;
   cursor: pointer;
 }

src/styles/sidebar/buttons.scss

+@use '../variables' as var;
 // Button styling for the sidebar extending common button-component styles
 @use '../shared/components/buttons/mixins' as buttons;
 
@@ -25,3 +26,11 @@
     }
   }
 }
+
+// This button style has asymmetrical padding. Hold it here to see if
+// this kind of padding is useful in other patterns.
+.PaginationPageButton {
+  @include buttons.LabeledButton {
+    padding: var.$layout-space--small var.$layout-space;
+  }
+}

src/styles/sidebar/components/PaginationNavigation.scss

@@ -26,31 +26,6 @@
     display: none;
   }
 
-  &__button,
-  &__page-button {
-    @include buttons.button--labeled(
-      $background-color: transparent,
-      $active-background-color: var.$grey-3
-    );
-  }
-
-  &__page-button {
-    margin: var.$layout-space--xxsmall;
-    padding: var.$layout-space--small var.$layout-space;
-
-    &[aria-pressed='true'] {
-      background-color: var.$grey-3;
-    }
-  }
-
-  &__button-right {
-    // FIXME Button SVG margins assume that the icon is on the left
-    svg {
-      margin-left: var.$layout-space--xxsmall;
-    }
-    padding-right: var.$layout-space--xxsmall;
-  }
-
   @include responsive.wide-handheld-and-up {
     // Where there's enough horizontal space,
     // lay out page navigation buttons horizontally as:
@@ -65,6 +40,7 @@
       display: block;
       grid-area: numberedPages;
       @include layout.row($justify: center, $align: center);
+      @include layout.horizontal-rhythm(0.5em);
     }
   }
 }