Merge pull request #2247 from hypothesis/add-layout-mixins

Add basic layout mixins

src/sidebar/components/annotation-share-control.js

@@ -126,7 +126,11 @@ function AnnotationShareControl({
               className="annotation-share-control__links"
             />
           </div>
-          <SvgIcon name="pointer" className="annotation-share-panel__arrow" />
+          <SvgIcon
+            name="pointer"
+            inline={true}
+            className="annotation-share-panel__arrow"
+          />
         </div>
       )}
     </div>

src/sidebar/components/annotation.js

@@ -122,18 +122,19 @@ function Annotation({
         />
       )}
 
+      {isEditing && (
+        <div className="annotation__form-actions">
+          <AnnotationPublishControl
+            annotation={annotation}
+            isDisabled={isEmpty}
+            onSave={onSave}
+          />
+        </div>
+      )}
+      {shouldShowLicense && <AnnotationLicense />}
+
       {!isCollapsedReply && (
         <footer className="annotation__footer">
-          {isEditing && (
-            <div className="annotation__form-actions">
-              <AnnotationPublishControl
-                annotation={annotation}
-                isDisabled={isEmpty}
-                onSave={onSave}
-              />
-            </div>
-          )}
-          {shouldShowLicense && <AnnotationLicense />}
           <div className="annotation__controls">
             {shouldShowReplyToggle && (
               <Button

src/sidebar/components/selection-tabs.js

 import classnames from 'classnames';
 import { createElement } from 'preact';
-import { Fragment } from 'preact';
 import propTypes from 'prop-types';
 
 import useStore from '../store/use-store';
@@ -30,26 +29,28 @@ function Tab({
   const title = count > 0 ? `${label} (${count} available)` : label;
 
   return (
-    <button
-      className={classnames('selection-tabs__type', {
-        'is-selected': isSelected,
-      })}
-      // Listen for `onMouseDown` so that the tab is selected when _pressed_
-      // as this makes the UI feel faster. Also listen for `onClick` as a fallback
-      // to enable selecting the tab via other input methods.
-      onClick={selectTab}
-      onMouseDown={selectTab}
-      role="tab"
-      tabIndex="0"
-      title={title}
-      aria-label={title}
-      aria-selected={isSelected.toString()}
-    >
-      {children}
-      {count > 0 && !isWaitingToAnchor && (
-        <span className="selection-tabs__count"> {count}</span>
-      )}
-    </button>
+    <div>
+      <button
+        className={classnames('selection-tabs__type', {
+          'is-selected': isSelected,
+        })}
+        // Listen for `onMouseDown` so that the tab is selected when _pressed_
+        // as this makes the UI feel faster. Also listen for `onClick` as a fallback
+        // to enable selecting the tab via other input methods.
+        onClick={selectTab}
+        onMouseDown={selectTab}
+        role="tab"
+        tabIndex="0"
+        title={title}
+        aria-label={title}
+        aria-selected={isSelected.toString()}
+      >
+        {children}
+        {count > 0 && !isWaitingToAnchor && (
+          <span className="selection-tabs__count"> {count}</span>
+        )}
+      </button>
+    </div>
   );
 }
 
@@ -114,7 +115,7 @@ function SelectionTabs({ isLoading, settings }) {
     selectedTab === uiConstants.TAB_NOTES && noteCount === 0;
 
   return (
-    <Fragment>
+    <div className="selection-tabs-container">
       <div
         className={classnames('selection-tabs', {
           'selection-tabs--theme-clean': isThemeClean,
@@ -153,29 +154,25 @@ function SelectionTabs({ isLoading, settings }) {
       </div>
       {selectedTab === uiConstants.TAB_NOTES &&
         settings.enableExperimentalNewNoteButton && <NewNoteBtn />}
-      {!isLoading && (
-        <div>
-          {showNotesUnavailableMessage && (
-            <div className="selection-tabs__message">
-              There are no page notes in this group.
-            </div>
-          )}
-          {showAnnotationsUnavailableMessage && (
-            <div className="selection-tabs__message">
-              There are no annotations in this group.
-              <br />
-              Create one by selecting some text and clicking the{' '}
-              <SvgIcon
-                name="annotate"
-                inline={true}
-                className="selection-tabs__icon"
-              />{' '}
-              button.
-            </div>
-          )}
+      {!isLoading && showNotesUnavailableMessage && (
+        <div className="selection-tabs__message">
+          There are no page notes in this group.
+        </div>
+      )}
+      {!isLoading && showAnnotationsUnavailableMessage && (
+        <div className="selection-tabs__message">
+          There are no annotations in this group.
+          <br />
+          Create one by selecting some text and clicking the{' '}
+          <SvgIcon
+            name="annotate"
+            inline={true}
+            className="selection-tabs__icon"
+          />{' '}
+          button.
         </div>
       )}
-    </Fragment>
+    </div>
   );
 }
 SelectionTabs.propTypes = {

src/styles/mixins/buttons.scss

@@ -18,6 +18,7 @@
  * And pass 'my-component-button' as the `className` prop to `Button`.
  */
 @use "./focus";
+@use "./layout";
 @use "../variables" as var;
 
 @mixin reset-native-btn-styles {
@@ -34,11 +35,8 @@
  */
 @mixin button {
   @include reset-native-btn-styles;
+  @include layout.row(center, center);
   padding: 0.5em;
-
-  display: flex;
-  align-items: center;
-  justify-content: center;
   border-radius: 2px;
   border: none;
 

src/styles/mixins/layout.scss

@@ -11,3 +11,69 @@
     padding-right: 4rem;
   }
 }
+/**
+ * Abstract mixin for establishing basic flex container. External users should
+ * use `row` or `column` as needed. Default values here reflect default CSS
+ * values for flex rules.
+ *
+ * @param {string} $direction [row] - value for flex-direction (row or column).
+ * @param {string} $justify [flex-start] - How to align contents on main axis.
+ *                                    Accepts and maps special value of 'right'
+ *                                   (roughly analogous to horizontal alignment)
+ * @param {string} $align [stretch] - How to align contents on cross axis.
+ *                                    (roughly analogous to vertical alignment)
+ */
+@mixin flex($direction: row, $justify: flex-start, $align: stretch) {
+  display: flex;
+  flex-direction: $direction;
+
+  @if $justify == right {
+    justify-content: flex-end;
+  } @else {
+    justify-content: $justify;
+  }
+  align-items: $align;
+}
+
+/**
+ * Establish a column (flex-direction: column) flex container.
+ *
+ * @param {string} $justify [flex-start] - How to justify flex contents
+ * @param {string} $align [stretch] - How to align flex contents
+ */
+@mixin column($justify: flex-start, $align: stretch) {
+  @include flex(column, $justify, $align);
+}
+
+/**
+ * Establish a row (flex-direction: column) flex container.
+ *
+ * @param {string} $justify [flex-start] - How to justify flex contents
+ * @param {string} $align [stretch] - How to align flex contents
+ */
+@mixin row($justify: flex-start, $align: stretch) {
+  @include flex(row, $justify, $align);
+}
+
+/**
+ * Establish a horizontal (margin) rhythm for this element's immediate
+ * children (i.e. put equal space between children).
+ *
+ * @param $size [5px] - Size of horizontal margin between child elements
+ */
+@mixin horizontal-rhythm($size: 5px) {
+  & > * + * {
+    margin-left: $size;
+  }
+}
+
+/**
+ * Establish a vertical (margin) rhythm for this element's immediate
+ * children (i.e. put equal space between children). Currently set to 1em.
+ * @FIXME Find workaround for SvgIcon exception
+ */
+@mixin vertical-rhythm {
+  & > * + *:not([class*='svg-icon--inline']) {
+    margin-top: 1em;
+  }
+}

src/styles/mixins/links.scss

 @use "../variables" as var;
+@use "./layout";
 
 /**
  * horizontally-oriented collection of linked icons
  */
 @mixin footer-links {
-  display: flex;
-  flex-direction: row;
-  justify-content: center;
+  @include layout.row(center);
   padding-top: 8px;
   border-top: 1px solid var.$grey-3;
 
   &__icon {
-    display: flex;
+    @include layout.row;
     width: 18px;
     height: 18px;
     margin: 0 8px;

src/styles/mixins/panel.scss

 @use "../variables" as var;
 @use "./buttons";
+@use "./layout";
 
 /**
  * Base styles for a "panel"-like element, with appropriate
@@ -11,9 +12,7 @@
   border-radius: 2px;
 
   &__header {
-    display: flex;
-    flex-direction: row;
-    align-items: center;
+    @include layout.row($align: center);
     padding: 1em 0;
     margin: 0 1em;
     border: 1px none var.$grey-3;

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

 @use '../../variables' as var;
 @use "../../mixins/buttons";
-
-.annotation-body {
-  margin: 1em 0;
-}
+@use "../../mixins/layout";
 
 .annotation-body__text {
   // Hidden annotations displayed to moderators, where the content is still
@@ -32,9 +29,8 @@
 }
 
 .annotation-body__collapse-toggle {
+  @include layout.row(right);
   margin: 0.5em 0;
-  display: flex;
-  justify-content: flex-end;
 
   .annotation-body__collapse-toggle-button {
     @include buttons.button--labeled;

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

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 .annotation-document-info {
+  @include layout.row;
+  @include layout.horizontal-rhythm;
   color: var.$color-text-light;
-  display: flex;
-
-  &__title {
-    margin-right: 5px;
-  }
-
-  &__domain {
-    margin-right: 5px;
-  }
 }

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

 @use "../../mixins/buttons";
 @use "../../mixins/forms";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .annotation-header {
   &__row {
-    display: flex;
+    @include layout.row($align: baseline);
+    @include layout.horizontal-rhythm;
+    // Wrap onto multiple rows from bottom to top
     flex-wrap: wrap-reverse;
-    align-items: baseline;
   }
 
   &__icon {
-    margin-right: 5px;
     width: 10px;
     height: 10px;
     color: var.$color-text;
@@ -42,10 +43,6 @@
     color: var.$color-text-light;
   }
 
-  &__highlight {
-    margin-right: 5px;
-  }
-
   &__highlight-icon {
     color: var.$grey-5;
     width: 12px;

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

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 .annotation-license {
   @include var.font-small;
   border-top: 1px solid var.$grey-3;
   padding-top: 0.5em;
-  margin: 1em 0;
 
   &__link {
-    display: flex;
+    @include layout.row;
     color: var.$color-text-light;
   }
 

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

 @use "../../mixins/buttons";
 @use "../../mixins/forms";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .annotation-publish-control {
-  display: flex;
+  @include layout.row;
 
   // A split button with a primary submit on the left and a drop-down menu
   // of related options to the right
   .annotation-publish-control__btn {
+    @include layout.row;
     $text-color: var.$color-text-inverted;
     $default-background-color: var.$grey-mid;
     $hover-background-color: var.$grey-6;
@@ -17,7 +19,6 @@
     $arrow-indicator-width: 26px;
 
     height: $height;
-    display: flex;
     position: relative;
     margin-right: 1em;
 
@@ -57,8 +58,7 @@
     // dropdown arrow which reveals the button's associated menu
     // when clicked
     &-dropdown-arrow {
-      display: flex;
-      align-items: center;
+      @include layout.row(center);
       right: 0px;
       top: 0px;
 

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

 @use '../../variables' as var;
 
 .annotation-quote {
-  margin: 1em 0;
-
   &.is-orphan {
     text-decoration: line-through;
   }

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

 @use '../../mixins/buttons';
+@use '../../mixins/layout';
 @use '../../mixins/links';
 @use '../../mixins/panel';
 @use "../../variables" as var;
@@ -17,7 +18,7 @@
   cursor: default;
 
   &__input {
-    display: flex;
+    @include layout.row;
   }
 
   &__icon-button {

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

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 .annotation-share-info {
-  display: flex;
-  align-items: baseline;
+  @include layout.row($align: baseline);
 
   &__group,
   &__private {
-    display: flex;
-    align-items: baseline;
+    @include layout.row($align: baseline);
     color: var.$color-text-light;
   }
 
-  &__group-info {
-    margin-right: 5px;
-  }
-
-  &__private-info {
-    margin-right: 5px;
-  }
-
   &__icon {
+    // This margin is currently needed because the icon is within an `a` element
     margin-right: 5px;
     width: 10px;
     height: 10px;

src/styles/sidebar/components/annotation.scss

 @use "../../mixins/buttons";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 // FIXME These hover-related rules should live elsewhere
@@ -21,6 +22,8 @@
 }
 
 .annotation {
+  @include layout.vertical-rhythm;
+
   &.is-highlighted {
     // Slightly darken the bodies of highlighted annotations to
     // make them stand out next to others, which will have the `is-dimmed` state
@@ -43,7 +46,7 @@
   }
 
   &__controls {
-    display: flex;
+    @include layout.row;
   }
 
   &__actions {
@@ -51,14 +54,7 @@
   }
 
   &__form-actions {
-    display: flex;
-    flex-direction: row;
-    margin-bottom: 10px;
-  }
-
-  &__footer {
-    color: var.$color-text-light;
-    margin-top: 1em;
+    @include layout.row;
   }
 }
 

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

 @use "../../mixins/focus";
 @use "../../mixins/forms";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 // A dark grey button used for the primary action
 // in a form
 .focused-mode-header {
-  display: flex;
-  align-items: center;
+  @include layout.row($align: center);
   position: relative;
 
   background-color: var.$color-background;
@@ -18,8 +18,7 @@
 
   &__btn {
     @include forms.primary-action-btn;
-    display: flex;
-    align-items: center;
+    @include layout.row($align: center);
     margin-left: auto;
     height: 30px;
     padding-left: 10px;

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

 @use "../../mixins/buttons";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .help-panel {
@@ -33,8 +34,7 @@
 
   &__sub-panel-link {
     @include buttons.reset-native-btn-styles;
-    display: flex;
-    align-items: center;
+    @include layout.row($align: center);
     margin-left: auto;
     color: var.$color-brand;
 
@@ -46,8 +46,7 @@
   }
 
   &-tabs {
-    display: flex;
-    align-items: center;
+    @include layout.row($align: center);
 
     &__tab {
       @include var.font-big;

src/styles/sidebar/components/logged-out-message.scss

 @use "../../mixins/buttons";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .logged-out-message {
+  @include layout.column;
   margin: 25px auto;
   width: 70%;
   text-align: center;
   color: var.$color-text;
-  display: flex;
-  flex-direction: column;
 }
 
 .logged-out-message__link {
@@ -19,9 +19,8 @@
 }
 
 .logged-out-message__logo {
+  @include layout.row(center);
   margin-top: 25px;
-  display: flex;
-  justify-content: center;
 }
 
 .logged-out-message__logo-icon {

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

 @use "../../mixins/buttons";
 @use "../../mixins/forms";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 $toolbar-border: 0.1em solid var.$grey-3;
 
 .markdown-editor__toolbar {
-  display: flex;
-  flex-direction: row;
+  @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.
@@ -23,9 +23,7 @@ $toolbar-border: 0.1em solid var.$grey-3;
 
 .markdown-editor__toolbar-button {
   @include buttons.reset-native-btn-styles;
-  display: flex;
-  justify-content: center;
-  align-items: center;
+  @include layout.row(center, center);
   appearance: none;
   min-width: 24px;
   min-height: 24px;
@@ -54,8 +52,7 @@ $toolbar-border: 0.1em solid var.$grey-3;
 }
 
 .markdown-editor__toolbar-help-link {
-  display: flex;
-  align-items: center;
+  @include layout.row($align: center);
   margin-bottom: 2px; // Tweak to align help icon better with adjacent buttons
 }
 

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

 @use "sass:color";
 @use "../../mixins/focus";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 $menu-item-padding: 10px;
@@ -16,11 +17,11 @@ a.menu-item:hover {
 
 .menu-item {
   @include focus.outline-on-keyboard-focus;
+  @include layout.row($align: center);
   align-self: stretch;
-  display: flex;
   flex-grow: 1;
+
   appearance: none;
-  align-items: center;
   padding-left: $menu-item-padding;
   color: var.$color-text;
   border: none;
@@ -91,16 +92,12 @@ a.menu-item:hover {
 }
 
 .menu-item__label {
+  @include layout.row($align: center);
   align-self: stretch;
 
-  display: flex;
-  flex-direction: row;
-  align-items: center;
-
   color: inherit;
   white-space: nowrap;
   flex-grow: 1;
-  flex-shrink: 1;
   font-weight: inherit;
 
   padding-right: $menu-item-padding;
@@ -113,16 +110,14 @@ a.menu-item:hover {
 // Toggle button used to expand or collapse the submenu associated with a menu
 // item.
 .menu-item__toggle {
-  color: var.$grey-5;
-  display: flex;
-  flex-direction: column;
-  justify-content: center;
-  align-items: center;
+  @include layout.column(center, center);
   align-self: stretch;
   width: 40px;
   padding: 0;
   height: 40px;
 
+  color: var.$grey-5;
+
   // Add a wide transparent border to provide a large-enough hit target (~40px),
   // larger than the visual size of the button (~20px).
   background-color: var.$grey-1;

src/styles/sidebar/components/menu.scss

 @use "../../mixins/focus";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .menu {
@@ -30,8 +31,7 @@
 }
 
 .menu__toggle-wrapper {
-  display: flex;
-  align-items: center;
+  @include layout.row($align: center);
   height: 100%;
 }
 .menu__toggle-icon {

src/styles/sidebar/components/moderation-banner.scss

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 @at-root {
   // Horizontal margin between edge of annotation card and content
   $h-padding: 15px;
 
   .moderation-banner {
+    @include layout.row;
     position: relative;
-
     color: white;
-    display: flex;
-    flex-direction: row;
     font-weight: bold;
   }
 

src/styles/sidebar/components/new-note.scss

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 .new-note-button {
-  display: flex;
-  justify-content: flex-end;
-  margin: 0 1em 1em 0;
+  @include layout.row(right);
 }

src/styles/sidebar/components/search-input.scss

 @use "../../mixins/forms";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .search-input__form {
-  display: flex;
-  flex-flow: row nowrap;
-
+  @include layout.row;
   position: relative;
   color: var.$color-text;
 }

src/styles/sidebar/components/search-status-bar.scss

 @use '../../mixins/buttons';
+@use '../../mixins/layout';
 
 .search-status-bar {
-  display: flex;
-  align-items: center;
+  @include layout.row($align: center);
   margin-bottom: 10px;
 }
 

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

 @use "../../mixins/focus";
 @use "../../mixins/buttons";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
-.selection-tabs {
-  display: flex;
-  flex-direction: row;
-
-  &:hover {
-    color: var.$color-text;
-  }
-
+.selection-tabs-container {
+  @include layout.vertical-rhythm;
+  // FIXME: This should be a margin, and it should be handled by the parent,
+  // but needs to be considered carefully because applying vertical rhythm to
+  // this component's parent messes with the calculations in the virtualized
+  // thread list. Needs another pass. Note also that it is `10px` (and looks
+  // unbalanced at the standard vertical rhythm size of `1em`)
   padding-bottom: 10px;
 }
 
+.selection-tabs {
+  @include layout.row;
+  @include layout.horizontal-rhythm(20px);
+}
+
 .selection-tabs__icon {
   color: var.$grey-mid;
   margin: 0 0.25em;
@@ -27,7 +32,6 @@
   @include focus.outline-on-keyboard-focus;
 
   color: var.$color-text;
-  margin-right: 20px;
   cursor: pointer;
   min-width: 85px;
   min-height: 18px;

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

 @use "../../mixins/forms";
 @use "../../mixins/buttons";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .tag-editor {
@@ -11,13 +12,13 @@
   }
 
   &__tags {
-    display: flex;
+    @include layout.row;
     flex-wrap: wrap;
     margin: 0.5em 0;
   }
 
   &__item {
-    display: flex;
+    @include layout.row;
     margin: 0.25em 0.5em 0.25em 0;
   }
 

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

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 .tag-list {
-  margin: 1em 0;
-  display: flex;
+  @include layout.row;
   flex-wrap: wrap;
+  margin: 1em 0;
 
   &__item {
     margin: 0.25em 0.5em 0.25em 0;

src/styles/sidebar/components/thread.scss

 @use "../../mixins/buttons";
+@use "../../mixins/layout";
 @use "../../variables" as var;
 
 .thread {
-  display: flex;
+  @include layout.row;
 
   &--reply {
     margin-top: 0.5em;
   }
 
-  // Conserve space for deeper usable nesting
+  &--reply &__content {
+    margin-left: 1em;
+  }
+
   &__children {
-    margin-left: -0.75em;
+    margin-left: -1em;
+  }
+
+  &__unavailable-message {
+    margin: 0 1em;
   }
 
   // Left "channel" of thread

src/styles/sidebar/components/toast-messages.scss

 @use "../../mixins/panel";
+@use "../../mixins/layout";
 @use "../mixins/responsive";
 @use "../../variables" as var;
 
@@ -36,7 +37,7 @@
 
 .toast-message {
   @include panel.panel;
-  display: flex;
+  @include layout.row;
   position: relative;
   width: 100%;
   box-shadow: 0px 2px 3px 0px rgba(0, 0, 0, 0.15);
@@ -56,8 +57,7 @@
   }
 
   &__type {
-    display: flex;
-    align-items: center;
+    @include layout.row($align: center);
     padding: 1em;
     color: white;
   }

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

@@ -76,22 +76,18 @@
 }
 
 .top-bar__login-links {
-  display: flex;
+  @include layout.row;
   flex-shrink: 0;
 }
 
 .top-bar__inner {
   @include layout.sidebar-content;
+  @include layout.row(right, center);
 
   // the edges of the top-bar's contents should be aligned
   // with the edges of annotation cards displayed below
   $h-padding: 9px;
 
-  display: flex;
-  flex-flow: row nowrap;
-  align-items: center;
-  justify-content: flex-end;
-
   padding-left: $h-padding;
   padding-right: $h-padding;
 

src/styles/sidebar/components/version-info.scss

 @use "../../variables" as var;
+@use "../../mixins/layout";
 
 .version-info {
   margin-top: 0.5em;
@@ -22,8 +23,7 @@
   }
 
   &__actions {
-    display: flex;
-    justify-content: center;
+    @include layout.row(center);
     padding-bottom: 0.5em;
   }
 }

src/styles/util.scss

 @use "./mixins/a11y";
+@use "./mixins/layout";
 
 // Utility classes for layout
 .u-stretch {
@@ -6,12 +7,7 @@
 }
 
 .u-layout-row {
-  display: flex;
-  flex-direction: row;
-}
-
-.u-strong {
-  font-weight: bold;
+  @include layout.row;
 }
 
 .u-screen-reader-only {