Bug 1444301 - Move Options button into meatball menu; r=jryans

MozReview-Commit-ID: HnTbtdI5gS6

--HG--
extra : rebase_source : 21fcdaf882e050985db6d9b2004418a4c7df8b72

devtools/client/debugger/test/mochitest/browser_dbg_worker-window.js

@@ -51,7 +51,7 @@ add_task(function* () {
   let toolTabs = toolbox.doc.querySelectorAll(".devtools-tab");
   let activeTools = [...toolTabs].map(tab=>tab.getAttribute("data-id"));
 
-  is(activeTools.join(","), "webconsole,jsdebugger,scratchpad,options",
+  is(activeTools.join(","), "webconsole,jsdebugger,scratchpad",
     "Correct set of tools supported by worker");
 
   terminateWorkerInTab(tab, WORKER_URL);

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

@@ -40,7 +40,6 @@ class ToolboxController extends Component {
     this.setCurrentToolId = this.setCurrentToolId.bind(this);
     this.highlightTool = this.highlightTool.bind(this);
     this.unhighlightTool = this.unhighlightTool.bind(this);
-    this.setOptionsPanel = this.setOptionsPanel.bind(this);
     this.setHostTypes = this.setHostTypes.bind(this);
     this.setDockOptionsEnabled = this.setDockOptionsEnabled.bind(this);
     this.setCanCloseToolbox = this.setCanCloseToolbox.bind(this);
@@ -68,7 +67,6 @@ class ToolboxController extends Component {
     const {
       toolboxButtons,
       panelDefinitions,
-      optionsPanel,
       canCloseToolbox,
     } = this.state;
 
@@ -78,7 +76,6 @@ class ToolboxController extends Component {
         ...toolboxButtons.filter(btn => btn.isInStartContainer).map(({id}) => id),
         ...panelDefinitions.map(({id}) => id),
         ...toolboxButtons.filter(btn => !btn.isInStartContainer).map(({id}) => id),
-        optionsPanel ? optionsPanel.id : null,
         canCloseToolbox ? "toolbox-close" : null
       ].filter(id => id)
     });
@@ -114,10 +111,6 @@ class ToolboxController extends Component {
     this.setState({ canRender: true }, this.updateButtonIds);
   }
 
-  setOptionsPanel(optionsPanel) {
-    this.setState({ optionsPanel }, this.updateButtonIds);
-  }
-
   highlightTool(highlightedTool) {
     let { highlightedTools } = this.state;
     highlightedTools.add(highlightedTool);

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

@@ -29,12 +29,11 @@ class ToolboxToolbar extends Component {
       toolboxButtons: PropTypes.array,
       // The id of the currently selected tool, e.g. "inspector"
       currentToolId: PropTypes.string,
-      // An optionally highlighted tools, e.g. "inspector".
+      // An optionally highlighted tools, e.g. "inspector" (used by ToolboxTabs
+      // component).
       highlightedTools: PropTypes.instanceOf(Set),
       // List of tool panel definitions (used by ToolboxTabs component).
       panelDefinitions: PropTypes.array,
-      // The options panel definition.
-      optionsPanel: PropTypes.object,
       // List of possible docking options.
       hostTypes: PropTypes.arrayOf(PropTypes.shape({
         position: PropTypes.string.isRequired,
@@ -75,7 +74,6 @@ class ToolboxToolbar extends Component {
           renderToolboxButtonsStart(this.props),
           ToolboxTabs(this.props),
           renderToolboxButtonsEnd(this.props),
-          renderOptions(this.props),
           renderSeparator(),
           renderToolboxControls(this.props)
         )
@@ -157,36 +155,6 @@ function renderToolboxButtons({focusedButton, toolboxButtons, focusButton}, isSt
   );
 }
 
-/**
- * The options button is a ToolboxTab just like in the ToolboxTabs component.
- * However it is separate from the normal tabs, so deal with it separately here.
- * The following props are expected.
- *
- * @param {string} focusedButton
- *        The id of the focused button.
- * @param {string} currentToolId
- *        The currently selected tool's id; e.g.  "inspector".
- * @param {Object} highlightedTools
- *        Optionally highlighted tools, e.g. "inspector".
- * @param {Object} optionsPanel
- *        A single panel definition for the options panel.
- * @param {Function} selectTool
- *        Function to select a tool in the toolbox.
- * @param {Function} focusButton
- *        Keep a record of the currently focused button.
- */
-function renderOptions({focusedButton, currentToolId, highlightedTools,
-                        optionsPanel, selectTool, focusButton}) {
-  return div({id: "toolbox-option-container"}, ToolboxTab({
-    panelDefinition: optionsPanel,
-    currentToolId,
-    selectTool,
-    highlightedTools,
-    focusedButton,
-    focusButton,
-  }));
-}
-
 /**
  * Render a separator.
  */
@@ -211,6 +179,8 @@ function renderSeparator() {
  * @param {boolean} canCloseToolbox
  *        Do we need to add UI for closing the toolbox? We don't when the
  *        toolbox is undocked, for example.
+ * @param {Function} selectTool
+ *        Function to select a tool based on its id.
  * @param {Function} closeToolbox
  *        Completely close the toolbox.
  * @param {Function} focusButton
@@ -282,15 +252,18 @@ function renderToolboxControls(props) {
  *        Function to switch the host.
  *        This array will be empty if we shouldn't shouldn't show any dock
  *        options.
+ * @param {Function} props.selectTool
+ *        Function to select a tool based on its id.
  * @param {Object} props.L10N
  *        Localization interface.
  * @param {Object} props.toolbox
  *        The devtools toolbox. Used by the Menu component to determine which
  *        document to use.
  */
-function showMeatballMenu(menuButton, {hostTypes, L10N, toolbox}) {
+function showMeatballMenu(menuButton, {hostTypes, selectTool, 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}`,
@@ -301,8 +274,17 @@ function showMeatballMenu(menuButton, {hostTypes, L10N, toolbox}) {
     }));
   }
 
-  // (Yes, it's true we might end up with an empty menu here. Don't worry,
-  // by the end of this patch series that won't be the case.)
+  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"),
+    // TODO: Show "F1" acceltext once MenuItem supports it.
+    click: () => selectTool("options"),
+  }));
 
   const rect = menuButton.getBoundingClientRect();
   const screenX = menuButton.ownerDocument.defaultView.mozInnerScreenX;

devtools/client/framework/test/browser_toolbox_tabsswitch_shortcuts.js

@@ -18,7 +18,11 @@ add_task(async function() {
   await target.makeRemote();
 
   let toolIDs = gDevTools.getToolDefinitionArray()
-                         .filter(def => def.isTargetSupported(target))
+                         .filter(
+                           def =>
+                             def.isTargetSupported(target) &&
+                             def.id !== "options"
+                         )
                          .map(def => def.id);
 
   let toolbox = await gDevTools.showToolbox(target, toolIDs[0], Toolbox.HostType.BOTTOM);

devtools/client/framework/toolbox.js

@@ -1104,10 +1104,6 @@ Toolbox.prototype = {
     // Get the definitions that will only affect the main tab area.
     this.panelDefinitions = definitions.filter(definition =>
       definition.isTargetSupported(this._target) && definition.id !== "options");
-
-    this.optionsDefinition = definitions.find(({id}) => id === "options");
-    // The options tool is treated slightly differently, and is in a different area.
-    this.component.setOptionsPanel(definitions.find(({id}) => id === "options"));
   },
 
   _mountReactComponent: function() {
@@ -1965,10 +1961,9 @@ Toolbox.prototype = {
   selectNextTool: function() {
     let definitions = this.component.panelDefinitions;
     const index = definitions.findIndex(({id}) => id === this.currentToolId);
-    let definition = definitions[index + 1];
-    if (!definition) {
-      definition = index === -1 ? definitions[0] : this.optionsDefinition;
-    }
+    let definition = index === -1 || index >= definitions.length - 1
+                     ? definitions[0]
+                     : definitions[index + 1];
     return this.selectTool(definition.id);
   },
 
@@ -1978,12 +1973,9 @@ Toolbox.prototype = {
   selectPreviousTool: function() {
     let definitions = this.component.panelDefinitions;
     const index = definitions.findIndex(({id}) => id === this.currentToolId);
-    let definition = definitions[index - 1];
-    if (!definition) {
-      definition = index === -1
-        ? definitions[definitions.length - 1]
-        : this.optionsDefinition;
-    }
+    let definition = index === -1 || index < 1
+                     ? definitions[definitions.length - 1]
+                     : definitions[index - 1];
     return this.selectTool(definition.id);
   },
 

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

@@ -159,6 +159,12 @@ toolbox.meatballMenu.dock.bottom.label=Dock to bottom
 toolbox.meatballMenu.dock.side.label=Dock to side
 toolbox.meatballMenu.dock.window.label=Undock
 
+# LOCALIZATION NOTE (toolbox.meatballMenu.settings.label): This is the label for
+# the item in the "..." menu in the toolbox that brings up the Settings
+# (Options) panel.
+# The keyboard shortcut will be shown to the side of the label.
+toolbox.meatballMenu.settings.label=Settings
+
 # LOCALIZATION NOTE (toolbox.closebutton.tooltip): This is the tooltip for
 # the close button the developer tools toolbox.
 toolbox.closebutton.tooltip=Close Developer Tools

devtools/client/themes/toolbox.css

@@ -59,7 +59,6 @@
    they don't overlap with the tab bar */
 #toolbox-buttons-start,
 #toolbox-buttons-end,
-#toolbox-option-container,
 #toolbox-controls {
   display: flex;
   align-items: stretch;
@@ -160,17 +159,6 @@
   fill: var(--theme-toolbar-highlighted-color);
 }
 
-/* The options tab is special - it doesn't have the same parent
-   as the other tabs (toolbox-option-container vs toolbox-tabs) */
-#toolbox-option-container .devtools-tab {
-  border-color: transparent;
-  border-width: 0;
-  padding-inline-start: 1px;
-}
-#toolbox-option-container img {
-  margin: 0 3px;
-}
-
 /* Toolbox controls */