Bug 1444301 - Use a consistent order when listing the props of the ToolboxToolbar component; r?jryans draft
authorBrian Birtles <birtles@gmail.com>
Thu, 05 Apr 2018 10:13:21 +0900
changeset 778369 f89a01b063dce8f817b8f36dcc21cdaba3ab7d9b
parent 778368 76e8e9fa89bc7b1ea54cad153d0ee2833d808835
child 778370 8346aa0c5f4d09e5d58bbd662d6aec9a81ef2823
child 778568 518dee7641441dcad0a1f46e9f0b0b894defb82c
push id105480
push userbmo:bbirtles@mozilla.com
push dateFri, 06 Apr 2018 08:29:15 +0000
reviewersjryans
bugs1444301
milestone61.0a1
Bug 1444301 - Use a consistent order when listing the props of the ToolboxToolbar component; r?jryans MozReview-Commit-ID: GF5RTlS3uIA
devtools/client/framework/components/toolbox-controller.js
devtools/client/framework/components/toolbox-toolbar.js
--- a/devtools/client/framework/components/toolbox-controller.js
+++ b/devtools/client/framework/components/toolbox-controller.js
@@ -16,43 +16,43 @@ const ELEMENT_PICKER_ID = "command-butto
 class ToolboxController extends Component {
   constructor(props, context) {
     super(props, context);
 
     // See the ToolboxToolbar propTypes for documentation on each of these items in
     // state, and for the definitions of the props that are expected to be passed in.
     this.state = {
       focusedButton: ELEMENT_PICKER_ID,
+      toolboxButtons: [],
       currentToolId: null,
-      canRender: false,
       highlightedTools: new Set(),
-      areDockButtonsEnabled: true,
       panelDefinitions: [],
       hostTypes: [],
+      areDockButtonsEnabled: true,
       canCloseToolbox: true,
-      toolboxButtons: [],
+      canRender: false,
       buttonIds: [],
       checkedButtonsUpdated: () => {
         this.forceUpdate();
       }
     };
 
+    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.setDockButtonsEnabled = this.setDockButtonsEnabled.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);
-    this.setFocusedButton = this.setFocusedButton.bind(this);
-    this.setCurrentToolId = this.setCurrentToolId.bind(this);
-    this.setCanRender = this.setCanRender.bind(this);
-    this.setOptionsPanel = this.setOptionsPanel.bind(this);
-    this.highlightTool = this.highlightTool.bind(this);
-    this.unhighlightTool = this.unhighlightTool.bind(this);
-    this.setDockButtonsEnabled = this.setDockButtonsEnabled.bind(this);
-    this.setHostTypes = this.setHostTypes.bind(this);
-    this.setCanCloseToolbox = this.setCanCloseToolbox.bind(this);
-    this.setPanelDefinitions = this.setPanelDefinitions.bind(this);
-    this.setToolboxButtons = this.setToolboxButtons.bind(this);
   }
 
   shouldComponentUpdate() {
     return this.state.canRender;
   }
 
   componentWillUnmount() {
     this.state.toolboxButtons.forEach(button => {
@@ -60,18 +60,23 @@ 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 {panelDefinitions, toolboxButtons, optionsPanel, hostTypes,
-           canCloseToolbox} = this.state;
+    const {
+      toolboxButtons,
+      panelDefinitions,
+      optionsPanel,
+      hostTypes,
+      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,
--- a/devtools/client/framework/components/toolbox-toolbar.js
+++ b/devtools/client/framework/components/toolbox-toolbar.js
@@ -24,37 +24,37 @@ class ToolboxToolbar extends Component {
       // 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".
       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,
       // Do we need to add UI for closing the toolbox? We don't when the
       // toolbox is undocked, for example.
       canCloseToolbox: PropTypes.bool,
-      // 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,
-      })),
       // Function to select a tool based on its id.
       selectTool: PropTypes.func,
       // Function to completely close the toolbox.
       closeToolbox: PropTypes.func,
       // Keep a record of what button is focused.
       focusButton: PropTypes.func,
-      // The options button definition.
-      optionsPanel: PropTypes.object,
       // Hold off displaying the toolbar until enough information is ready for
       // it to render nicely.
       canRender: PropTypes.bool,
       // Localization interface.
       L10N: PropTypes.object,
       // The devtools toolbox
       toolbox: PropTypes.object,
     };
@@ -101,22 +101,22 @@ function renderToolboxButtonsEnd(props) 
 }
 
 /**
  * Render all of the tabs, this takes in a list of toolbox button states. These are plain
  * objects that have all of the relevant information needed to render the button.
  * See Toolbox.prototype._createButtonState in devtools/client/framework/toolbox.js for
  * documentation on this object.
  *
+ * @param {String} focusedButton - The id of the focused button.
  * @param {Array} toolboxButtons - Array of objects that define the command buttons.
- * @param {String} focusedButton - The id of the focused button.
  * @param {Function} focusButton - Keep a record of the currently focused button.
  * @param {boolean} isStart - Render either the starting buttons, or ending buttons.
  */
-function renderToolboxButtons({toolboxButtons, focusedButton, focusButton}, isStart) {
+function renderToolboxButtons({focusedButton, toolboxButtons, focusButton}, isStart) {
   const visibleButtons = toolboxButtons.filter(command => {
     const {isVisible, isInStartContainer} = command;
     return isVisible && (isStart ? isInStartContainer : !isInStartContainer);
   });
 
   if (visibleButtons.length === 0) {
     return null;
   }
@@ -151,28 +151,35 @@ function renderToolboxButtons({toolboxBu
         }
       });
     }),
     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 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 {Object}   optionsPanel - A single panel definition for the options panel.
- * @param {String}   currentToolId - The currently selected tool's id; e.g. "inspector".
- * @param {Function} selectTool - Function to select a tool in the toolbox.
- * @param {String}   focusedButton - The id of the focused button.
- * @param {Function} focusButton - Keep a record of the currently focused button.
- * @param {Object}   highlightedTools - Optionally highlighted tools, e.g. "inspector".
+ * @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({optionsPanel, currentToolId, selectTool, focusedButton,
-                        focusButton, highlightedTools}) {
+function renderOptions({focusedButton, currentToolId, highlightedTools,
+                        optionsPanel, selectTool, focusButton}) {
   return div({id: "toolbox-option-container"}, ToolboxTab({
     panelDefinition: optionsPanel,
     currentToolId,
     selectTool,
     highlightedTools,
     focusedButton,
     focusButton,
   }));
@@ -184,28 +191,36 @@ function renderOptions({optionsPanel, cu
 function renderSeparator() {
   return div({className: "devtools-separator"});
 }
 
 /**
  * Render the dock buttons, and handle all the cases for what type of host the toolbox
  * is attached to. The following props are expected.
  *
- * @property {String} focusedButton - The id of the focused button.
- * @property {Function} closeToolbox - Completely close the toolbox.
- * @property {Array} hostTypes - Array of host type objects, containing:
- *                   @property {String} position - Position name
- *                   @property {Function} switchHost - Function to switch the host.
- * @property {Function} focusButton - Keep a record of the currently focused button.
- * @property {Object} L10N - Localization interface.
- * @property {Boolean} areDockButtonsEnabled - They are not enabled in certain situations
- *                                             like when they are in the WebIDE.
- * @property {Boolean} canCloseToolbox - Are the tools in a context where they can be
- *                                       closed? This is not always the case, e.g. in the
- *                                       WebIDE.
+ * @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 {boolean} areDockButtonsEnabled
+ *        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} closeToolbox
+ *        Completely close the toolbox.
+ * @param {Function} focusButton
+ *        Keep a record of the currently focused button.
+ * @param {Object} L10N
+ *        Localization interface.
  */
 function renderDockButtons(props) {
   const {
     focusedButton,
     closeToolbox,
     hostTypes,
     focusButton,
     L10N,