Merge pull request #2252 from hypothesis/add-atomic-mixins

Add atomic mixins for borders, icons

src/images/icons/highlight.svg

-<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
-  <path fill="currentColor" fill-rule="nonzero" d="M16 13.344V16H0v-2.656h16zM9.328 2.172l2.5 2.5-6.656 6.672h-2.5v-2.5l6.656-6.672zM11.773 0c.146 0 .277.043.393.13l.084.073 1.547 1.563a.603.603 0 01.203.46.639.639 0 01-.13.393l-.073.084L12.5 4 10 1.5 11.297.203A.652.652 0 0111.773 0z"/>
-</svg>
+<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"><g fill-rule="evenodd"><rect fill="none" stroke="none" x="0" y="0" width="16" height="16"></rect><path fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 15H1h11zm-.5-6v2l-1 1v-2l1-1zm.5-7v6h-2V2h2zm0-1h-2 2zm0 8h-2 2z"></path></g></svg>

src/styles/mixins/buttons.scss

@@ -19,6 +19,7 @@
  */
 @use "./focus";
 @use "./layout";
+@use "./utils";
 @use "../variables" as var;
 
 @mixin reset-native-btn-styles {
@@ -42,8 +43,7 @@
 
   // Icon
   svg {
-    width: 16px;
-    height: 16px;
+    @include utils.icon--medium;
   }
 
   &:hover {
@@ -108,12 +108,12 @@
  */
 @mixin button--input {
   @include button;
+  @include utils.border;
   color: var.$grey-mid;
   padding: 0.5em 0.75em;
   background-color: var.$grey-1;
 
   border-radius: 0; // Turn off border-radius to align with <input> edges
-  border: 1px solid var.$grey-3;
   border-left: 0; // Avoid double border with the <input>
 
   &:hover {

src/styles/mixins/forms.scss

-@use "../mixins/focus";
+@use "./focus";
+@use "./utils";
 @use "../variables" as var;
 
 /* Style input placeholders */
@@ -32,7 +33,7 @@
 
 @mixin form-input {
   @include var.font-normal;
-  border: 1px solid var.$grey-3;
+  @include utils.border;
   border-radius: 2px;
   padding: 0.5em 0.75em;
   font-weight: normal;

src/styles/mixins/links.scss

 @use "../variables" as var;
 @use "./layout";
+@use "./utils";
 
 /**
  * horizontally-oriented collection of linked icons
  */
 @mixin footer-links {
   @include layout.row(center);
+  @include utils.border-top;
   padding-top: 8px;
-  border-top: 1px solid var.$grey-3;
 
   &__icon {
     @include layout.row;

src/styles/mixins/panel.scss

 @use "../variables" as var;
 @use "./buttons";
 @use "./layout";
+@use "./utils";
 
 /**
  * Base styles for a "panel"-like element, with appropriate
  * padding, heading and close-button styles.
  */
 @mixin panel {
+  @include utils.border;
   background-color: var.$color-background;
-  border: solid 1px var.$grey-3;
   border-radius: 2px;
 
   &__header {
     @include layout.row($align: center);
+    @include utils.border-bottom;
     padding: 1em 0;
     margin: 0 1em;
-    border: 1px none var.$grey-3;
-    border-bottom-style: solid;
   }
 
   &__header-icon {

src/styles/mixins/utils.scss

+@use "../variables" as var;
+
+@mixin border {
+  border: var.$border-width solid var.$color-border;
+}
+
+@mixin border-top {
+  border-top: var.$border-width solid var.$color-border;
+}
+
+@mixin border-right {
+  border-right: var.$border-width solid var.$color-border;
+}
+
+@mixin border-bottom {
+  border-bottom: var.$border-width solid var.$color-border;
+}
+
+@mixin border-left {
+  border-left: var.$border-width solid var.$color-border;
+}
+
+@mixin icon--xsmall {
+  width: var.$icon-size--xsmall;
+  height: var.$icon-size--xsmall;
+}
+
+// TODO: This may be only applicable to mobile devices, and maybe should be
+// conflated into icon--xsmall
+@mixin icon--small {
+  width: var.$icon-size--small;
+  height: var.$icon-size--small;
+}
+
+@mixin icon--medium {
+  width: var.$icon-size--medium;
+  height: var.$icon-size--medium;
+}
+
+@mixin icon--large {
+  width: var.$icon-size--large;
+  height: var.$icon-size--large;
+}
+
+@mixin icon--inline {
+  width: 1em;
+  height: 1em;
+}

src/styles/sidebar/components/annotation-header.scss

 @use "../../mixins/buttons";
 @use "../../mixins/forms";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .annotation-header {
@@ -12,8 +13,7 @@
   }
 
   &__icon {
-    width: 10px;
-    height: 10px;
+    @include utils.icon--xsmall;
     color: var.$color-text;
   }
 
@@ -44,10 +44,7 @@
   }
 
   &__highlight-icon {
-    color: var.$grey-5;
-    width: 12px;
-    height: 12px;
-    /* bring down the baseline a little */
-    margin-bottom: -1px;
+    @include utils.icon--xsmall;
+    color: var.$color-text;
   }
 }

src/styles/sidebar/components/annotation-license.scss

 @use "../../variables" as var;
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 
 .annotation-license {
   @include var.font-small;
-  border-top: 1px solid var.$grey-3;
+  @include utils.border-top;
   padding-top: 0.5em;
 
   &__link {
@@ -12,8 +13,8 @@
   }
 
   &__icon {
-    width: 12px;
-    height: 12px;
+    @include utils.icon--xsmall;
+    // Need a tiny space between license icons
     margin-right: 1px;
   }
 

src/styles/sidebar/components/annotation-quote.scss

@@ -6,7 +6,7 @@
   }
 
   &__quote {
-    border-left: 3px solid var.$grey-3;
+    border-left: 3px solid var.$color-border;
     color: var.$color-text-light;
     font-style: italic;
     padding: 0 1em;

src/styles/sidebar/components/annotation-share-control.scss

@@ -2,6 +2,7 @@
 @use '../../mixins/layout';
 @use '../../mixins/links';
 @use '../../mixins/panel';
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .annotation-share-control {
@@ -47,7 +48,6 @@
   }
 
   .share-links__icon {
-    width: 18px;
-    height: 18px;
+    @include utils.icon--medium;
   }
 }

src/styles/sidebar/components/annotation-share-info.scss

 @use "../../variables" as var;
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 
 .annotation-share-info {
   @include layout.row($align: baseline);
@@ -11,9 +12,8 @@
   }
 
   &__icon {
+    @include utils.icon--xsmall;
     // This margin is currently needed because the icon is within an `a` element
     margin-right: 5px;
-    width: 10px;
-    height: 10px;
   }
 }

src/styles/sidebar/components/focused-mode-header.scss

 @use "../../mixins/focus";
 @use "../../mixins/forms";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 // A dark grey button used for the primary action
 // in a form
 .focused-mode-header {
   @include layout.row($align: center);
+  @include utils.border;
   position: relative;
 
   background-color: var.$color-background;
   border-radius: 2px;
-  border: solid 1px var.$grey-3;
   font-weight: 300;
   margin-bottom: 0.72em;
   padding: 0.5em 1em;

src/styles/sidebar/components/group-list.scss

 @use "../../variables" as var;
+@use "../../mixins/utils";
 
 .group-list__content {
   min-width: 250px;
@@ -17,8 +18,7 @@
 }
 
 .group-list__menu-icon {
-  width: 15px;
-  height: 15px;
+  @include utils.icon--medium;
   margin-right: 4px;
 
   // A minor adjustment to make the default icon (the Hypothesis logo) align

src/styles/sidebar/components/help-panel.scss

 @use "../../mixins/buttons";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .help-panel {
@@ -11,9 +12,9 @@
   }
 
   &__content {
+    @include utils.border-top;
+    @include utils.border-bottom;
     padding: 0.5em;
-    border-top: 1px solid var.$grey-3;
-    border-bottom: 1px solid var.$grey-3;
 
     a {
       text-decoration: underline;
@@ -21,8 +22,7 @@
   }
 
   &__icon {
-    width: 12px;
-    height: 12px;
+    @include utils.icon--xsmall;
   }
 
   &__footer {
@@ -50,9 +50,9 @@
 
     &__tab {
       @include var.font-big;
+      @include utils.border-right;
       flex: 1 1 0px;
       margin-top: 0.5em;
-      border-right: 1px solid var.$grey-3;
       text-align: center;
       color: var.$color-text-light;
 
@@ -66,8 +66,7 @@
     }
 
     &__icon {
-      width: 12px;
-      height: 12px;
+      @include utils.icon--xsmall;
     }
   }
 }

src/styles/sidebar/components/markdown-editor.scss

 @use "../../mixins/buttons";
 @use "../../mixins/forms";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
-$toolbar-border: 0.1em solid var.$grey-3;
-
 .markdown-editor__toolbar {
   @include layout.row;
-
   // Toolbar buttons wrap on non-touch devices if they don't fit. We don't use
   // scrolling because that's less convenient to use with a mouse/touchpad.
   flex-wrap: wrap;
 
-  background-color: white;
-  border: $toolbar-border;
+  @include utils.border;
   border-bottom: none;
+
+  background-color: white;
   border-radius: 0.15em 0.15em 0 0;
   width: 100%;
   margin-bottom: -0.1em;
@@ -40,13 +39,12 @@ $toolbar-border: 0.1em solid var.$grey-3;
   }
 
   &-icon {
-    width: 10px;
-    height: 10px;
+    @include utils.icon--xsmall;
   }
 }
 
 .markdown-editor__preview {
-  border: $toolbar-border;
+  @include utils.border;
   background-color: var.$grey-1;
   padding: 10px;
 }
@@ -88,8 +86,7 @@ $toolbar-border: 0.1em solid var.$grey-3;
     min-height: var.$touch-target-size;
 
     &-icon {
-      width: 12px;
-      height: 12px;
+      @include utils.icon--small;
     }
   }
 }

src/styles/sidebar/components/menu-item.scss

 @use "sass:color";
 @use "../../mixins/focus";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 $menu-item-padding: 10px;
@@ -60,9 +61,9 @@ a.menu-item:hover {
   }
 
   &.is-expanded {
+    @include utils.border-bottom;
     background-color: var.$grey-1;
     color: var.$color-text;
-    border-bottom: solid 1px var.$grey-3;
     &:hover {
       background-color: var.$grey-3;
     }
@@ -77,17 +78,16 @@ a.menu-item:hover {
   }
 }
 .menu-item__icon {
+  @include utils.icon--medium;
   color: inherit;
   display: inline-block;
   margin-right: 4px;
   position: relative;
-  height: 15px;
-  width: 15px;
-
   &-container {
     margin-right: 10px;
-    width: 15px;
-    height: 15px;
+    // TODO Not using icon mixins for now until reviewing this entire pattern
+    width: 16px;
+    height: 16px;
   }
 }
 
@@ -134,6 +134,7 @@ a.menu-item:hover {
   }
 
   &-icon {
+    // TODO not an icon in the general sense of the term; rename class?
     width: 12px;
     height: 12px;
   }
@@ -141,7 +142,7 @@ a.menu-item:hover {
 
 // The container for open submenus
 .menu-item__submenu {
-  border-bottom: solid 1px var.$grey-3;
+  @include utils.border-bottom;
   &:hover {
     // Make it a bit darker on hover.
     background-color: var.$grey-3;

src/styles/sidebar/components/menu-section.scss

 @use "../../variables" as var;
+@use "../../mixins/utils";
 
 .menu-section__content {
-  border-bottom: solid 1px var.$grey-3;
+  @include utils.border-bottom;
 }
 
 .menu-section__heading {

src/styles/sidebar/components/menu.scss

 @use "../../mixins/focus";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .menu {
@@ -35,8 +36,7 @@
   height: 100%;
 }
 .menu__toggle-icon {
-  width: 10px;
-  height: 10px;
+  @include utils.icon--xsmall;
 }
 
 // Triangular indicator next to the toggle button indicating that there is
@@ -72,8 +72,8 @@
 // Content area of the menu.
 .menu__content {
   @include var.font-big;
+  @include utils.border;
   background-color: white;
-  border: 1px solid var.$grey-3;
   box-shadow: var.$popup-menu-shadow;
   position: absolute;
   top: calc(100% + 5px);

src/styles/sidebar/components/selection-tabs.scss

 @use "../../mixins/focus";
 @use "../../mixins/buttons";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .selection-tabs-container {
@@ -63,8 +64,8 @@
 }
 
 .selection-tabs__message {
+  @include utils.border;
   color: var.$color-text;
-  border: 1px solid var.$grey-3;
   padding: 2em;
   text-align: center;
 }

src/styles/sidebar/components/share-annotations-panel.scss

 @use "../../variables" as var;
 @use "../../mixins/buttons";
 @use "../../mixins/links";
+@use "../../mixins/utils";
 
 .share-annotations-panel {
   color: var.$color-text-light;
@@ -31,8 +32,7 @@
     @include links.footer-links;
 
     &__icon {
-      width: 24px;
-      height: 24px;
+      @include utils.icon--large;
     }
   }
 }

src/styles/sidebar/components/share-links.scss

 @use '../../mixins/links';
+@use "../../mixins/utils";
 
 .share-links {
   @include links.footer-links;
 
   &__icon {
-    width: 24px;
-    height: 24px;
+    @include utils.icon--large;
   }
 }

src/styles/sidebar/components/tag-editor.scss

 @use "../../mixins/forms";
 @use "../../mixins/buttons";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .tag-editor {
@@ -23,9 +24,9 @@
   }
 
   &__edit {
+    @include utils.border;
     color: var.$color-text;
     background: var.$grey-1;
-    border: 1px solid var.$grey-3;
     border-radius: 2px 0 0 2px;
     border-right-width: 0;
     padding: 2px 0.5em;
@@ -33,10 +34,10 @@
 
   &__delete {
     @include buttons.button;
+    @include utils.border;
     color: var.$grey-mid;
     background-color: var.$grey-1;
     padding: 0 0.5em;
-    border: 1px solid var.$grey-3;
     border-radius: 0 2px 2px 0;
 
     &:hover {

src/styles/sidebar/components/tag-list.scss

 @use "../../variables" as var;
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 
 .tag-list {
   @include layout.row;
@@ -7,10 +8,10 @@
   margin: 1em 0;
 
   &__item {
+    @include utils.border;
     margin: 0.25em 0.5em 0.25em 0;
     padding: 2px 0.5em;
     background: var.$grey-0;
-    border: 1px solid var.$grey-3;
     border-radius: 2px;
   }
 

src/styles/sidebar/components/thread.scss

@@ -24,14 +24,14 @@
   // Left "channel" of thread
   &__collapse {
     margin-right: 1em;
-    border-right: 1px dashed var.$grey-3;
+    border-right: 1px dashed var.$color-border;
     // The entire channel is NOT clickable so don't make it look like it is
     // (overrides `pointer` cursor applied to entire card)
     cursor: auto;
 
     // Darken thread line on hover as a visual cue to show related thread items
     &:hover {
-      border-right: 1px dashed var.$grey-4;
+      border-color: var.$grey-4;
     }
 
     .is-collapsed & {
@@ -49,6 +49,7 @@
     background-color: var.$white;
 
     svg {
+      // This is not an icon and as such does not use a mixin
       width: 12px;
       height: 12px;
       color: var.$grey-4;

src/styles/sidebar/components/top-bar.scss

 @use "../../mixins/buttons";
 @use "../../mixins/layout";
+@use "../../mixins/utils";
 @use "../../variables" as var;
 
 .top-bar {
   @include var.font-big;
+  @include utils.border-bottom;
   color: var.$grey-mid;
   background: var.$white;
-  border-bottom: solid 1px var.$grey-3;
   height: var.$top-bar-height;
   position: absolute;
   left: 0;
@@ -84,6 +85,11 @@
   @include layout.sidebar-content;
   @include layout.row(right, center);
 
+  // FIXME: This odd font size rule is because the spinner uses a `1em` dimension
+  // and we want to assure it displays at the same dimensions as the other icons
+  // in this top bar
+  font-size: var.$icon-size--medium;
+
   // the edges of the top-bar's contents should be aligned
   // with the edges of annotation cards displayed below
   $h-padding: 9px;

src/styles/sidebar/components/tutorial.scss

 @use "../../variables" as var;
+@use "../../mixins/utils";
 
 .tutorial {
   &__list {
@@ -11,8 +12,7 @@
   }
 
   &__icon {
-    width: 12px;
-    height: 12px;
+    @include utils.icon--inline;
     margin-right: 1px;
     margin-bottom: -1px; // Pull the icon a little toward the baseline
     color: var.$grey-5;

src/styles/variables.scss

@@ -104,6 +104,14 @@ $bucket-bar-width: 22px;
 $top-bar-height: 40px;
 $popup-menu-shadow: 0 1px 1px rgba(0, 0, 0, 0.1);
 
+$color-border: $grey-3;
+$border-width: 1px;
+
+$icon-size--xsmall: 10px;
+$icon-size--small: 12px;
+$icon-size--medium: 16px;
+$icon-size--large: 24px;
+
 // Mixins
 // ------