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

--HG--
extra : rebase_source : 50f1060ab65c307a5474b40ebfb7bfaf8649070d

devtools/client/framework/components/toolbox-controller.js

@@ -26,6 +26,7 @@ class ToolboxController extends Component {
       highlightedTools: new Set(),
       panelDefinitions: [],
       hostTypes: [],
+      currentHostType: undefined,
       areDockOptionsEnabled: true,
       canCloseToolbox: true,
       isSplitConsoleActive: false,
@@ -43,6 +44,7 @@ class ToolboxController extends Component {
     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);
@@ -137,6 +139,10 @@ class ToolboxController extends Component {
     this.setState({ hostTypes });
   }
 
+  setCurrentHostType(currentHostType) {
+    this.setState({ currentHostType });
+  }
+
   setCanCloseToolbox(canCloseToolbox) {
     this.setState({ canCloseToolbox }, this.updateButtonIds);
   }

devtools/client/framework/components/toolbox-toolbar.js

@@ -39,6 +39,9 @@ class ToolboxToolbar extends Component {
         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,
@@ -212,6 +215,8 @@ function renderSeparator() {
  *        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.
@@ -298,12 +303,14 @@ function renderToolboxControls(props) {
  *        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
+ * @param {string} props.currentHostType
- *        options.
+ *        The current docking configuration.
  * @param {boolean} isSplitConsoleActive
  *        Is the split console currently visible?
  * @param {boolean|undefined} disableAutohide
@@ -327,6 +334,7 @@ function showMeatballMenu(
   {
     currentToolId,
     hostTypes,
+    currentHostType,
     isSplitConsoleActive,
     disableAutohide,
     selectTool,
@@ -340,13 +348,23 @@ function showMeatballMenu(
 
   // Dock options
   for (const hostType of hostTypes) {
-    menu.append(new MenuItem({
+    const l10nkey =
-      id: `toolbox-meatball-menu-dock-${hostType.position}`,
+      hostType.position === "window"
-      label: L10N.getStr(
+        ? "separateWindow"
-        `toolbox.meatballMenu.dock.${hostType.position}.label`
+        : hostType.position;
-      ),
+    menu.append(
-      click: () => hostType.switchHost(),
+      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
@@ -377,10 +395,6 @@ function showMeatballMenu(
     }));
   }
 
-  if (menu.items.length) {
-    menu.append(new MenuItem({ type: "separator" }));
-  }
-
   // Settings
   menu.append(new MenuItem({
     id: "toolbox-meatball-menu-settings",

devtools/client/framework/toolbox.js

@@ -1094,8 +1094,7 @@ Toolbox.prototype = {
     let hostTypes = [];
     for (let type in Toolbox.HostType) {
       let position = Toolbox.HostType[type];
-      if (position == this.hostType ||
+      if (position == Toolbox.HostType.CUSTOM ||
-          position == Toolbox.HostType.CUSTOM ||
           (!sideEnabled && position == Toolbox.HostType.SIDE)) {
         continue;
       }
@@ -1106,6 +1105,7 @@ Toolbox.prototype = {
       });
     }
 
+    this.component.setCurrentHostType(this.hostType);
     this.component.setHostTypes(hostTypes);
   },
 
@@ -2445,6 +2445,8 @@ Toolbox.prototype = {
 
     this.emit("host-changed");
     this._telemetry.log(HOST_HISTOGRAM, this._getTelemetryHostId());
+
+    this.component.setCurrentHostType(hostType);
   },
 
   /**

devtools/client/locales/en-US/toolbox.properties

@@ -151,7 +151,7 @@ toolbox.meatballMenu.button.tooltip=Customize Developer Tools and get help
 # 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