From 352e42561ad9cb17e26f0b6d02698e2c3bf386a8 Mon Sep 17 00:00:00 2001
From: Lyza Danger Gardner <lyza@lyza.com>
Date: Tue, 22 Sep 2020 15:55:32 -0400
Subject: [PATCH] Extract and refactor annotator toolbar SASS

---
 src/annotator/components/toolbar.js          | 43 +++++-----
 src/styles/annotator/annotator.scss          | 90 +++-----------------
 src/styles/annotator/bucket-bar.scss         |  4 +-
 src/styles/annotator/components/toolbar.scss | 77 +++++++++++++++++
 src/styles/variables.scss                    |  6 +-
 5 files changed, 117 insertions(+), 103 deletions(-)
 create mode 100644 src/styles/annotator/components/toolbar.scss

diff --git a/src/annotator/components/toolbar.js b/src/annotator/components/toolbar.js
index 16207b294fb..6d36fc60583 100644
--- a/src/annotator/components/toolbar.js
+++ b/src/annotator/components/toolbar.js
@@ -1,4 +1,3 @@
-import classnames from 'classnames';
 import propTypes from 'prop-types';
 import { createElement } from 'preact';
 
@@ -8,7 +7,7 @@ import SvgIcon from '../../shared/components/svg-icon';
  * @param {Object} props
  *  @param {import("preact").Ref<HTMLButtonElement>} [props.buttonRef]
  *  @param {boolean} [props.expanded]
- *  @param {string} [props.extraClasses]
+ *  @param {string} [props.className]
  *  @param {string} props.label
  *  @param {string} props.icon
  *  @param {() => any} props.onClick
@@ -17,7 +16,7 @@ import SvgIcon from '../../shared/components/svg-icon';
 function ToolbarButton({
   buttonRef,
   expanded,
-  extraClasses,
+  className = 'annotator-toolbar-button',
   label,
   icon,
   onClick,
@@ -32,7 +31,7 @@ function ToolbarButton({
 
   return (
     <button
-      className={classnames('annotator-frame-button', extraClasses)}
+      className={className}
       aria-label={label}
       aria-expanded={expanded}
       aria-pressed={selected}
@@ -48,7 +47,7 @@ function ToolbarButton({
 ToolbarButton.propTypes = {
   buttonRef: propTypes.any,
   expanded: propTypes.bool,
-  extraClasses: propTypes.string,
+  className: propTypes.string,
   label: propTypes.string.isRequired,
   icon: propTypes.string.isRequired,
   onClick: propTypes.func.isRequired,
@@ -104,7 +103,7 @@ export default function Toolbar({
     <div>
       {useMinimalControls && isSidebarOpen && (
         <ToolbarButton
-          extraClasses="annotator-frame-button--sidebar_close"
+          className="annotator-toolbar__sidebar-close"
           label="Close annotation sidebar"
           icon="cancel"
           onClick={closeSidebar}
@@ -112,7 +111,7 @@ export default function Toolbar({
       )}
       {!useMinimalControls && (
         <ToolbarButton
-          extraClasses="annotator-frame-button--sidebar_toggle"
+          className="annotator-toolbar__sidebar-toggle"
           buttonRef={toggleSidebarRef}
           label="Annotation sidebar"
           icon={isSidebarOpen ? 'caret-right' : 'caret-left'}
@@ -121,21 +120,21 @@ export default function Toolbar({
         />
       )}
       {!useMinimalControls && (
-        <ToolbarButton
-          label="Show highlights"
-          icon={showHighlights ? 'show' : 'hide'}
-          selected={showHighlights}
-          onClick={toggleHighlights}
-        />
-      )}
-      {!useMinimalControls && (
-        <ToolbarButton
-          label={
-            newAnnotationType === 'note' ? 'New page note' : 'New annotation'
-          }
-          icon={newAnnotationType === 'note' ? 'note' : 'annotate'}
-          onClick={createAnnotation}
-        />
+        <div className="annotator-toolbar-buttonbar">
+          <ToolbarButton
+            label="Show highlights"
+            icon={showHighlights ? 'show' : 'hide'}
+            selected={showHighlights}
+            onClick={toggleHighlights}
+          />
+          <ToolbarButton
+            label={
+              newAnnotationType === 'note' ? 'New page note' : 'New annotation'
+            }
+            icon={newAnnotationType === 'note' ? 'note' : 'annotate'}
+            onClick={createAnnotation}
+          />
+        </div>
       )}
     </div>
   );
diff --git a/src/styles/annotator/annotator.scss b/src/styles/annotator/annotator.scss
index a0b6d47ec75..a121d8ffc5b 100644
--- a/src/styles/annotator/annotator.scss
+++ b/src/styles/annotator/annotator.scss
@@ -1,21 +1,18 @@
+// SASS entry point for annotator styling
+
 @use "sass:meta";
-@use "sass:color";
 @use '../variables' as var;
-@use '../mixins/focus';
 @use '../mixins/reset';
-@use '../mixins/utils';
 
 // Shared components.
 @use '../components/svg-icon';
 
 // Annotator-specific components.
 @use './components/adder';
+@use './components/toolbar';
 @use './bucket-bar';
 @use './highlights';
 
-$sidebar-collapse-transition-time: 150ms;
-$shadow--sidebar: 0px 1px 4px color.scale(var.$color-shadow, $alpha: -50%);
-
 // Sidebar
 .annotator-frame {
   // CSS reset which attempts to isolate this element and its children from
@@ -33,16 +30,16 @@ $shadow--sidebar: 0px 1px 4px color.scale(var.$color-shadow, $alpha: -50%);
   }
 
   // frame styles
-  user-select: none;
-  direction: ltr;
-  background: none;
-  font-size: var.$annotator-base-font-size;
-  line-height: var.$annotator-base-line-height;
-  height: 100%;
   position: fixed;
   top: 0;
   left: 100%;
+  height: 100%;
   z-index: 2147483647;
+  direction: ltr;
+  font-size: var.$annotator-adder-font-size;
+  line-height: var.$annotator-base-line-height;
+
+  user-select: none;
   -webkit-tap-highlight-color: rgba(255, 255, 255, 0);
 
   &.annotator-collapsed {
@@ -52,7 +49,7 @@ $shadow--sidebar: 0px 1px 4px color.scale(var.$color-shadow, $alpha: -50%);
       // Add a transition when collapsing only. This serves to delay
       // the effect until the sidebar finishes closing. Visibility is
       // a boolean value and can not actually animate.
-      transition: visibility $sidebar-collapse-transition-time;
+      transition: visibility var.$annotator-timing--sidebar-collapse-transition;
       visibility: hidden;
     }
   }
@@ -64,74 +61,11 @@ $shadow--sidebar: 0px 1px 4px color.scale(var.$color-shadow, $alpha: -50%);
     z-index: 3;
     position: relative;
   }
-
-  // the vertical toolbar at the left-edge of the sidebar
-  // which provides controls for toggling the sidebar,
-  // toggling highlights etc.
-
-  // note: These components CSS classes are nested inside
-  // .annotator-frame to avoid being overridden by the
-  // CSS reset styles applied to .annotator-frame, which would otherwise
-  // have higher specificity
-  .annotator-toolbar {
-    position: absolute;
-    left: -(var.$bucket-bar-width + 18px - 7px);
-    width: 37px;
-    z-index: 2;
-  }
-
-  // FIXME:  Use variables for sizing here
-  .annotator-frame-button {
-    @include focus.outline-on-keyboard-focus;
-
-    transition: background-color 0.25s;
-    @include utils.shadow;
-    background: var.$white;
-    border: solid 1px var.$grey-3;
-    border-radius: 4px;
-    color: var.$grey-5;
-    text-decoration: none;
-    height: 30px;
-    width: 30px;
-    padding: 1px 6px;
-    margin-bottom: 5px;
-
-    &:active {
-      background-color: var.$grey-semi;
-    }
-
-    &:focus,
-    &:hover {
-      color: var.$grey-7;
-    }
-  }
-
-  .annotator-frame-button--sidebar_toggle {
-    box-shadow: none;
-    border-radius: 0px;
-    border-style: none none solid solid;
-    padding-right: 6px;
-    width: 36px;
-    margin-bottom: 10px;
-    height: var.$top-bar-height;
-    color: var.$grey-semi;
-  }
-
-  /** Visible with clean theme */
-  .annotator-frame-button--sidebar_close {
-    box-shadow: $shadow--sidebar;
-    border-radius: 0px;
-    border-style: solid none solid solid;
-    width: 27px;
-    margin-top: 140px;
-    margin-left: 6px;
-    height: 27px;
-  }
 }
 
 /** Visible with clean theme */
 .annotator-frame--drop-shadow-enabled {
-  box-shadow: $shadow--sidebar;
+  box-shadow: var.$annotator-shadow--sidebar;
 }
 
 .annotator-placeholder {
@@ -181,7 +115,7 @@ $shadow--sidebar: 0px 1px 4px color.scale(var.$color-shadow, $alpha: -50%);
 
 @media screen and (min-width: 37.5em) {
   .annotator-frame {
-    transition: margin-left $sidebar-collapse-transition-time
+    transition: margin-left var.$annotator-timing--sidebar-collapse-transition
       cubic-bezier(0.55, 0, 0.2, 0.8);
     width: 428px;
     margin-left: -428px;
diff --git a/src/styles/annotator/bucket-bar.scss b/src/styles/annotator/bucket-bar.scss
index 4f03ff9739f..1f3655e7ef4 100644
--- a/src/styles/annotator/bucket-bar.scss
+++ b/src/styles/annotator/bucket-bar.scss
@@ -10,8 +10,8 @@
     pointer-events: none;
     position: absolute;
     height: 100%;
-    width: var.$bucket-bar-width;
-    left: -(var.$bucket-bar-width);
+    width: var.$annotator-bucket-bar-width;
+    left: -(var.$annotator-bucket-bar-width);
   }
 
   // When the sidebar is collapsed, make the background semi-transparent so the
diff --git a/src/styles/annotator/components/toolbar.scss b/src/styles/annotator/components/toolbar.scss
new file mode 100644
index 00000000000..82f674f01fe
--- /dev/null
+++ b/src/styles/annotator/components/toolbar.scss
@@ -0,0 +1,77 @@
+@use "sass:color";
+
+@use '../variables' as var;
+
+@use '../mixins/buttons';
+@use '../mixins/layout';
+@use '../mixins/utils';
+
+// note: These components CSS classes are nested inside
+// .annotator-frame to avoid being overridden by the
+// CSS reset styles applied to .annotator-frame, which would otherwise
+// have higher specificity
+.annotator-frame {
+  // the vertical toolbar at the left-edge of the sidebar
+  // which provides controls for toggling the sidebar,
+  // toggling highlights etc.
+
+  .annotator-toolbar {
+    position: absolute;
+    left: -(var.$annotator-toolbar-width);
+    width: var.$annotator-toolbar-width;
+    z-index: 2;
+  }
+
+  .annotator-toolbar-buttonbar {
+    @include layout.vertical-rhythm(5px);
+    margin-top: var.$layout-space--small;
+  }
+
+  // Common styling for buttons in the toolbar
+  @mixin annotator-button {
+    @include buttons.button--icon-only($with-active-state: false);
+    // These toolbar buttons are slightly lighter than other icon buttons
+    color: var.$grey-5;
+    background: var.$color-background;
+  }
+
+  // Toolbar button with icon
+  .annotator-toolbar-button {
+    @include annotator-button;
+    @include utils.shadow;
+    @include utils.border;
+    border-radius: var.$annotator-border-radius;
+  }
+
+  // Control to collapse/expand the sidebar
+  .annotator-toolbar__sidebar-toggle {
+    @include annotator-button;
+    @include utils.border--left;
+    @include utils.border--bottom;
+    // Precise positioning of expand/collapse icon
+    padding-left: 2px;
+    // Make the button fill the entire width of the toolbar and the
+    // entire height of the top bar
+    width: var.$annotator-toolbar-width;
+    height: var.$top-bar-height;
+    // Lighten the icon color
+    color: var.$grey-semi;
+  }
+
+  /** Visible with clean theme */
+  .annotator-toolbar__sidebar-close {
+    @include buttons.reset-native-btn-styles;
+    @include buttons.button-hover;
+    @include utils.border;
+    border-right-width: 0;
+    background: var.$color-background;
+    box-shadow: var.$annotator-shadow--sidebar;
+    color: var.$grey-5;
+    // Precise positioning of close button
+    padding: 1px 6px;
+    width: 27px;
+    height: 27px;
+    margin-top: 140px;
+    margin-left: 6px;
+  }
+}
diff --git a/src/styles/variables.scss b/src/styles/variables.scss
index 7a8bd94255d..b284be6ea66 100644
--- a/src/styles/variables.scss
+++ b/src/styles/variables.scss
@@ -99,7 +99,6 @@ $touch-target-size: 44px;
 
 // Other Variables
 // -------------------------
-$bucket-bar-width: 22px;
 $top-bar-height: 40px;
 $border-radius: 2px;
 
@@ -113,6 +112,8 @@ $icon-size--large: 24px;
 
 // Annotator-specific values
 // -----------------------------------
+$annotator-bucket-bar-width: 22px;
+$annotator-toolbar-width: 33px;
 
 $annotator-base-font-size: 14px;
 $annotator-base-line-height: 20px;
@@ -124,3 +125,6 @@ $annotator-bucket-bar-font-size: 10px;
 $annotator-bucket-bar-line-height: 14px;
 
 $annotator-border-radius: 4px;
+
+$annotator-shadow--sidebar: 0px 1px 4px color.scale($color-shadow, $alpha: -50%);
+$annotator-timing--sidebar-collapse-transition: 150ms;