Bug 1467572 - Part 12: Use native context menu instead of select element in the Device selector. r=Honza draft
authorGabriel Luong <gabriel.luong@gmail.com>
Tue, 14 Aug 2018 17:54:54 -0400 (2018-08-14)
changeset 829217 a2ea7996091a6db0a7d1124b3a7d5c6a6651b66f
parent 829216 8672371de55d45e83eb5e7d7b5033cf902c782d2
child 829218 17e86c0835b72e2e4cdf4f18281b96f300326111
push id118750
push userbmo:gl@mozilla.com
push dateTue, 14 Aug 2018 21:55:43 +0000 (2018-08-14)
reviewersHonza
bugs1467572
milestone63.0a1
Bug 1467572 - Part 12: Use native context menu instead of select element in the Device selector. r=Honza - Removes all the unused styling for the <select> and <option> elements and the toolbar-dropdown class. MozReview-Commit-ID: 1sf6VSBIJy9
devtools/client/locales/en-US/responsive.properties
devtools/client/responsive.html/components/DeviceSelector.js
devtools/client/responsive.html/index.css
--- a/devtools/client/locales/en-US/responsive.properties
+++ b/devtools/client/locales/en-US/responsive.properties
@@ -6,68 +6,60 @@
 # available from the Web Developer sub-menu -> 'Responsive Design Mode'.
 #
 # The correct localization of this file might be to keep it in
 # English, or another language commonly spoken among web developers.
 # You want to make that choice consistent across the developer tools.
 # A good criteria is the language in which you'd find the best
 # documentation on web development on the web.
 
-# LOCALIZATION NOTE (responsive.editDeviceList): option displayed in the device
-# selector
-responsive.editDeviceList=Edit list…
+# LOCALIZATION NOTE (responsive.editDeviceList2): Context menu item displayed in the
+# device selector.
+responsive.editDeviceList2=Edit List…
 
-# LOCALIZATION NOTE (responsive.exit): tooltip text of the exit button.
+# LOCALIZATION NOTE (responsive.exit): Tooltip text of the exit button.
 responsive.exit=Close Responsive Design Mode
 
-# LOCALIZATION NOTE (responsive.rotate): tooltip text of the rotate button.
+# LOCALIZATION NOTE (responsive.rotate): Tooltip text of the rotate button.
 responsive.rotate=Rotate viewport
 
-# LOCALIZATION NOTE (responsive.deviceListLoading): placeholder text for
-# device selector when it's still fetching devices
-responsive.deviceListLoading=Loading…
-
-# LOCALIZATION NOTE (responsive.deviceListError): placeholder text for
-# device selector when an error occurred
-responsive.deviceListError=No list available
-
-# LOCALIZATION NOTE (responsive.done): button text in the device list modal
+# LOCALIZATION NOTE (responsive.done): Button text in the device list modal
 responsive.done=Done
 
-# LOCALIZATION NOTE (responsive.noDeviceSelected): placeholder text for the
-# device selector
-responsive.noDeviceSelected=no device selected
+# LOCALIZATION NOTE (responsive.responsiveMode): Placeholder text for the
+# device selector.
+responsive.responsiveMode=Responsive
 
-# LOCALIZATION NOTE (responsive.enableTouch): tooltip text for the touch
-# simulation button when it's disabled
+# LOCALIZATION NOTE (responsive.enableTouch): Tooltip text for the touch
+# simulation button when it's disabled.
 responsive.enableTouch=Enable touch simulation
 
-# LOCALIZATION NOTE (responsive.disableTouch): tooltip text for the touch
-# simulation button when it's enabled
+# LOCALIZATION NOTE (responsive.disableTouch): Tooltip text for the touch
+# simulation button when it's enabled.
 responsive.disableTouch=Disable touch simulation
 
-# LOCALIZATION NOTE  (responsive.screenshot): tooltip of the screenshot button.
+# LOCALIZATION NOTE  (responsive.screenshot): Tooltip of the screenshot button.
 responsive.screenshot=Take a screenshot of the viewport
 
 # LOCALIZATION NOTE (responsive.screenshotGeneratedFilename): The auto generated
 # filename.
 # The first argument (%1$S) is the date string in yyyy-mm-dd format and the
 # second argument (%2$S) is the time string in HH.MM.SS format.
 responsive.screenshotGeneratedFilename=Screen Shot %1$S at %2$S
 
 # LOCALIZATION NOTE (responsive.remoteOnly): Message displayed in the tab's
 # notification box if a user tries to open Responsive Design Mode in a
 # non-remote tab.
 responsive.remoteOnly=Responsive Design Mode is only available for remote browser tabs, such as those used for web content in multi-process Firefox.
 
-# LOCALIZATION NOTE (responsive.changeDevicePixelRatio): tooltip for the
+# LOCALIZATION NOTE (responsive.changeDevicePixelRatio): Tooltip for the
 # device pixel ratio dropdown when is enabled.
 responsive.changeDevicePixelRatio=Change device pixel ratio of the viewport
 
-# LOCALIZATION NOTE (responsive.devicePixelRatio.auto): tooltip for the device pixel ratio
+# LOCALIZATION NOTE (responsive.devicePixelRatio.auto): Tooltip for the device pixel ratio
 # dropdown when it is disabled because a device is selected.
 # The argument (%1$S) is the selected device (e.g. iPhone 6) that set
 # automatically the device pixel ratio value.
 responsive.devicePixelRatio.auto=Device pixel ratio automatically set by %1$S
 
 # LOCALIZATION NOTE (responsive.customDeviceName): Default value in a form to
 # add a custom device based on an arbitrary size (no association to an existing
 # device).
--- a/devtools/client/responsive.html/components/DeviceSelector.js
+++ b/devtools/client/responsive.html/components/DeviceSelector.js
@@ -6,128 +6,98 @@
 
 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 } = require("../utils/l10n");
 const Types = require("../types");
 
-const OPEN_DEVICE_MODAL_VALUE = "OPEN_DEVICE_MODAL";
+loader.lazyRequireGetter(this, "showMenu", "devtools/client/shared/components/menu/utils", true);
 
 class DeviceSelector extends PureComponent {
   static get propTypes() {
     return {
       devices: PropTypes.shape(Types.devices).isRequired,
       selectedDevice: PropTypes.string.isRequired,
       viewportId: PropTypes.number.isRequired,
       onChangeDevice: PropTypes.func.isRequired,
       onResizeViewport: PropTypes.func.isRequired,
       onUpdateDeviceModal: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
-    this.onSelectChange = this.onSelectChange.bind(this);
+    this.onShowDeviceMenu = this.onShowDeviceMenu.bind(this);
   }
 
-  onSelectChange({ target }) {
+  onShowDeviceMenu(event) {
     const {
       devices,
+      selectedDevice,
       viewportId,
       onChangeDevice,
       onResizeViewport,
       onUpdateDeviceModal,
     } = this.props;
 
-    if (target.value === OPEN_DEVICE_MODAL_VALUE) {
-      onUpdateDeviceModal(true, viewportId);
-      return;
-    }
+    const menuItems = [];
+
     for (const type of devices.types) {
       for (const device of devices[type]) {
-        if (device.name === target.value) {
-          onResizeViewport(viewportId, device.width, device.height);
-          onChangeDevice(viewportId, device, type);
-          return;
+        if (device.displayed) {
+          menuItems.push({
+            label: device.name,
+            type: "checkbox",
+            checked: selectedDevice === device.name,
+            click: () => {
+              onResizeViewport(viewportId, device.width, device.height);
+              onChangeDevice(viewportId, device, type);
+            },
+          });
         }
       }
     }
+
+    menuItems.sort(function(a, b) {
+      return a.label.localeCompare(b.label);
+    });
+
+    if (menuItems.length > 0) {
+      menuItems.push("-");
+    }
+
+    menuItems.push({
+      label: getStr("responsive.editDeviceList2"),
+      click: () => onUpdateDeviceModal(true, viewportId),
+    });
+
+    showMenu(menuItems, {
+      button: event.target,
+      useTopLevelWindow: true,
+    });
   }
 
   render() {
     const {
       devices,
       selectedDevice,
     } = this.props;
 
-    const options = [];
-    for (const type of devices.types) {
-      for (const device of devices[type]) {
-        if (device.displayed) {
-          options.push(device);
-        }
-      }
-    }
-
-    options.sort(function(a, b) {
-      return a.name.localeCompare(b.name);
-    });
-
-    let selectClass = "toolbar-dropdown";
-    if (selectedDevice) {
-      selectClass += " selected";
-    }
-
-    const state = devices.listState;
-    let listContent;
-
-    if (state == Types.loadableState.LOADED) {
-      listContent = [
-        dom.option({
-          value: "",
-          title: "",
-          disabled: true,
-          hidden: true,
+    return (
+      dom.button(
+        {
+          id: "device-selector",
+          className: "devtools-button devtools-dropdown-button",
+          disabled: devices.listState !== Types.loadableState.LOADED,
+          title: selectedDevice,
+          onClick: this.onShowDeviceMenu,
         },
-        getStr("responsive.noDeviceSelected")),
-        options.map(device => {
-          return dom.option({
-            key: device.name,
-            value: device.name,
-            title: "",
-          }, device.name);
-        }),
-        dom.option({
-          value: OPEN_DEVICE_MODAL_VALUE,
-          title: "",
-        }, getStr("responsive.editDeviceList"))];
-    } else if (state == Types.loadableState.LOADING
-      || state == Types.loadableState.INITIALIZED) {
-      listContent = [dom.option({
-        value: "",
-        title: "",
-        disabled: true,
-      }, getStr("responsive.deviceListLoading"))];
-    } else if (state == Types.loadableState.ERROR) {
-      listContent = [dom.option({
-        value: "",
-        title: "",
-        disabled: true,
-      }, getStr("responsive.deviceListError"))];
-    }
-
-    return dom.select(
-      {
-        id: "device-selector",
-        className: selectClass,
-        value: selectedDevice,
-        title: selectedDevice,
-        onChange: this.onSelectChange,
-        disabled: (state !== Types.loadableState.LOADED),
-      },
-      ...listContent
+        dom.span({ className: "title" },
+          selectedDevice || getStr("responsive.responsiveMode")
+        )
+      )
     );
   }
 }
 
 module.exports = DeviceSelector;
--- a/devtools/client/responsive.html/index.css
+++ b/devtools/client/responsive.html/index.css
@@ -7,17 +7,16 @@
 
 :root {
   --rdm-box-shadow: 0 4px 4px 0 rgba(155, 155, 155, 0.26);
   --submit-button-active-background-color: rgba(0,0,0,0.12);
   --submit-button-active-color: var(--theme-body-color);
   --viewport-color: #999797;
   --viewport-hover-color: var(--theme-body-color);
   --viewport-active-color: #3b3b3b;
-  --viewport-selection-arrow: url("chrome://devtools/skin/images/select-arrow.svg");
 }
 
 :root.theme-dark {
   --rdm-box-shadow: 0 4px 4px 0 rgba(105, 105, 105, 0.26);
   --submit-button-active-background-color: var(--theme-toolbar-hover-active);
   --submit-button-active-color: var(--theme-selection-color);
   --viewport-color: #c6ccd0;
   --viewport-hover-color: #dde1e4;
@@ -25,17 +24,16 @@
 }
 
 * {
   box-sizing: border-box;
 }
 
 :root,
 input,
-select,
 button {
   font-size: 12px;
 }
 
 html,
 body,
 #root {
   height: 100%;
@@ -85,80 +83,16 @@ body,
 .toolbar-button.selected {
   color: var(--viewport-active-color);
 }
 
 .toolbar-button:not(:disabled):hover {
   color: var(--viewport-hover-color);
 }
 
-select {
-  -moz-appearance: none;
-  color: var(--viewport-color);
-  border: none;
-  height: 100%;
-  padding: 0 8px;
-  text-align: center;
-  text-overflow: ellipsis;
-}
-
-select.selected {
-  color: var(--viewport-active-color);
-}
-
-select:not(:disabled):hover {
-  color: var(--viewport-hover-color);
-}
-
-/* This is (believed to be?) separate from the identical select.selected rule
-   set so that it overrides select:hover because of file ordering once the
-   select is focused.  It's unclear whether the visual effect that results here
-   is intentional and desired. */
-select:focus {
-  color: var(--viewport-active-color);
-}
-
-select > option {
-  text-align: left;
-  padding: 5px 10px;
-}
-
-select > option,
-select > option:hover {
-  color: var(--viewport-active-color);
-}
-
-select > option.divider {
-  border-top: 1px solid var(--theme-splitter-color);
-  height: 0px;
-  padding: 0;
-  font-size: 0px;
-}
-
-/**
- * Common background for dropdowns like select and toggle menu
- */
-
-.toolbar-dropdown,
-.toolbar-dropdown.devtools-button,
-.toolbar-dropdown.devtools-button:hover:not(:empty):not(:disabled):not(.checked) {
-  background-color: var(--theme-toolbar-background);
-  background-image: var(--viewport-selection-arrow);
-  background-position: 100% 50%;
-  background-repeat: no-repeat;
-  background-size: 7px;
-  -moz-context-properties: fill;
-  fill: currentColor;
-}
-
-.toolbar-dropdown {
-  background-position-x: right 5px;
-  padding-right: 15px;
-}
-
 /**
  * Toolbar
  */
 
 #toolbar {
   background-color: var(--theme-toolbar-background);
   border-bottom: 1px solid var(--theme-splitter-color);
   display: grid;
@@ -208,16 +142,28 @@ 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-selector {
+  align-self: center;
+  background-position: right 4px center;
+  margin-inline-start: 4px;
+  padding-left: 0;
+  width: 8em;
+}
+
+#device-selector .title {
+  width: 85%;
+}
+
 #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;
   background-position: right 4px center;
   padding-left: 0;