Use `IconButton` in several components

- Eliminate custom sub-components that fulfill the same task
- Replace `AnnotationActionButton` with `IconButton` where used
- Consolidate and adjust styling

src/sidebar/components/annotation-action-bar.js

@@ -6,11 +6,11 @@ const { createElement } = require('preact');
 const { withServices } = require('../util/service-context');
 const { isShareable, shareURI } = require('../util/annotation-sharing');
 
-const AnnotationActionButton = require('./annotation-action-button');
 const AnnotationShareControl = require('./annotation-share-control');
+const IconButton = require('./icon-button');
 
 /**
- * A collection of `AnnotationActionButton`s in the footer area of an annotation.
+ * A collection of `IconButton`s in the footer area of an annotation.
  */
 function AnnotationActionBar({
   annotation,
@@ -45,16 +45,12 @@ function AnnotationActionBar({
   return (
     <div className="annotation-action-bar">
       {showEditAction && (
-        <AnnotationActionButton icon="edit" label="Edit" onClick={onEdit} />
+        <IconButton icon="edit" title="Edit" onClick={onEdit} />
       )}
       {showDeleteAction && (
-        <AnnotationActionButton
-          icon="trash"
-          label="Delete"
-          onClick={onDelete}
-        />
+        <IconButton icon="trash" title="Delete" onClick={onDelete} />
       )}
-      <AnnotationActionButton icon="reply" label="Reply" onClick={onReply} />
+      <IconButton icon="reply" title="Reply" onClick={onReply} />
       {showShareAction && (
         <AnnotationShareControl
           group={annotationGroup}
@@ -63,17 +59,17 @@ function AnnotationActionBar({
         />
       )}
       {showFlagAction && !annotation.flagged && (
-        <AnnotationActionButton
+        <IconButton
           icon="flag"
-          label="Report this annotation to moderators"
+          title="Report this annotation to moderators"
           onClick={onFlag}
         />
       )}
       {showFlagAction && annotation.flagged && (
-        <AnnotationActionButton
+        <IconButton
           isActive={true}
           icon="flag--active"
-          label="Annotation has been reported to the moderators"
+          title="Annotation has been reported to the moderators"
         />
       )}
     </div>

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

@@ -8,7 +8,7 @@ const useElementShouldClose = require('./hooks/use-element-should-close');
 const { copyText } = require('../util/copy-to-clipboard');
 const { withServices } = require('../util/service-context');
 
-const AnnotationActionButton = require('./annotation-action-button');
+const IconButton = require('./icon-button');
 const ShareLinks = require('./share-links');
 const SvgIcon = require('./svg-icon');
 
@@ -84,12 +84,7 @@ function AnnotationShareControl({
 
   return (
     <div className="annotation-share-control" ref={shareRef}>
-      <AnnotationActionButton
-        icon="share"
-        isDisabled={false}
-        label="Share"
-        onClick={toggleSharePanel}
-      />
+      <IconButton icon="share" title="Share" onClick={toggleSharePanel} />
       {isOpen && (
         <div className="annotation-share-panel">
           <div className="annotation-share-panel__header">

src/sidebar/components/icon-button.js

@@ -15,11 +15,13 @@ function IconButton({
   isActive = false,
   title,
   onClick = () => null,
+  useCompactStyle = false,
 }) {
   return (
     <button
       className={classnames('icon-button', className, {
         'is-active': isActive,
+        'icon-button--compact': useCompactStyle,
       })}
       onClick={onClick}
       aria-pressed={isActive}
@@ -31,7 +33,10 @@ function IconButton({
 }
 
 IconButton.propTypes = {
-  /** Optional additional class(es) to apply to the component element */
+  /** Optional additional class(es) to apply to the component element
+   * NB: Padding is controlled by the component's styles. Use
+   * `useCompactStyle` for tighter padding.
+   */
   className: propTypes.string,
   /** The name of the SVGIcon to render */
   icon: propTypes.string.isRequired,
@@ -41,6 +46,8 @@ IconButton.propTypes = {
   title: propTypes.string.isRequired,
   /** optional callback for clicks */
   onClick: propTypes.func,
+  /** tighten padding and make icon button fit in smaller space */
+  useCompactStyle: propTypes.bool,
 };
 
 module.exports = IconButton;

src/sidebar/components/search-input.js

@@ -7,6 +7,7 @@ const propTypes = require('prop-types');
 
 const useStore = require('../store/use-store');
 
+const IconButton = require('./icon-button');
 const Spinner = require('./spinner');
 
 /**
@@ -60,14 +61,13 @@ function SearchInput({ alwaysExpanded, query, onSearch }) {
         onInput={e => setPendingQuery(e.target.value)}
       />
       {!isLoading && (
-        <button
-          type="button"
-          className="search-input__icon top-bar__btn"
-          title="Search"
+        <IconButton
+          className="search-input__icon-button top-bar__icon-button"
+          icon="search"
           onClick={() => input.current.focus()}
-        >
-          <i className="h-icon-search" />
-        </button>
+          title="Search annotations"
+          useCompactStyle
+        />
       )}
       {isLoading && <Spinner className="top-bar__btn" title="Loading…" />}
     </form>

src/sidebar/components/sort-menu.js

@@ -4,6 +4,7 @@ const { createElement } = require('preact');
 
 const useStore = require('../store/use-store');
 
+const IconButton = require('./icon-button');
 const Menu = require('./menu');
 const MenuItem = require('./menu-item');
 
@@ -35,7 +36,14 @@ function SortMenu() {
     );
   });
 
-  const menuLabel = <i className="h-icon-sort top-bar__btn" />;
+  const menuLabel = (
+    <IconButton
+      className="top-bar__icon-button"
+      icon="sort"
+      title="Sort annotations"
+      useCompactStyle
+    />
+  );
 
   return (
     <div className="sort-menu">

src/sidebar/components/test/annotation-action-bar-test.js

@@ -52,7 +52,7 @@ describe('AnnotationActionBar', () => {
   };
 
   const getButton = (wrapper, iconName) => {
-    return wrapper.find('AnnotationActionButton').filter({ icon: iconName });
+    return wrapper.find('IconButton').filter({ icon: iconName });
   };
 
   beforeEach(() => {

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

@@ -33,7 +33,7 @@ describe('AnnotationShareControl', () => {
   function openElement(wrapper) {
     act(() => {
       wrapper
-        .find('AnnotationActionButton')
+        .find('IconButton')
         .props()
         .onClick();
     });
@@ -92,7 +92,7 @@ describe('AnnotationShareControl', () => {
 
     act(() => {
       wrapper
-        .find('AnnotationActionButton')
+        .find('IconButton')
         .props()
         .onClick();
     });

src/sidebar/components/test/top-bar-test.js

@@ -54,12 +54,9 @@ describe('TopBar', () => {
     TopBar.$imports.$restore();
   });
 
-  function applyUpdateBtn(wrapper) {
-    return wrapper.find('.top-bar__btn--refresh');
-  }
-
-  function helpBtn(wrapper) {
-    return wrapper.find('.top-bar__help-btn');
+  // Helper to retrieve an `IconButton` by icon name, for convenience
+  function getButton(wrapper, iconName) {
+    return wrapper.find('IconButton').filter({ icon: iconName });
   }
 
   function createTopBar(props = {}) {
@@ -79,21 +76,23 @@ describe('TopBar', () => {
   it('shows the pending update count', () => {
     fakeStore.pendingUpdateCount.returns(1);
     const wrapper = createTopBar();
-    const applyBtn = applyUpdateBtn(wrapper);
+    const applyBtn = getButton(wrapper, 'refresh');
     assert.isTrue(applyBtn.exists());
   });
 
   it('does not show the pending update count when there are no updates', () => {
     const wrapper = createTopBar();
-    const applyBtn = applyUpdateBtn(wrapper);
+    const applyBtn = getButton(wrapper, 'refresh');
     assert.isFalse(applyBtn.exists());
   });
 
   it('applies updates when clicked', () => {
     fakeStore.pendingUpdateCount.returns(1);
     const wrapper = createTopBar();
-    const applyBtn = applyUpdateBtn(wrapper);
-    applyBtn.simulate('click');
+    const applyBtn = getButton(wrapper, 'refresh');
+
+    applyBtn.props().onClick();
+
     assert.called(fakeStreamer.applyPendingUpdates);
   });
 
@@ -101,8 +100,10 @@ describe('TopBar', () => {
     context('no help service handler configured in services (default)', () => {
       it('toggles Help Panel on click', () => {
         const wrapper = createTopBar();
-        const help = helpBtn(wrapper);
-        help.simulate('click');
+        const helpButton = getButton(wrapper, 'help');
+
+        helpButton.props().onClick();
+
         assert.calledWith(fakeStore.toggleSidebarPanel, uiConstants.PANEL_HELP);
       });
 
@@ -114,20 +115,22 @@ describe('TopBar', () => {
           },
         });
         const wrapper = createTopBar();
-        const help = helpBtn(wrapper);
+        const helpButton = getButton(wrapper, 'help');
 
         wrapper.update();
 
-        assert.isTrue(help.hasClass('is-active'));
-        assert.isOk(help.prop('aria-pressed'));
+        assert.isTrue(helpButton.props().isActive);
       });
 
       context('help service handler configured in services', () => {
         it('fires a bridge event if help clicked and service is configured', () => {
           fakeServiceConfig.returns({ onHelpRequestProvided: true });
           const wrapper = createTopBar();
-          const help = helpBtn(wrapper);
-          help.simulate('click');
+
+          const helpButton = getButton(wrapper, 'help');
+
+          helpButton.props().onClick();
+
           assert.equal(fakeStore.toggleSidebarPanel.callCount, 0);
           assert.calledWith(fakeBridge.call, bridgeEvents.HELP_REQUESTED);
         });
@@ -208,7 +211,10 @@ describe('TopBar', () => {
 
   it('toggles the share annotations panel when "Share" is clicked', () => {
     const wrapper = createTopBar();
-    wrapper.find('.top-bar__share-btn').simulate('click');
+    const shareButton = getButton(wrapper, 'share');
+
+    shareButton.props().onClick();
+
     assert.calledWith(
       fakeStore.toggleSidebarPanel,
       uiConstants.PANEL_SHARE_ANNOTATIONS
@@ -221,11 +227,10 @@ describe('TopBar', () => {
         activePanelName: uiConstants.PANEL_SHARE_ANNOTATIONS,
       },
     });
-
     const wrapper = createTopBar();
-    const shareEl = wrapper.find('.top-bar__share-btn');
+    const shareButton = getButton(wrapper, 'share');
 
-    assert.include(shareEl.prop('className'), 'is-active');
+    assert.isTrue(shareButton.prop('isActive'));
   });
 
   it('displays search input in the sidebar', () => {

src/sidebar/components/top-bar.js

@@ -13,48 +13,12 @@ const { withServices } = require('../util/service-context');
 const uiConstants = require('../ui-constants');
 
 const GroupList = require('./group-list');
+const IconButton = require('./icon-button');
 const SearchInput = require('./search-input');
 const StreamSearchInput = require('./stream-search-input');
 const SortMenu = require('./sort-menu');
-const SvgIcon = require('./svg-icon');
 const UserMenu = require('./user-menu');
 
-/**
- * Reusable component to render a button for toggling a sidebar panel.
- * Takes an `onClick` callback, as panel toggles are sometimes more complex than
- * just opening and closing a panel.
- */
-function TogglePanelButton({ panelName, iconName, title, onClick }) {
-  const currentActivePanel = useStore(
-    store => store.getState().sidebarPanels.activePanelName
-  );
-  const isActive = currentActivePanel === panelName;
-  return (
-    <button
-      className={classnames('top-bar__btn', `top-bar__${iconName}-btn`, {
-        'is-active': isActive,
-      })}
-      onClick={onClick}
-      title={title}
-      aria-pressed={isActive ? 'true' : 'false'}
-    >
-      <SvgIcon
-        name={iconName}
-        className={classnames(`top-bar__{$iconName}-icon`)}
-      />
-    </button>
-  );
-}
-
-TogglePanelButton.propTypes = {
-  /** The panel's string name as defined in `uiConstants` */
-  panelName: propTypes.string.isRequired,
-  iconName: propTypes.string.isRequired,
-  title: propTypes.string.isRequired,
-  /** callback */
-  onClick: propTypes.func.isRequired,
-};
-
 /**
  * The toolbar which appears at the top of the sidebar providing actions
  * to switch groups, view account information, sort/filter annotations etc.
@@ -86,6 +50,10 @@ function TopBar({
     togglePanelFn(uiConstants.PANEL_SHARE_ANNOTATIONS);
   };
 
+  const currentActivePanel = useStore(
+    store => store.getState().sidebarPanels.activePanelName
+  );
+
   /**
    * Open the help panel, or, if a service callback is configured to handle
    * help requests, fire a relevant event instead
@@ -130,11 +98,13 @@ function TopBar({
         <div className="top-bar__inner content">
           <StreamSearchInput />
           <div className="top-bar__expander" />
-          <TogglePanelButton
-            panelName={uiConstants.PANEL_HELP}
-            iconName="help"
-            title="Help"
+          <IconButton
+            className="top-bar__icon-button"
+            icon="help"
+            isActive={currentActivePanel === uiConstants.PANEL_HELP}
             onClick={requestHelp}
+            title="Help"
+            useCompactStyle
           />
           {loginControl}
         </div>
@@ -145,31 +115,37 @@ function TopBar({
           <GroupList className="GroupList" auth={auth} />
           <div className="top-bar__expander" />
           {pendingUpdateCount > 0 && (
-            <button
-              className="top-bar__btn top-bar__btn--refresh"
+            <IconButton
+              className="top-bar__icon-button top-bar__icon-button--refresh"
+              icon="refresh"
               onClick={applyPendingUpdates}
               title={`Show ${pendingUpdateCount} new/updated ${
                 pendingUpdateCount === 1 ? 'annotation' : 'annotations'
               }`}
-            >
-              <SvgIcon className="top-bar__apply-icon" name="refresh" />
-            </button>
+              useCompactStyle
+            />
           )}
           <SearchInput query={filterQuery} onSearch={setFilterQuery} />
           <SortMenu />
           {showSharePageButton && (
-            <TogglePanelButton
-              panelName={uiConstants.PANEL_SHARE_ANNOTATIONS}
-              iconName="share"
-              title="Share annotations on this page"
+            <IconButton
+              className="top-bar__icon-button"
+              icon="share"
+              isActive={
+                currentActivePanel === uiConstants.PANEL_SHARE_ANNOTATIONS
+              }
               onClick={toggleSharePanel}
+              title="Share annotations on this page"
+              useCompactStyle
             />
           )}
-          <TogglePanelButton
-            panelName={uiConstants.PANEL_HELP}
-            iconName="help"
-            title="Help"
+          <IconButton
+            className="top-bar__icon-button"
+            icon="help"
+            isActive={currentActivePanel === uiConstants.PANEL_HELP}
             onClick={requestHelp}
+            title="Help"
+            useCompactStyle
           />
           {loginControl}
         </div>

src/sidebar/components/user-menu.js

@@ -8,6 +8,7 @@ const { isThirdPartyUser } = require('../util/account-id');
 const serviceConfig = require('../service-config');
 const { withServices } = require('../util/service-context');
 
+const IconButton = require('./icon-button');
 const Menu = require('./menu');
 const MenuSection = require('./menu-section');
 const MenuItem = require('./menu-item');
@@ -44,7 +45,14 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) {
     return props;
   })();
 
-  const menuLabel = <i className="h-icon-account top-bar__btn" />;
+  const menuLabel = (
+    <IconButton
+      className="top-bar__icon-button"
+      icon="profile"
+      title="User menu"
+      useCompactStyle
+    />
+  );
   return (
     <div className="user-menu">
       <Menu label={menuLabel} title={auth.displayName} align="right">

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

@@ -11,6 +11,10 @@
       color: var.$brand;
     }
   }
+
+  &--compact {
+    padding: 0.25em;
+  }
 }
 
 @media (pointer: coarse) {
@@ -18,4 +22,12 @@
     min-width: var.$touch-target-size;
     min-height: var.$touch-target-size;
   }
+
+  // Until the top bar can be refactored to allow for breathing room around
+  // the search interface, we can't spare the room for comfortable tap targets
+  // on touchscreen devices. This overrides `IconButton`'s larger tap targets.
+  .icon-button--compact {
+    min-width: auto;
+    min-height: auto;
+  }
 }

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

@@ -9,7 +9,7 @@
   color: var.$grey-mid;
 }
 
-.search-input__icon {
+.search-input__icon-button {
   order: 0;
 }
 
@@ -27,11 +27,8 @@
   width: 100%;
 
   // The search box expands when focused, via a change in the
-  // `max-width` property. In Safari, the <input> will not accept
-  // focus if `max-width` is set to 0px so we set it to
-  // a near-zero positive value instead.
-  // See https://github.com/hypothesis/h/issues/2654
-  max-width: 0.1px;
+  // `max-width` property.
+  max-width: 0px;
 
   transition: max-width 0.3s ease-out, padding-left 0.3s ease-out;
 

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

@@ -16,6 +16,24 @@
   // Force top-bar onto a new compositor layer so that it does not judder when
   // the window is scrolled.
   transform: translate3d(0, 0, 0);
+
+  &__icon-button {
+    &.is-active {
+      color: var.$grey-6;
+
+      &:hover {
+        color: var.$grey-6;
+      }
+    }
+  }
+
+  &__icon-button--refresh {
+    color: var.$brand;
+
+    &:hover {
+      color: var.$brand;
+    }
+  }
 }
 
 .top-bar--theme-clean {
@@ -55,29 +73,3 @@
 .top-bar__expander {
   flex-grow: 1;
 }
-
-.top-bar__btn {
-  @include buttons.reset-native-btn-styles;
-
-  height: 100%;
-  color: var.$grey-semi;
-  display: inline-block;
-  cursor: pointer;
-  padding: 1px 3px 0 3px;
-
-  &:hover {
-    color: var.$grey-mid;
-  }
-
-  &.is-active {
-    color: var.$grey-mid;
-  }
-
-  &--refresh {
-    color: var.$brand;
-
-    &:hover {
-      color: var.$brand;
-    }
-  }
-}