Bug 1565460 - improve accessibility and configurability of the shared menu components. r=jdescottes

Differential Revision: https://phabricator.services.mozilla.com/D39117

--HG--
extra : moz-landing-system : lando

devtools/client/shared/components/menu/MenuButton.js

@@ -45,6 +45,9 @@ class MenuButton extends PureComponent {
       // The document to be used for rendering the menu popup.
       doc: PropTypes.object.isRequired,
 
+      // A text content for the button.
+      label: PropTypes.string,
+
       // An optional ID to assign to the menu's container tooltip object.
       menuId: PropTypes.string,
 
@@ -411,10 +414,10 @@ class MenuButton extends PureComponent {
         this.tooltip.panel
       );
 
-      return button(buttonProps, menu);
+      return button(buttonProps, this.props.label, menu);
     }
 
-    return button(buttonProps);
+    return button(buttonProps, this.props.label);
   }
 }
 

devtools/client/shared/components/menu/MenuItem.js

@@ -30,14 +30,8 @@ class MenuItem extends PureComponent {
       // a space-separated string.
       className: PropTypes.string,
 
-      // An optional ID to be assigned to the item.
-      id: PropTypes.string,
-
-      // The item label.
-      label: PropTypes.string.isRequired,
-
-      // An optional callback to be invoked when the item is selected.
-      onClick: PropTypes.func,
+      // A disabled state of the menu item.
+      disabled: PropTypes.bool,
 
       // URL of the icon to associate with the MenuItem. (Optional)
       //
@@ -48,6 +42,22 @@ class MenuItem extends PureComponent {
       // use, not simply the URL (e.g.
       // "url(chrome://devtools/skim/image/foo.svg)").
       icon: PropTypes.string,
+
+      // An optional ID to be assigned to the item.
+      id: PropTypes.string,
+
+      // The item label.
+      label: PropTypes.string.isRequired,
+
+      // An optional callback to be invoked when the item is selected.
+      onClick: PropTypes.func,
+
+      // Optional menu item role override. Use this property with a value
+      // "menuitemradio" if the menu item is a radio.
+      role: PropTypes.string,
+
+      // An optional text for the item tooltip.
+      tooltip: PropTypes.string,
     };
   }
 
@@ -111,15 +121,26 @@ class MenuItem extends PureComponent {
       attr.onClick = this.props.onClick;
     }
 
-    if (typeof this.props.checked !== "undefined") {
+    if (this.props.tooltip) {
+      attr.title = this.props.tooltip;
+    }
+
+    if (this.props.disabled) {
+      attr.disabled = this.props.disabled;
+    }
+
+    if (this.props.role) {
+      attr.role = this.props.role;
+    } else if (typeof this.props.checked !== "undefined") {
       attr.role = "menuitemcheckbox";
-      if (this.props.checked) {
-        attr["aria-checked"] = true;
-      }
     } else {
       attr.role = "menuitem";
     }
 
+    if (this.props.checked) {
+      attr["aria-checked"] = true;
+    }
+
     const textLabel = span(
       { key: "label", className: "label", ref: this.labelRef },
       this.props.label

devtools/client/shared/components/menu/MenuList.js

@@ -80,6 +80,7 @@ class MenuList extends PureComponent {
       Array.from(this.wrapperRef.querySelectorAll(focusableSelector));
 
     switch (e.key) {
+      case "Tab":
       case "ArrowUp":
       case "ArrowDown":
         {
@@ -88,7 +89,7 @@ class MenuList extends PureComponent {
           const currentIndex = tabList.indexOf(currentElement);
           if (currentIndex !== -1) {
             let nextIndex;
-            if (e.key === "ArrowDown") {
+            if (e.key === "ArrowDown" || (e.key === "Tab" && !e.shiftKey)) {
               nextIndex =
                 currentIndex === tabList.length - 1 ? 0 : currentIndex + 1;
             } else {