Merge pull request #1492 from hypothesis/top-bar-cleanup

Clean up `TopBar`: CSS, ARIA

src/images/icons/refresh.svg

@@ -3,7 +3,7 @@
     <!-- Generator: Sketch 39.1 (31720) - http://www.bohemiancoding.com/sketch -->
     <defs/>
     <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
-        <g id="Artboard" fill="#D00032">
+        <g id="Artboard" fill="currentColor">
             <g id="Group">
                 <path d="M0,8 C0,12.418278 3.581722,16 8,16 C12.418278,16 16,12.418278 16,8 C16,4.89580324 14.2154684,2.11256098 11.4682644,0.789110134 L10.6002482,2.59092808 C12.661769,3.58405472 14,5.6712248 14,8 C14,11.3137085 11.3137085,14 8,14 C4.6862915,14 2,11.3137085 2,8 C2,5.65296151 3.35941993,3.55225774 5.44569583,2.56903563 L4.59307587,0.759881355 C1.81273067,2.07020511 0,4.87140735 0,8 Z" id="Oval"/>
                 <polygon id="Combined-Shape" points="7 8.58578644 7 0 9 0 9 8.58578644 10.2928932 7.29289322 11 6.58578644 12.4142136 8 11.7071068 8.70710678 8.70710678 11.7071068 8 12.4142136 7.64644661 12.0606602 7.29289322 11.7071068 4.29289322 8.70710678 3.58578644 8 5 6.58578644 5.70710678 7.29289322"/>

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

@@ -55,7 +55,7 @@ describe('TopBar', () => {
   });
 
   function applyUpdateBtn(wrapper) {
-    return wrapper.find('.top-bar__apply-update-btn');
+    return wrapper.find('.top-bar__btn--refresh');
   }
 
   function helpBtn(wrapper) {
@@ -117,8 +117,9 @@ describe('TopBar', () => {
         const help = helpBtn(wrapper);
 
         wrapper.update();
-        assert.isTrue(help.hasClass('top-bar__btn--active'));
-        assert.isOk(help.prop('aria-expanded'));
+
+        assert.isTrue(help.hasClass('is-active'));
+        assert.isOk(help.prop('aria-pressed'));
       });
 
       context('help service handler configured in services', () => {
@@ -207,7 +208,7 @@ describe('TopBar', () => {
 
   it('toggles the share annotations panel when "Share" is clicked', () => {
     const wrapper = createTopBar();
-    wrapper.find('[title="Share annotations on this page"]').simulate('click');
+    wrapper.find('.top-bar__share-btn').simulate('click');
     assert.calledWith(
       fakeStore.toggleSidebarPanel,
       uiConstants.PANEL_SHARE_ANNOTATIONS
@@ -222,9 +223,9 @@ describe('TopBar', () => {
     });
 
     const wrapper = createTopBar();
-    const shareEl = wrapper.find('[title="Share annotations on this page"]');
+    const shareEl = wrapper.find('.top-bar__share-btn');
 
-    assert.include(shareEl.prop('className'), 'top-bar__btn--active');
+    assert.include(shareEl.prop('className'), 'is-active');
   });
 
   it('displays search input in the sidebar', () => {

src/sidebar/components/top-bar.js

@@ -20,30 +20,38 @@ const SvgIcon = require('./svg-icon');
 const UserMenu = require('./user-menu');
 
 /**
- * Button for opening/closing the help panel
+ * 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 HelpButton({ onClick }) {
-  const isActive = useStore(
-    store =>
-      store.getState().sidebarPanels.activePanelName === uiConstants.PANEL_HELP
+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__help-btn', {
-        'top-bar__btn--active': isActive,
+      className={classnames('top-bar__btn', `top-bar__${iconName}-btn`, {
+        'is-active': isActive,
       })}
       onClick={onClick}
-      title="Help"
-      aria-expanded={isActive}
-      aria-pressed={isActive}
+      title={title}
+      aria-pressed={isActive ? 'true' : 'false'}
     >
-      <SvgIcon name="help" className="top-bar__help-icon" />
+      <SvgIcon
+        name={iconName}
+        className={classnames(`top-bar__{$iconName}-icon`)}
+      />
     </button>
   );
 }
 
-HelpButton.propTypes = {
-  /* callback */
+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,
 };
 
@@ -71,9 +79,6 @@ function TopBar({
   const pendingUpdateCount = useStore(store => store.pendingUpdateCount());
 
   const togglePanelFn = useStore(store => store.toggleSidebarPanel);
-  const currentActivePanel = useStore(
-    store => store.getState().sidebarPanels.activePanelName
-  );
 
   const applyPendingUpdates = () => streamer.applyPendingUpdates();
 
@@ -125,7 +130,12 @@ function TopBar({
         <div className="top-bar__inner content">
           <StreamSearchInput />
           <div className="top-bar__expander" />
-          <HelpButton onClick={requestHelp} />
+          <TogglePanelButton
+            panelName={uiConstants.PANEL_HELP}
+            iconName="help"
+            title="Help"
+            onClick={requestHelp}
+          />
           {loginControl}
         </div>
       )}
@@ -135,32 +145,32 @@ function TopBar({
           <GroupList className="GroupList" auth={auth} />
           <div className="top-bar__expander" />
           {pendingUpdateCount > 0 && (
-            <a
-              className="top-bar__apply-update-btn"
+            <button
+              className="top-bar__btn top-bar__btn--refresh"
               onClick={applyPendingUpdates}
               title={`Show ${pendingUpdateCount} new/updated ${
                 pendingUpdateCount === 1 ? 'annotation' : 'annotations'
               }`}
             >
               <SvgIcon className="top-bar__apply-icon" name="refresh" />
-            </a>
+            </button>
           )}
           <SearchInput query={filterQuery} onSearch={setFilterQuery} />
           <SortMenu />
           {showSharePageButton && (
-            <button
-              className={classnames('top-bar__btn', {
-                'top-bar__btn--active':
-                  currentActivePanel === uiConstants.PANEL_SHARE_ANNOTATIONS,
-              })}
-              onClick={toggleSharePanel}
+            <TogglePanelButton
+              panelName={uiConstants.PANEL_SHARE_ANNOTATIONS}
+              iconName="share"
               title="Share annotations on this page"
-              aria-label="Share annotations on this page"
-            >
-              <SvgIcon name="share" />
-            </button>
+              onClick={toggleSharePanel}
+            />
           )}
-          <HelpButton onClick={requestHelp} />
+          <TogglePanelButton
+            panelName={uiConstants.PANEL_HELP}
+            iconName="help"
+            title="Help"
+            onClick={requestHelp}
+          />
           {loginControl}
         </div>
       )}

src/styles/mixins/buttons.scss

+@import './focus';
+
+@mixin reset-native-btn-styles {
+  @include outline-on-keyboard-focus;
+
+  padding: 0px;
+  margin: 0px;
+  background-color: transparent;
+  border-style: none;
+}

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

 @import '../../base';
 @import '../../mixins/responsive';
+@import '../../mixins/buttons';
 
 .top-bar {
   background: $white;
-  border-bottom: solid 1px $gray-lighter;
+  border-bottom: solid 1px $grey-3;
   height: $top-bar-height;
+  // TODO bring into standard rhythm of typography
   font-size: 15px;
   position: absolute;
   left: 0;
@@ -24,7 +26,7 @@
 .top-bar__login-links {
   flex-shrink: 0;
   font-size: $body2-font-size;
-  padding-left: 6px;
+  padding-left: 5px;
 }
 
 .top-bar__inner {
@@ -55,16 +57,6 @@
   flex-grow: 1;
 }
 
-// Removes the native styling from a <button> element
-@mixin reset-native-btn-styles {
-  @include outline-on-keyboard-focus;
-
-  padding: 0px;
-  margin: 0px;
-  background-color: transparent;
-  border-style: none;
-}
-
 .top-bar__btn {
   @include reset-native-btn-styles;
 
@@ -77,37 +69,16 @@
   &:hover {
     color: $gray-dark;
   }
-  &--active {
+
+  &.is-active {
     color: $gray-dark;
   }
-}
 
-// Button which indicates that other users have made or edited annotations
-// on the current page and applies them when clicked
-//
-// The button displays a 'refresh' icon and an update count, both of which
-// should be vertically centered within the icon.
-.top-bar__apply-update-btn {
-  display: flex;
-  flex-direction: row;
-  align-items: center;
-  flex-shrink: 0;
-  margin-right: 5px;
+  &--refresh {
+    color: $brand;
 
-  border-radius: 2px;
-  color: white;
-  cursor: pointer;
-  font-weight: bold;
-  padding-bottom: 2px;
-  padding-top: 2px;
-  user-select: none;
-}
-
-.top-bar__apply-icon {
-  display: inline-block;
-  line-height: 12px;
-}
-
-.top-bar__dropdown-arrow {
-  color: $color-dove-gray;
+    &:hover {
+      color: $brand;
+    }
+  }
 }