Bug 1455443 - Convert dock menu options to a checkbox list; r?jryans draft
authorBrian Birtles <birtles@gmail.com>
Tue, 24 Apr 2018 13:22:02 +0900
changeset 787597 578ea885aff35e3873c94ce29a46857eb7cf7ed3
parent 787539 6eeb97ca94f40189d5aa552da9e0b0b11bfa0441
push id107754
push userbmo:bbirtles@mozilla.com
push dateWed, 25 Apr 2018 00:47:54 +0000
reviewersjryans
bugs1455443
milestone61.0a1
Bug 1455443 - Convert dock menu options to a checkbox list; r?jryans This also fixes the grouping so that the checkboxes appear before the separator as per the mockup here: https://mozilla.invisionapp.com/share/M5G8OO1ZVE4#/screens MozReview-Commit-ID: FfVNzPHEk43
devtools/client/framework/components/toolbox-controller.js
devtools/client/framework/components/toolbox-toolbar.js
devtools/client/framework/toolbox.js
devtools/client/locales/en-US/toolbox.properties
--- a/devtools/client/framework/components/toolbox-controller.js
+++ b/devtools/client/framework/components/toolbox-controller.js
@@ -21,16 +21,17 @@ class ToolboxController extends Componen
     // state, and for the definitions of the props that are expected to be passed in.
     this.state = {
       focusedButton: ELEMENT_PICKER_ID,
       toolboxButtons: [],
       currentToolId: null,
       highlightedTools: new Set(),
       panelDefinitions: [],
       hostTypes: [],
+      currentHostType: undefined,
       areDockOptionsEnabled: true,
       canCloseToolbox: true,
       isSplitConsoleActive: false,
       disableAutohide: undefined,
       canRender: false,
       buttonIds: [],
       checkedButtonsUpdated: () => {
         this.forceUpdate();
@@ -38,16 +39,17 @@ class ToolboxController extends Componen
     };
 
     this.setFocusedButton = this.setFocusedButton.bind(this);
     this.setToolboxButtons = this.setToolboxButtons.bind(this);
     this.setCurrentToolId = this.setCurrentToolId.bind(this);
     this.highlightTool = this.highlightTool.bind(this);
     this.unhighlightTool = this.unhighlightTool.bind(this);
     this.setHostTypes = this.setHostTypes.bind(this);
+    this.setCurrentHostType = this.setCurrentHostType.bind(this);
     this.setDockOptionsEnabled = this.setDockOptionsEnabled.bind(this);
     this.setCanCloseToolbox = this.setCanCloseToolbox.bind(this);
     this.setIsSplitConsoleActive = this.setIsSplitConsoleActive.bind(this);
     this.setDisableAutohide = this.setDisableAutohide.bind(this);
     this.setCanRender = this.setCanRender.bind(this);
     this.setPanelDefinitions = this.setPanelDefinitions.bind(this);
     this.updateButtonIds = this.updateButtonIds.bind(this);
     this.updateFocusedButton = this.updateFocusedButton.bind(this);
@@ -132,16 +134,20 @@ class ToolboxController extends Componen
   setDockOptionsEnabled(areDockOptionsEnabled) {
     this.setState({ areDockOptionsEnabled });
   }
 
   setHostTypes(hostTypes) {
     this.setState({ hostTypes });
   }
 
+  setCurrentHostType(currentHostType) {
+    this.setState({ currentHostType });
+  }
+
   setCanCloseToolbox(canCloseToolbox) {
     this.setState({ canCloseToolbox }, this.updateButtonIds);
   }
 
   setIsSplitConsoleActive(isSplitConsoleActive) {
     this.setState({ isSplitConsoleActive });
   }
 
--- a/devtools/client/framework/components/toolbox-toolbar.js
+++ b/devtools/client/framework/components/toolbox-toolbar.js
@@ -34,16 +34,19 @@ class ToolboxToolbar extends Component {
       highlightedTools: PropTypes.instanceOf(Set),
       // List of tool panel definitions (used by ToolboxTabs component).
       panelDefinitions: PropTypes.array,
       // List of possible docking options.
       hostTypes: PropTypes.arrayOf(PropTypes.shape({
         position: PropTypes.string.isRequired,
         switchHost: PropTypes.func.isRequired,
       })),
+      // Current docking type. Typically one of the position values in
+      // |hostTypes| but this is not always the case (e.g. when it is "custom").
+      currentHostType: PropTypes.string,
       // Should the docking options be enabled? They are disabled in some
       // contexts such as WebIDE.
       areDockButtonsEnabled: PropTypes.bool,
       // Do we need to add UI for closing the toolbox? We don't when the
       // toolbox is undocked, for example.
       canCloseToolbox: PropTypes.bool,
       // Is the split console currently visible?
       isSplitConsoleActive: PropTypes.bool,
@@ -207,16 +210,18 @@ function renderSeparator() {
  * @param {string} focusedButton
  *        The id of the focused button.
  * @param {Object[]} hostTypes
  *        Array of host type objects.
  * @param {string} hostTypes[].position
  *        Position name.
  * @param {Function} hostTypes[].switchHost
  *        Function to switch the host.
+ * @param {string} currentHostType
+ *        The current docking configuration.
  * @param {boolean} areDockOptionsEnabled
  *        They are not enabled in certain situations like when they are in the
  *        WebIDE.
  * @param {boolean} canCloseToolbox
  *        Do we need to add UI for closing the toolbox? We don't when the
  *        toolbox is undocked, for example.
  * @param {boolean} isSplitConsoleActive
  *         Is the split console currently visible?
@@ -293,22 +298,24 @@ function renderToolboxControls(props) {
  *        The <button> element from which the menu should pop out. The geometry
  *        of this element is used to position the menu.
  * @param {Object} props
  *        Properties as described below.
  * @param {string} props.currentToolId
  *        The id of the currently selected tool.
  * @param {Object[]} props.hostTypes
  *        Array of host type objects.
+ *        This array will be empty if we shouldn't shouldn't show any dock
+ *        options.
  * @param {string} props.hostTypes[].position
  *        Position name.
  * @param {Function} props.hostTypes[].switchHost
  *        Function to switch the host.
- *        This array will be empty if we shouldn't shouldn't show any dock
- *        options.
+ * @param {string} props.currentHostType
+ *        The current docking configuration.
  * @param {boolean} isSplitConsoleActive
  *        Is the split console currently visible?
  * @param {boolean|undefined} disableAutohide
  *        Are we disabling the behavior where pop-ups are automatically
  *        closed when clicking outside them.
  *        (Only defined for the browser toolbox.)
  * @param {Function} props.selectTool
  *        Function to select a tool based on its id.
@@ -322,36 +329,47 @@ function renderToolboxControls(props) {
  *        The devtools toolbox. Used by the Menu component to determine which
  *        document to use.
  */
 function showMeatballMenu(
   menuButton,
   {
     currentToolId,
     hostTypes,
+    currentHostType,
     isSplitConsoleActive,
     disableAutohide,
     selectTool,
     toggleSplitConsole,
     toggleNoAutohide,
     L10N,
     toolbox,
   }
 ) {
   const menu = new Menu({ id: "toolbox-meatball-menu" });
 
   // Dock options
   for (const hostType of hostTypes) {
-    menu.append(new MenuItem({
-      id: `toolbox-meatball-menu-dock-${hostType.position}`,
-      label: L10N.getStr(
-        `toolbox.meatballMenu.dock.${hostType.position}.label`
-      ),
-      click: () => hostType.switchHost(),
-    }));
+    const l10nkey =
+      hostType.position === "window"
+        ? "separateWindow"
+        : hostType.position;
+    menu.append(
+      new MenuItem({
+        id: `toolbox-meatball-menu-dock-${hostType.position}`,
+        label: L10N.getStr(`toolbox.meatballMenu.dock.${l10nkey}.label`),
+        click: () => hostType.switchHost(),
+        type: "checkbox",
+        checked: hostType.position === currentHostType,
+      })
+    );
+  }
+
+  if (menu.items.length) {
+    menu.append(new MenuItem({ type: "separator" }));
   }
 
   // Split console
   if (currentToolId !== "webconsole") {
     menu.append(new MenuItem({
       id: "toolbox-meatball-menu-splitconsole",
       label: L10N.getStr(
         `toolbox.meatballMenu.${
@@ -372,20 +390,16 @@ function showMeatballMenu(
       id: "toolbox-meatball-menu-noautohide",
       label: L10N.getStr("toolbox.meatballMenu.noautohide.label"),
       type: "checkbox",
       checked: disableAutohide,
       click: toggleNoAutohide,
     }));
   }
 
-  if (menu.items.length) {
-    menu.append(new MenuItem({ type: "separator" }));
-  }
-
   // Settings
   menu.append(new MenuItem({
     id: "toolbox-meatball-menu-settings",
     label: L10N.getStr("toolbox.meatballMenu.settings.label"),
     accelerator: L10N.getStr("toolbox.help.key"),
     click: () => selectTool("options"),
   }));
 
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1089,28 +1089,28 @@ Toolbox.prototype = {
     this.component.setDockOptionsEnabled(true);
     this.component.setCanCloseToolbox(this.hostType !== Toolbox.HostType.WINDOW);
 
     let sideEnabled = Services.prefs.getBoolPref(this._prefs.SIDE_ENABLED);
 
     let hostTypes = [];
     for (let type in Toolbox.HostType) {
       let position = Toolbox.HostType[type];
-      if (position == this.hostType ||
-          position == Toolbox.HostType.CUSTOM ||
+      if (position == Toolbox.HostType.CUSTOM ||
           (!sideEnabled && position == Toolbox.HostType.SIDE)) {
         continue;
       }
 
       hostTypes.push({
         position,
         switchHost: this.switchHost.bind(this, position)
       });
     }
 
+    this.component.setCurrentHostType(this.hostType);
     this.component.setHostTypes(hostTypes);
   },
 
   postMessage: function(msg) {
     // We sometime try to send messages in middle of destroy(), where the
     // toolbox iframe may already be detached and no longer have a parent.
     if (this.win.parent) {
       // Toolbox document is still chrome and disallow identifying message
@@ -2440,16 +2440,18 @@ Toolbox.prototype = {
     this._addKeysToWindow();
 
     // We blurred the tools at start of switchHost, but also when clicking on
     // host switching button. We now have to restore the focus.
     this.focusTool(this.currentToolId, true);
 
     this.emit("host-changed");
     this._telemetry.log(HOST_HISTOGRAM, this._getTelemetryHostId());
+
+    this.component.setCurrentHostType(hostType);
   },
 
   /**
    * Test the availability of a tool (both globally registered tools and
    * additional tools registered to this toolbox) by tool id.
    *
    * @param  {string} toolId
    *         Id of the tool definition to search in the per-toolbox or globally
--- a/devtools/client/locales/en-US/toolbox.properties
+++ b/devtools/client/locales/en-US/toolbox.properties
@@ -146,17 +146,17 @@ toolbox.showFrames.key=Alt+Down
 # for the "..." button on the developer tools toolbox.
 toolbox.meatballMenu.button.tooltip=Customize Developer Tools and get help
 
 # LOCALIZATION NOTE (toolbox.meatballMenu.dock.*.label): These labels are shown
 # in the "..." menu in the toolbox and represent the different arrangements for
 # docking (or undocking) the developer tools toolbox.
 toolbox.meatballMenu.dock.bottom.label=Dock to bottom
 toolbox.meatballMenu.dock.side.label=Dock to side
-toolbox.meatballMenu.dock.window.label=Undock
+toolbox.meatballMenu.dock.separateWindow.label=Separate window
 
 # LOCALIZATION NOTE (toolbox.meatballMenu.{splitconsole,hideconsole}.label):
 # These are the labels in the "..." menu in the toolbox for toggling the split
 # console window.
 # The keyboard shortcut will be shown to the side of the label.
 toolbox.meatballMenu.splitconsole.label=Show split console
 toolbox.meatballMenu.hideconsole.label=Hide split console