Bug 1467572 - Part 11: Use native context menu instead of select elements in the DPR menu. r=Honza
☠☠ backed out by dc2c0b075c0e ☠ ☠
authorGabriel Luong <gabriel.luong@gmail.com>
Wed, 15 Aug 2018 17:27:44 -0400
changeset 431825 470b9ce4ba0fd5b6c5b88f3d7b95492791d77815
parent 431824 37609995dc8f1ce64c3bcd7bc9b89928c013173d
child 431826 abb52c2c68ab5063ba4bcc83cdeeef8a83a1e643
push id34451
push userebalazs@mozilla.com
push dateThu, 16 Aug 2018 09:25:15 +0000
treeherdermozilla-central@161817e6d127 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1467572
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 1467572 - Part 11: Use native context menu instead of select elements in the DPR menu. r=Honza
devtools/client/responsive.html/components/DevicePixelRatioMenu.js
devtools/client/responsive.html/components/DevicePixelRatioSelector.js
devtools/client/responsive.html/components/Toolbar.js
devtools/client/responsive.html/components/moz.build
devtools/client/responsive.html/index.css
rename from devtools/client/responsive.html/components/DevicePixelRatioSelector.js
rename to devtools/client/responsive.html/components/DevicePixelRatioMenu.js
--- a/devtools/client/responsive.html/components/DevicePixelRatioSelector.js
+++ b/devtools/client/responsive.html/components/DevicePixelRatioMenu.js
@@ -8,129 +8,86 @@
 
 const { PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 const { getStr, getFormatStr } = require("../utils/l10n");
 const Types = require("../types");
 
-const labelForOption = value => getFormatStr("responsive.devicePixelRatioOption", value);
+loader.lazyRequireGetter(this, "showMenu", "devtools/client/shared/components/menu/utils", true);
+
 const PIXEL_RATIO_PRESET = [1, 2, 3];
 
-const createVisibleOption = value => {
-  const label = labelForOption(value);
-  return dom.option({
-    value,
-    title: label,
-    key: value,
-  }, label);
-};
-
-const createHiddenOption = value => {
-  const label = labelForOption(value);
-  return dom.option({
-    value,
-    title: label,
-    hidden: true,
-    disabled: true,
-  }, label);
-};
-
-class DevicePixelRatioSelector extends PureComponent {
+class DevicePixelRatioMenu extends PureComponent {
   static get propTypes() {
     return {
       devices: PropTypes.shape(Types.devices).isRequired,
       displayPixelRatio: Types.pixelRatio.value.isRequired,
       selectedDevice: PropTypes.string.isRequired,
       selectedPixelRatio: PropTypes.shape(Types.pixelRatio).isRequired,
       onChangePixelRatio: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
-
-    this.state = {
-      isFocused: false
-    };
-
-    this.onFocusChange = this.onFocusChange.bind(this);
-    this.onSelectChange = this.onSelectChange.bind(this);
+    this.onShowDevicePixelMenu = this.onShowDevicePixelMenu.bind(this);
   }
 
-  onFocusChange({type}) {
-    this.setState({
-      isFocused: type === "focus"
+  onShowDevicePixelMenu(event) {
+    const {
+      displayPixelRatio,
+      onChangePixelRatio,
+    } = this.props;
+
+    const menuItems = PIXEL_RATIO_PRESET.map(value => {
+      return {
+        label: getFormatStr("responsive.devicePixelRatioOption", value),
+        type: "checkbox",
+        checked: displayPixelRatio === value,
+        click: () => onChangePixelRatio(+value),
+      };
     });
-  }
 
-  onSelectChange({ target }) {
-    this.props.onChangePixelRatio(+target.value);
+    showMenu(menuItems, {
+      button: event.target,
+      useTopLevelWindow: true,
+    });
   }
 
   render() {
     const {
       devices,
       displayPixelRatio,
       selectedDevice,
       selectedPixelRatio,
     } = this.props;
 
-    const hiddenOptions = [];
-
-    for (const type of devices.types) {
-      for (const device of devices[type]) {
-        if (device.displayed &&
-            !hiddenOptions.includes(device.pixelRatio) &&
-            !PIXEL_RATIO_PRESET.includes(device.pixelRatio)) {
-          hiddenOptions.push(device.pixelRatio);
-        }
-      }
-    }
+    const isDisabled = devices.listState !== Types.loadableState.LOADED ||
+      selectedDevice !== "";
 
-    if (!PIXEL_RATIO_PRESET.includes(displayPixelRatio)) {
-      hiddenOptions.push(displayPixelRatio);
-    }
-
-    const state = devices.listState;
-    const isDisabled = (state !== Types.loadableState.LOADED) || (selectedDevice !== "");
-    let selectorClass = "toolbar-dropdown";
     let title;
-
     if (isDisabled) {
-      selectorClass += " disabled";
       title = getFormatStr("responsive.devicePixelRatio.auto", selectedDevice);
     } else {
       title = getStr("responsive.changeDevicePixelRatio");
-
-      if (selectedPixelRatio.value) {
-        selectorClass += " selected";
-      }
-    }
-
-    if (this.state.isFocused) {
-      selectorClass += " focused";
     }
 
-    let listContent = PIXEL_RATIO_PRESET.map(createVisibleOption);
-
-    if (state == Types.loadableState.LOADED) {
-      listContent = listContent.concat(hiddenOptions.map(createHiddenOption));
-    }
-
-    return dom.select(
-      {
-        id: "device-pixel-ratio-selector",
-        value: selectedPixelRatio.value || displayPixelRatio,
-        disabled: isDisabled,
-        onChange: this.onSelectChange,
-        onFocus: this.onFocusChange,
-        onBlur: this.onFocusChange,
-        className: selectorClass,
-        title: title
-      },
-      ...listContent
+    return (
+      dom.button(
+        {
+          id: "device-pixel-ratio-menu",
+          className: "devtools-button devtools-dropdown-button",
+          disabled: isDisabled,
+          title,
+          onClick: this.onShowDevicePixelMenu,
+        },
+        dom.span({ className: "title" },
+          getFormatStr("responsive.devicePixelRatioOption",
+            selectedPixelRatio.value || displayPixelRatio)
+        )
+      )
     );
   }
 }
 
-module.exports = DevicePixelRatioSelector;
+module.exports = DevicePixelRatioMenu;
--- a/devtools/client/responsive.html/components/Toolbar.js
+++ b/devtools/client/responsive.html/components/Toolbar.js
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { PureComponent, createFactory } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
-const DevicePixelRatioSelector = createFactory(require("./DevicePixelRatioSelector"));
+const DevicePixelRatioMenu = createFactory(require("./DevicePixelRatioMenu"));
 const DeviceSelector = createFactory(require("./DeviceSelector"));
 const NetworkThrottlingMenu = createFactory(require("devtools/client/shared/components/throttling/NetworkThrottlingMenu"));
 const SettingsMenu = createFactory(require("./SettingsMenu"));
 const ViewportDimension = createFactory(require("./ViewportDimension"));
 
 const { getStr } = require("../utils/l10n");
 const Types = require("../types");
 
@@ -91,17 +91,17 @@ class Toolbar extends PureComponent {
         }),
         dom.button({
           id: "rotate-button",
           className: "toolbar-button devtools-button",
           onClick: () => onRotateViewport(viewport.id),
           title: getStr("responsive.rotate"),
         }),
         dom.div({ className: "devtools-separator" }),
-        DevicePixelRatioSelector({
+        DevicePixelRatioMenu({
           devices,
           displayPixelRatio,
           selectedDevice,
           selectedPixelRatio,
           onChangePixelRatio,
         }),
         dom.div({ className: "devtools-separator" }),
         NetworkThrottlingMenu({
--- a/devtools/client/responsive.html/components/moz.build
+++ b/devtools/client/responsive.html/components/moz.build
@@ -4,16 +4,16 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DevToolsModules(
     'App.js',
     'Browser.js',
     'DeviceAdder.js',
     'DeviceModal.js',
-    'DevicePixelRatioSelector.js',
+    'DevicePixelRatioMenu.js',
     'DeviceSelector.js',
     'ResizableViewport.js',
     'SettingsMenu.js',
     'Toolbar.js',
     'ViewportDimension.js',
     'Viewports.js',
 )
--- a/devtools/client/responsive.html/index.css
+++ b/devtools/client/responsive.html/index.css
@@ -207,41 +207,24 @@ select > option.divider {
   background-image: url("chrome://devtools/skin/images/close.svg");
 }
 
 #screenshot-button:disabled {
   filter: var(--theme-icon-checked-filter);
   opacity: 1 !important;
 }
 
-#device-pixel-ratio-selector {
-  -moz-user-select: none;
-  color: var(--viewport-color);
-  height: 15px;
+#device-pixel-ratio-menu {
+  width: 6em;
   /* `max-width` is here to keep the UI compact if the device pixel ratio changes to a
      repeating decimal value.  This can happen if you zoom the UI (Cmd + Plus / Minus on
      macOS for example). */
   max-width: 8em;
-}
-
-#device-pixel-ratio-selector.focused,
-#device-pixel-ratio-selector:not(.disabled):hover {
-  color: var(--viewport-hover-color);
-}
-
-#device-pixel-ratio-selector:focus {
-  color: var(--viewport-active-color);
-}
-
-#device-pixel-ratio-selector.selected {
-  color: var(--viewport-active-color);
-}
-
-#device-pixel-ratio-selector > option {
-  padding: 5px;
+  background-position: right 4px center;
+  padding-left: 0;
 }
 
 #viewports-container {
   display: flex;
   justify-content: center;
   overflow: auto;
   height: 100%;
   width: 100%;