Bug 1474221 - Part 1. Allow specifying MenuItem icons from Javascript. r=birtles
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Thu, 19 Jul 2018 16:11:26 +0900
changeset 427451 c300d08cad3060e250a7ac5eb5dba6537c98d602
parent 427450 3c2db10f62dedf67dc812b9ec8f5f6c40bce5037
child 427452 a3459b4851d3c5f6938466a8d86aba1fbe404594
push id34306
push usercsabou@mozilla.com
push dateFri, 20 Jul 2018 21:41:18 +0000
treeherdermozilla-central@d6a5e8aea651 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1474221
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1474221 - Part 1. Allow specifying MenuItem icons from Javascript. r=birtles This patch introduces the icon property for MenuItem. It also add padding to allow for checkboxes only if there is at least one MenuItem using a checkbox. MozReview-Commit-ID: CvvlR51mA53
devtools/client/shared/components/menu/MenuItem.js
devtools/client/shared/components/menu/MenuList.js
devtools/client/themes/toolbox.css
devtools/client/themes/tooltips.css
--- a/devtools/client/shared/components/menu/MenuItem.js
+++ b/devtools/client/shared/components/menu/MenuItem.js
@@ -19,16 +19,21 @@ const MenuItem = props => {
   if (props.id) {
     attr.id = props.id;
   }
 
   if (props.className) {
     attr.className += " " + props.className;
   }
 
+  if (props.icon) {
+    attr.className += " iconic";
+    attr.style = { "--menuitem-icon-image": "url(" + props.icon + ")" };
+  }
+
   if (props.onClick) {
     attr.onClick = props.onClick;
   }
 
   if (typeof props.checked !== "undefined") {
     attr.role = "menuitemcheckbox";
     if (props.checked) {
       attr["aria-checked"] = true;
@@ -67,11 +72,20 @@ MenuItem.propTypes = {
   // 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,
+
+  // URL of the icon to associate with the MenuItem. (Optional)
+  //
+  //   e.g. chrome://devtools/skim/image/foo.svg
+  //
+  // This may also be set in CSS using the --menuitem-icon-image variable.
+  // Note that in this case, the variable should specify the CSS <image> to use,
+  // not simply the URL (e.g. "url(chrome://devtools/skim/image/foo.svg)").
+  icon: PropTypes.string,
 };
 
 module.exports = MenuItem;
--- a/devtools/client/shared/components/menu/MenuList.js
+++ b/devtools/client/shared/components/menu/MenuList.js
@@ -5,17 +5,17 @@
 /* eslint-env browser */
 "use strict";
 
 // A list of menu items.
 //
 // This component provides keyboard navigation amongst any focusable
 // children.
 
-const { PureComponent } = require("devtools/client/shared/vendor/react");
+const { Children, PureComponent } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const { div } = dom;
 const { focusableSelector } = require("devtools/client/shared/focus");
 
 class MenuList extends PureComponent {
   static get propTypes() {
     return {
@@ -104,13 +104,24 @@ class MenuList extends PureComponent {
       ref: this.setWrapperRef,
       onKeyDown: this.onKeyDown,
     };
 
     if (this.props.id) {
       attr.id = this.props.id;
     }
 
+    // Add padding for checkbox image if necessary.
+    let hasCheckbox = false;
+    Children.forEach(this.props.children, child => {
+      if (typeof child.props.checked !== "undefined") {
+        hasCheckbox = true;
+      }
+    });
+    if (hasCheckbox) {
+      attr.className = "checkbox-container";
+    }
+
     return div(attr, this.props.children);
   }
 }
 
 module.exports = MenuList;
--- a/devtools/client/themes/toolbox.css
+++ b/devtools/client/themes/toolbox.css
@@ -222,35 +222,35 @@
 }
 
 #toolbox-meatball-menu-button::before {
   fill: var(--theme-toolbar-photon-icon-color);
   background-image: var(--more-button-image);
 }
 
 #toolbox-meatball-menu-dock-bottom > .label::before {
-  background-image: var(--dock-bottom-image);
+  --menuitem-icon-image: var(--dock-bottom-image);
 }
 #toolbox-meatball-menu-dock-left > .label::before {
-  background-image: var(--dock-side-left-image);
+  --menuitem-icon-image: var(--dock-side-left-image);
 }
 #toolbox-meatball-menu-dock-right > .label::before {
-  background-image: var(--dock-side-right-image);
+  --menuitem-icon-image: var(--dock-side-right-image);
 }
 #toolbox-meatball-menu-dock-window > .label::before {
-  background-image: var(--dock-undock-image);
+  --menuitem-icon-image: var(--dock-undock-image);
 }
 #toolbox-meatball-menu-splitconsole > .label::before {
-  background-image: var(--command-console-image);
+  --menuitem-icon-image: var(--command-console-image);
 }
 #toolbox-meatball-menu-noautohide > .label::before {
-  background-image: var(--command-noautohide-image);
+  --menuitem-icon-image: var(--command-noautohide-image);
 }
 #toolbox-meatball-menu-settings > .label::before {
-  background-image: var(--settings-image);
+  --menuitem-icon-image: var(--settings-image);
 }
 
 /* Command buttons */
 
 .command-button,
 #toolbox-controls > button {
   /* !important is needed to override .devtools-button rules in common.css */
   padding: 0 !important;
--- a/devtools/client/themes/tooltips.css
+++ b/devtools/client/themes/tooltips.css
@@ -429,29 +429,34 @@
 }
 
 .tooltip-container[type="doorhanger"] .menuitem > .command[aria-checked="true"]:-moz-locale-dir(rtl) {
   background-position: center right 7px;
 }
 
 .tooltip-container[type="doorhanger"] .menuitem > .command > .label {
   flex: 1;
+  font: menu;
+}
+
+.tooltip-container[type="doorhanger"] .checkbox-container .menuitem > .command > .label {
   padding-inline-start: 16px;
-  font: menu;
 }
 
 .tooltip-container[type="doorhanger"] .menuitem > .command.iconic > .label::before {
   content: " ";
   display: inline-block;
   margin-inline-end: 8px;
   width: 16px;
   height: 16px;
   vertical-align: top;
   -moz-context-properties: fill;
   fill: currentColor;
+  background-image: var(--menuitem-icon-image);
+  background-size: contain;
   /*
    * The icons in the sidebar menu have opacity: 0.8 here, but those in the
    * hamburger menu don't. For now we match the hamburger menu styling,
    * especially because the 80% opacity makes the icons look dull in dark mode.
    */
 }
 
 .tooltip-container[type="doorhanger"] .menuitem > .command > .accelerator {