Merge pull request #1598 from hypothesis/action-button

Add and use new `ActionButton` component

src/sidebar/components/action-button.js

+'use strict';
+
+const classnames = require('classnames');
+const propTypes = require('prop-types');
+const { createElement } = require('preact');
+
+const SvgIcon = require('./svg-icon');
+
+/**
+ * A button, with (required) text label and an (optional) icon
+ */
+function ActionButton({
+  icon = '',
+  isActive = false,
+  isPrimary = false,
+  label,
+  onClick = () => null,
+  useCompactStyle = false,
+}) {
+  return (
+    <button
+      className={classnames('action-button', {
+        'action-button--compact': useCompactStyle,
+        'action-button--primary': isPrimary,
+        'is-active': isActive,
+      })}
+      onClick={onClick}
+      aria-pressed={isActive}
+      title={label}
+    >
+      {icon && (
+        <SvgIcon
+          name={icon}
+          className={classnames('action-button__icon', {
+            'action-button__icon--compact': useCompactStyle,
+            'action-button__icon--primary': isPrimary,
+          })}
+        />
+      )}
+      <span className="action-button__label">{label}</span>
+    </button>
+  );
+}
+
+ActionButton.propTypes = {
+  /** The name of the SVGIcon to render */
+  icon: propTypes.string,
+  /** Is this button currently in an "active" or in an "on" state? */
+  isActive: propTypes.bool,
+  /**
+   * Does this button represent the "primary" action available? If so,
+   * differentiating styles will be applied.
+   */
+  isPrimary: propTypes.bool,
+  /** a label used for the `title` and `aria-label` attributes */
+  label: propTypes.string.isRequired,
+  /** callback for button clicks */
+  onClick: propTypes.func,
+  /** Allows a variant of this type of button that takes up less space */
+  useCompactStyle: propTypes.bool,
+};
+
+module.exports = ActionButton;

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

@@ -6,9 +6,9 @@ const { createElement } = require('preact');
 const { applyTheme } = require('../util/theme');
 const { withServices } = require('../util/service-context');
 
+const ActionButton = require('./action-button');
 const Menu = require('./menu');
 const MenuItem = require('./menu-item');
-const SvgIcon = require('./svg-icon');
 
 /**
  * Render a compound control button for publishing (saving) an annotation:
@@ -75,14 +75,12 @@ function AnnotationPublishControl({
           />
         </Menu>
       </div>
-      <button
-        className="action-button"
+      <ActionButton
+        icon="cancel"
+        label="Cancel"
         onClick={onCancel}
-        title="Cancel changes to this annotation"
-      >
-        <SvgIcon name="cancel" className="action-button__icon--compact" />
-        Cancel
-      </button>
+        useCompactStyle
+      />
     </div>
   );
 }

src/sidebar/components/search-status-bar.js

@@ -8,6 +8,8 @@ const { withServices } = require('../util/service-context');
 const uiConstants = require('../ui-constants');
 const useStore = require('../store/use-store');
 
+const ActionButton = require('./action-button');
+
 /**
  * Of the annotations in the thread `annThread`, how many
  * are currently `visible` in the browser (sidebar)?
@@ -143,22 +145,17 @@ function SearchStatusBar({ rootThread }) {
     })(),
   };
 
-  const btnProps = {
-    className: 'primary-action-btn primary-action-btn--short',
-    onClick: actions.clearSelection,
-  };
-
   return (
     <div>
       {modes.filtered && (
         <div className="search-status-bar">
-          <button
-            title="Clear the search filter and show all annotations"
-            {...btnProps}
-          >
-            <i className="primary-action-btn__icon h-icon-close" />
-            Clear search
-          </button>
+          <ActionButton
+            icon="cancel"
+            label="Clear search"
+            onClick={actions.clearSelection}
+            isPrimary
+            useCompactStyle
+          />
           <span className="search-status-bar__filtered-text">
             {modeText.filtered}
           </span>
@@ -173,14 +170,12 @@ function SearchStatusBar({ rootThread }) {
       )}
       {modes.selected && (
         <div className="search-status-bar">
-          <button
-            title="Clear the selection and show all annotations"
-            {...btnProps}
-          >
-            <span className="search-status-bar__selected-text">
-              {modeText.selected}
-            </span>
-          </button>
+          <ActionButton
+            label={modeText.selected}
+            onClick={actions.clearSelection}
+            isPrimary
+            useCompactStyle
+          />
         </div>
       )}
     </div>

src/sidebar/components/sidebar-panel.js

@@ -7,8 +7,8 @@ const scrollIntoView = require('scroll-into-view');
 
 const useStore = require('../store/use-store');
 
+const ActionButton = require('./action-button');
 const Slider = require('./slider');
-const SvgIcon = require('./svg-icon');
 
 /**
  * Base component for a sidebar panel.
@@ -47,14 +47,12 @@ function SidebarPanel({ children, panelName, title, onActiveChanged }) {
         <div className="sidebar-panel__header">
           <div className="sidebar-panel__title u-stretch">{title}</div>
           <div>
-            <button
-              className="sidebar-panel__close-btn"
+            <ActionButton
+              icon="cancel"
+              label="Close"
               onClick={closePanel}
-              aria-label="close panel"
-            >
-              <SvgIcon name="cancel" className="action-button__icon--compact" />
-              Close
-            </button>
+              useCompactStyle
+            />
           </div>
         </div>
         <div className="sidebar-panel__content">{children}</div>

src/sidebar/components/test/action-button-test.js

+'use strict';
+
+const { createElement } = require('preact');
+const { mount } = require('enzyme');
+
+const ActionButton = require('../action-button');
+const mockImportedComponents = require('./mock-imported-components');
+
+describe('ActionButton', () => {
+  let fakeOnClick;
+
+  function createComponent(props = {}) {
+    return mount(
+      <ActionButton
+        icon="fakeIcon"
+        label="My Action"
+        onClick={fakeOnClick}
+        {...props}
+      />
+    );
+  }
+
+  beforeEach(() => {
+    fakeOnClick = sinon.stub();
+    ActionButton.$imports.$mock(mockImportedComponents());
+  });
+
+  afterEach(() => {
+    ActionButton.$imports.$restore();
+  });
+
+  it('renders `SvgIcon` if icon property set', () => {
+    const wrapper = createComponent();
+    assert.equal(wrapper.find('SvgIcon').prop('name'), 'fakeIcon');
+  });
+
+  it('does not render `SvgIcon` if no icon property set', () => {
+    const wrapper = createComponent({ icon: undefined });
+    assert.isFalse(wrapper.find('SvgIcon').exists());
+  });
+
+  it('invokes `onClick` callback when pressed', () => {
+    const wrapper = createComponent();
+    wrapper.find('button').simulate('click');
+    assert.calledOnce(fakeOnClick);
+  });
+
+  it('adds compact styles if `useCompactStyle` is `true`', () => {
+    const wrapper = createComponent({ useCompactStyle: true });
+
+    assert.isTrue(wrapper.find('button').hasClass('action-button--compact'));
+  });
+
+  context('in active state (`isActive` is `true`)', () => {
+    it('adds `is-active` className', () => {
+      const wrapper = createComponent({ isActive: true });
+
+      assert.isTrue(wrapper.find('button').hasClass('is-active'));
+    });
+
+    it('sets `aria-pressed` attribute on button', () => {
+      const wrapper = createComponent({ isActive: true });
+
+      assert.isTrue(wrapper.find('button').prop('aria-pressed'));
+    });
+  });
+});

src/sidebar/components/test/annotation-publish-control-test.js

@@ -191,10 +191,9 @@ describe('AnnotationPublishControl', () => {
       const wrapper = createAnnotationPublishControl({
         onCancel: fakeOnCancel,
       });
-      const cancelBtn = wrapper.find(
-        '[title="Cancel changes to this annotation"]'
-      );
-      cancelBtn.prop('onClick')();
+      const cancelBtn = wrapper.find('ActionButton');
+
+      cancelBtn.props().onClick();
 
       assert.calledOnce(fakeOnCancel);
     });

src/sidebar/components/test/search-status-bar-test.js

@@ -113,8 +113,8 @@ describe('SearchStatusBar', () => {
 
         const wrapper = createComponent({});
 
-        const buttonText = wrapper.find('button').text();
-        assert.equal(buttonText, 'Clear search');
+        const button = wrapper.find('ActionButton');
+        assert.equal(button.props().label, 'Clear search');
 
         const searchResultsText = wrapper.find('span').text();
         assert.equal(searchResultsText, test.expectedText);
@@ -251,14 +251,10 @@ describe('SearchStatusBar', () => {
 
           const wrapper = createComponent({});
 
-          const button = wrapper.find('button');
-          const selectedText = wrapper.find(
-            '.search-status-bar__selected-text'
-          );
+          const button = wrapper.find('ActionButton');
 
           assert.isTrue(button.exists());
-          assert.equal(button.text(), test.buttonText);
-          assert.isTrue(selectedText.exists());
+          assert.equal(button.props().label, test.buttonText);
         });
       });
     });
@@ -285,14 +281,10 @@ describe('SearchStatusBar', () => {
 
           const wrapper = createComponent({});
 
-          const button = wrapper.find('button');
-          const selectedText = wrapper.find(
-            '.search-status-bar__selected-text'
-          );
+          const button = wrapper.find('ActionButton');
 
           assert.isTrue(button.exists());
-          assert.equal(button.text(), test.buttonText);
-          assert.isTrue(selectedText.exists());
+          assert.equal(button.props().label, test.buttonText);
         });
       });
     });
@@ -315,16 +307,11 @@ describe('SearchStatusBar', () => {
 
         const wrapper = createComponent({});
 
-        const button = wrapper.find('button');
-        const selectedText = wrapper.find('.search-status-bar__selected-text');
-        const filteredText = wrapper.find('.search-status-bar__filtered-text');
+        const button = wrapper.find('ActionButton');
 
         assert.isTrue(button.exists());
-        assert.isFalse(selectedText.exists());
-        assert.isTrue(filteredText.exists());
+        assert.equal(button.props().label, 'Clear search');
       });
     });
   });
-
-  // });
 });

src/sidebar/components/test/sidebar-panel-test.js

@@ -45,7 +45,11 @@ describe('SidebarPanel', () => {
 
   it('closes the panel when close button is clicked', () => {
     const wrapper = createSidebarPanel({ panelName: 'flibberty' });
-    wrapper.find('button').simulate('click');
+
+    wrapper
+      .find('ActionButton')
+      .props()
+      .onClick();
 
     assert.calledWith(fakeStore.toggleSidebarPanel, 'flibberty', false);
   });

src/sidebar/components/version-info.js

@@ -6,7 +6,7 @@ const { createElement } = require('preact');
 const { copyText } = require('../util/copy-to-clipboard');
 const { withServices } = require('../util/service-context');
 
-const SvgIcon = require('./svg-icon');
+const ActionButton = require('./action-button');
 
 /**
  * Display current client version info
@@ -38,13 +38,11 @@ function VersionInfo({ flash, versionData }) {
         <dd className="version-info__value">{versionData.timestamp}</dd>
       </dl>
       <div className="version-info__actions">
-        <button
-          className="version-info__copy-btn action-button"
+        <ActionButton
+          label="Copy version details"
           onClick={copyVersionData}
-        >
-          <SvgIcon name="copy" className="action-button__icon" />
-          Copy version details
-        </button>
+          icon="copy"
+        />
       </div>
     </div>
   );

src/styles/mixins/buttons.scss

@@ -43,13 +43,36 @@
   color: var.$grey-5;
   font-weight: 700;
 
+  &:hover {
+    background-color: var.$grey-2;
+  }
+
+  &__label {
+    padding-left: 0.25em;
+    padding-right: 0.25em;
+  }
+
   &__icon {
     color: var.$grey-5;
     margin: $icon-margin;
+
+    &--compact {
+      margin: 0;
+    }
+
+    &--primary {
+      color: var.$grey-1;
+    }
   }
 
-  &:hover {
-    background-color: var.$grey-2;
+  &--primary {
+    color: var.$grey-1;
+    background-color: var.$grey-mid;
+
+    &:hover {
+      color: var.$grey-1;
+      background-color: var.$grey-6;
+    }
   }
 }
 

src/styles/sidebar/components/action-button.scss

@@ -2,8 +2,4 @@
 
 .action-button {
   @include buttons.action-button;
-
-  &__icon--compact {
-    margin: 0;
-  }
 }

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

 .search-status-bar {
+  display: flex;
+  align-items: center;
   margin-bottom: 10px;
 }