Merge pull request #1711 from hypothesis/menu-focus-fix

Fix Menu tab focus on keyboard navigation
hypothesis · Jan 24, 2020 · 2c9bf06 · 2c9bf06
2 parents 7dca162 + 75ed100
commit 2c9bf06
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 20 deletions.
diff --git a/src/sidebar/components/annotation-publish-control.js b/src/sidebar/components/annotation-publish-control.js
@@ -58,9 +58,7 @@ function AnnotationPublishControl({
       <div
         className="annotation-publish-control__btn-dropdown-arrow-indicator"
         style={applyTheme(themeProps, settings)}
-      >
-        <div>▼</div>
-      </div>
+      />
     </div>
   );
 

diff --git a/src/sidebar/components/menu.js b/src/sidebar/components/menu.js
@@ -126,14 +126,19 @@ export default function Menu({
         onClick={toggleMenu}
         title={title}
       >
-        {label}
-        {menuIndicator && (
-          <span
-            className={classnames('menu__toggle-arrow', isOpen && 'is-open')}
-          >
-            <SvgIcon name="expand-menu" className="menu__toggle-icon" />
-          </span>
-        )}
+        <span
+          // wrapper is needed to serve as the flex layout for the label and indicator content.
+          className="menu__toggle-wrapper"
+        >
+          {label}
+          {menuIndicator && (
+            <span
+              className={classnames('menu__toggle-arrow', isOpen && 'is-open')}
+            >
+              <SvgIcon name="expand-menu" className="menu__toggle-icon" />
+            </span>
+          )}
+        </span>
       </button>
       {isOpen && (
         <Fragment>

diff --git a/src/styles/sidebar/components/annotation-publish-control.scss b/src/styles/sidebar/components/annotation-publish-control.scss
@@ -16,6 +16,7 @@
     $arrow-indicator-width: 26px;
 
     height: $height;
+    display: flex;
     position: relative;
     margin-right: 1em;
 
@@ -38,22 +39,20 @@
       width: 100%;
       height: 100%;
       text-align: left;
-      padding-left: $h-padding;
-      padding-right: $arrow-indicator-width + 8px;
+      border-top-right-radius: 0;
+      border-bottom-right-radius: 0;
     }
 
     // dropdown arrow which reveals the button's associated menu
     // when clicked
     &-dropdown-arrow {
-      position: absolute;
+      display: flex;
+      align-items: center;
       right: 0px;
       top: 0px;
 
       height: 100%;
       width: $arrow-indicator-width;
-      padding-left: 0px;
-      padding-right: $h-padding;
-      margin-left: 8px;
 
       border: none;
       background-color: var.$grey-mid;
@@ -90,15 +89,17 @@
       // the ▼ arrow which reveals the dropdown menu when clicked
       &-indicator {
         color: $text-color;
-        position: absolute;
         left: 0px;
         right: 0px;
         top: 0px;
         bottom: 0px;
         line-height: $height;
         text-align: center;
+        width: 100%;
 
-        & > div {
+        &:after {
+          content: '▼';
+          display: inline-block;
           transform: scaleY(0.7);
         }
       }

diff --git a/src/styles/sidebar/components/menu.scss b/src/styles/sidebar/components/menu.scss
@@ -14,10 +14,18 @@
   background: none;
   padding: 0;
   color: inherit;
-  display: flex;
+  // "block" display is needed so it can take up the
+  //  full height of its parent container
+  display: block;
+  height: 100%;
   align-items: center;
 }
 
+.menu__toggle-wrapper {
+  display: flex;
+  align-items: center;
+  height: 100%;
+}
 .menu__toggle-icon {
   width: 10px;
   height: 10px;