Bug 1444301 - Move Options button into meatball menu; r?jryans
MozReview-Commit-ID: HnTbtdI5gS6
--- a/devtools/client/debugger/test/mochitest/browser_dbg_worker-window.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_worker-window.js
@@ -46,17 +46,17 @@ add_task(function* () {
});
});
ok(toolbox.win.parent.document.title.includes(WORKER_URL),
"worker URL in host title");
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);
yield waitForWorkerClose(workerClient);
yield close(client);
yield toolbox.destroy();
});
--- a/devtools/client/framework/components/toolbox-controller.js
+++ b/devtools/client/framework/components/toolbox-controller.js
@@ -35,17 +35,16 @@ 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.setOptionsPanel = this.setOptionsPanel.bind(this);
this.setHostTypes = this.setHostTypes.bind(this);
this.setDockOptionsEnabled = this.setDockOptionsEnabled.bind(this);
this.setCanCloseToolbox = this.setCanCloseToolbox.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);
}
@@ -63,27 +62,25 @@ class ToolboxController extends Componen
/**
* The button and tab ids must be known in order to be able to focus left and right
* using the arrow keys.
*/
updateButtonIds() {
const {
toolboxButtons,
panelDefinitions,
- optionsPanel,
canCloseToolbox,
} = this.state;
// This is a little gnarly, but go through all of the state and extract the IDs.
this.setState({
buttonIds: [
...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)
});
this.updateFocusedButton();
}
updateFocusedButton() {
@@ -109,20 +106,16 @@ class ToolboxController extends Componen
this.setFocusedButton(currentToolId);
});
}
setCanRender() {
this.setState({ canRender: true }, this.updateButtonIds);
}
- setOptionsPanel(optionsPanel) {
- this.setState({ optionsPanel }, this.updateButtonIds);
- }
-
highlightTool(highlightedTool) {
let { highlightedTools } = this.state;
highlightedTools.add(highlightedTool);
this.setState({ highlightedTools });
}
unhighlightTool(id) {
let { highlightedTools } = this.state;
--- a/devtools/client/framework/components/toolbox-toolbar.js
+++ b/devtools/client/framework/components/toolbox-toolbar.js
@@ -24,22 +24,21 @@ class ToolboxToolbar extends Component {
return {
// The currently focused item (for arrow keyboard navigation)
// This ID determines the tabindex being 0 or -1.
focusedButton: PropTypes.string,
// List of command button definitions.
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,
switchHost: PropTypes.func.isRequired,
})),
// Should the docking options be enabled? They are disabled in some
// contexts such as WebIDE.
areDockButtonsEnabled: PropTypes.bool,
@@ -70,17 +69,16 @@ class ToolboxToolbar extends Component {
const containerProps = {className: "devtools-tabbar"};
return this.props.canRender
? (
div(
containerProps,
renderToolboxButtonsStart(this.props),
ToolboxTabs(this.props),
renderToolboxButtonsEnd(this.props),
- renderOptions(this.props),
renderSeparator(),
renderToolboxControls(this.props)
)
)
: div(containerProps);
}
}
@@ -153,46 +151,16 @@ function renderToolboxButtons({focusedBu
}
});
}),
isStart ? div({className: "devtools-separator"}) : null
);
}
/**
- * 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.
*/
function renderSeparator() {
return div({className: "devtools-separator"});
}
/**
* Render the toolbox control buttons. The following props are expected:
@@ -206,16 +174,18 @@ function renderSeparator() {
* @param {Function} hostTypes[].switchHost
* Function to switch the host.
* @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 {Function} selectTool
+ * Function to select a tool based on its id.
* @param {Function} closeToolbox
* Completely close the toolbox.
* @param {Function} focusButton
* Keep a record of the currently focused button.
* @param {Object} L10N
* Localization interface.
*/
function renderToolboxControls(props) {
@@ -277,37 +247,49 @@ function renderToolboxControls(props) {
* @param {Object[]} props.hostTypes
* Array of host type objects.
* @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 {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}`,
label: L10N.getStr(
`toolbox.meatballMenu.dock.${hostType.position}.label`
),
click: () => hostType.switchHost(),
}));
}
- // (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;
const screenY = menuButton.ownerDocument.defaultView.mozInnerScreenY;
// Display the popup below the button.
menu.popup(rect.left + screenX, rect.bottom + screenY, toolbox);
}
--- a/devtools/client/framework/test/browser_toolbox_tabsswitch_shortcuts.js
+++ b/devtools/client/framework/test/browser_toolbox_tabsswitch_shortcuts.js
@@ -13,17 +13,21 @@ const {LocalizationHelper} = require("de
const L10N = new LocalizationHelper("devtools/client/locales/toolbox.properties");
add_task(async function () {
let tab = await addTab("about:blank");
let target = TargetFactory.forTab(tab);
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);
let nextShortcut = L10N.getStr("toolbox.nextTool.key");
let prevShortcut = L10N.getStr("toolbox.previousTool.key");
// Iterate over all tools, starting from options to netmonitor, in normal
// order.
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1098,20 +1098,16 @@ Toolbox.prototype = {
// Get the initial list of tab definitions. This list can be amended at a later time
// by tools registering themselves.
const definitions = gDevTools.getToolDefinitionArray();
definitions.forEach(definition => this._buildPanelForTool(definition));
// 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() {
// Ensure the toolbar doesn't try to render until the tool is ready.
const element = this.React.createElement(this.ToolboxController, {
L10N,
currentToolId: this.currentToolId,
selectTool: this.selectTool,
@@ -1959,35 +1955,31 @@ Toolbox.prototype = {
},
/**
* Loads the tool next to the currently selected tool.
*/
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);
},
/**
* Loads the tool just left to the currently selected tool.
*/
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);
},
/**
* Highlights the tool's tab if it is not the currently selected tool.
*
* @param {string} id
* The id of the tool to highlight
--- a/devtools/client/locales/en-US/toolbox.properties
+++ b/devtools/client/locales/en-US/toolbox.properties
@@ -158,16 +158,22 @@ toolbox.meatballMenu.button.tooltip=Cust
# 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
+# 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
# LOCALIZATION NOTE (toolbox.allToolsButton.tooltip): This is the tooltip for the
# "all tools" button displayed when some tools are hidden by overflow of the toolbar.
toolbox.allToolsButton.tooltip=Select another tool
--- a/devtools/client/themes/toolbox.css
+++ b/devtools/client/themes/toolbox.css
@@ -54,17 +54,16 @@
flex: 1;
overflow: hidden;
}
/* Set flex attribute to Toolbox buttons and Picker container so,
they don't overlap with the tab bar */
#toolbox-buttons-start,
#toolbox-buttons-end,
-#toolbox-option-container,
#toolbox-controls {
display: flex;
align-items: stretch;
}
/* Toolbox tabs */
.devtools-tab {
@@ -155,27 +154,16 @@
.devtools-tab.selected > img {
fill: var(--theme-toolbar-selected-color);
}
.devtools-tab.highlighted > img {
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 */
#toolbox-close::before {
background-image: var(--close-button-image);
}
#toolbox-meatball-menu-button::before {