Bug 1623906 - [devtools] Dispatch thunks from Flexbox panel React components to highlight nodes r=gl,jdescottes,nchevobbe

### Problem
There is a lot of [prop drilling](https://kentcdodds.com/blog/prop-drilling) in React components to pass down a callback that is invoked at the bottom of a long chain of components.

[onShowBoxModelHighlighterForNode()](https://searchfox.org/mozilla-central/search?path=&q=onShowBoxModelHighlighterForNode) is one such example. It is created at the [Inspector client level](https://searchfox.org/mozilla-central/rev/25d491b7924493b5d4bedc07841a1cd92f271100/devtools/client/inspector/inspector.js#1885-1908) then passed to React components for panels, then drilled down to the consumer component which invokes it. There is also some [needless duplication](https://searchfox.org/mozilla-central/source/devtools/client/inspector/layout/layout.js#85-86) with [onShowBoxModelHighlighter](https://searchfox.org/mozilla-central/search?q=onShowBoxModelHighlighter%5CW&path=&case=false&regexp=true).

### Solution

With this patch, we leverage thunks in Redux.
In Redux, you can `dispatch()`:
- actions -> an object with an action type string which is matched by reducers
- thunks -> a function which can be async and can itself dispatch actions or other thunks

Thunks are supported by middleware already set up in the DevTools Redux [createStore()](https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/create-store.js#54,104) helper. During store setup, we pass `thunkOptions` to the middleware, an object with arguments which will be available to all thunks when invoked. This is where we pass in the inspector client as a thunk option so we can invoke the highlighter.

This is the replacement for prop drilling the `onShowBoxModelHighlighterForNode()` method. The same way they dispatch actions, components can now dispatch the thunk to show/hide the highlighter when they need it without direct knowledge of the `inspector`, thus satisfying the original intent of passing down the `onShowBoxModelHighlighterForNode()` callback while cleaning up the code.

### Prior art
Thunks are not something new to DevTools. They are extensively used by the WebConsole, for example:
- passing [thunk options to the store](https://searchfox.org/mozilla-central/rev/25d491b7924493b5d4bedc07841a1cd92f271100/devtools/client/webconsole/webconsole-wrapper.js#95-104)
- making use of thunk options,  [webConsoleUI and hud](https://searchfox.org/mozilla-central/source/devtools/client/webconsole/actions/autocomplete.js#14-91) in a thunk to handle autocomplete


###Useful context:
DevTools helpers
- [thunkWithOptions](https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/middleware/thunk-with-options.js)
- [shared Redux createStore helper with thunks middleware](https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/create-store.js#54,104)
Redux docs
- [Async Actions in Redux](https://redux.js.org/advanced/async-actions)
- [dispatch() as default prop with redux-connect](https://react-redux.js.org/using-react-redux/connect-mapdispatch#default-dispatch-as-a-prop)

Differential Revision: https://phabricator.services.mozilla.com/D79556

devtools/client/inspector/boxmodel/actions/box-model-highlighter.js

@@ -0,0 +1,48 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+/**
+ * This module exports thunks.
+ * Thunks are functions that can be dispatched to the Inspector Redux store.
+ *
+ * These functions receive one object with options that contains:
+ * - dispatch() => function to dispatch Redux actions to the store
+ * - getState() => function to get the current state of the entire Inspector Redux store
+ * - inspector => object instance of Inspector client
+ *
+ * They provide a shortcut for React components to invoke the box model highlighter
+ * without having to know where the highlighter exists.
+ */
+
+module.exports = {
+  /**
+   * Show the box model highlighter for the given node front.
+   *
+   * @param {NodeFront} nodeFront
+   *        Node that should be highlighted.
+   */
+  highlightNode(nodeFront) {
+    return async thunkOptions => {
+      const { inspector } = thunkOptions;
+      await inspector.highlighters.showHighlighterTypeForNode(
+        inspector.highlighters.TYPES.BOXMODEL,
+        nodeFront
+      );
+    };
+  },
+
+  /**
+   * Hide the box model highlighter for any highlighted node.
+   */
+  unhighlightNode() {
+    return async thunkOptions => {
+      const { inspector } = thunkOptions;
+      await inspector.highlighters.hideHighlighterType(
+        inspector.highlighters.TYPES.BOXMODEL
+      );
+    };
+  },
+};

devtools/client/inspector/boxmodel/actions/moz.build

@@ -5,6 +5,7 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DevToolsModules(
+    "box-model-highlighter.js",
     "box-model.js",
     "index.js",
 )

devtools/client/inspector/flexbox/components/FlexContainer.js

@@ -12,7 +12,6 @@ const {
 } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-const { connect } = require("devtools/client/shared/vendor/react-redux");
 
 loader.lazyRequireGetter(
   this,
@@ -22,15 +21,19 @@ loader.lazyRequireGetter(
 
 const Types = require("devtools/client/inspector/flexbox/types");
 
+const {
+  highlightNode,
+  unhighlightNode,
+} = require("devtools/client/inspector/boxmodel/actions/box-model-highlighter");
+
 class FlexContainer extends PureComponent {
   static get propTypes() {
     return {
+      dispatch: PropTypes.func.isRequired,
       color: PropTypes.string.isRequired,
       flexContainer: PropTypes.shape(Types.flexContainer).isRequired,
       getSwatchColorPickerTooltip: PropTypes.func.isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
       onSetFlexboxOverlayColor: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
     };
   }
 
@@ -69,12 +72,7 @@ class FlexContainer extends PureComponent {
   }
 
   render() {
-    const {
-      color,
-      flexContainer,
-      onHideBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
-    } = this.props;
+    const { color, flexContainer, dispatch } = this.props;
     const { nodeFront, properties } = flexContainer;
 
     return createElement(
@@ -85,8 +83,8 @@ class FlexContainer extends PureComponent {
           className: "flex-header-container-label",
         },
         getNodeRep(nodeFront, {
-          onDOMNodeMouseOut: () => onHideBoxModelHighlighter(),
-          onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront),
+          onDOMNodeMouseOut: () => dispatch(unhighlightNode()),
+          onDOMNodeMouseOver: () => dispatch(highlightNode(nodeFront)),
         }),
         dom.div({
           className: "layout-color-swatch",
@@ -121,10 +119,4 @@ class FlexContainer extends PureComponent {
   }
 }
 
-const mapStateToProps = state => {
-  return {
-    color: state.flexbox.color,
-  };
-};
-
-module.exports = connect(mapStateToProps)(FlexContainer);
+module.exports = FlexContainer;

devtools/client/inspector/flexbox/components/FlexItem.js

@@ -16,13 +16,17 @@ loader.lazyRequireGetter(
 
 const Types = require("devtools/client/inspector/flexbox/types");
 
+const {
+  highlightNode,
+  unhighlightNode,
+} = require("devtools/client/inspector/boxmodel/actions/box-model-highlighter");
+
 class FlexItem extends PureComponent {
   static get propTypes() {
     return {
+      dispatch: PropTypes.func.isRequired,
       flexItem: PropTypes.shape(Types.flexItem).isRequired,
       index: PropTypes.number.isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       scrollToTop: PropTypes.func.isRequired,
       setSelectedNode: PropTypes.func.isRequired,
     };
@@ -30,10 +34,9 @@ class FlexItem extends PureComponent {
 
   render() {
     const {
+      dispatch,
       flexItem,
       index,
-      onHideBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
       scrollToTop,
       setSelectedNode,
     } = this.props;
@@ -46,10 +49,10 @@ class FlexItem extends PureComponent {
           e.stopPropagation();
           scrollToTop();
           setSelectedNode(nodeFront);
-          onHideBoxModelHighlighter();
+          dispatch(unhighlightNode());
         },
-        onMouseOut: () => onHideBoxModelHighlighter(),
-        onMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront),
+        onMouseOut: () => dispatch(unhighlightNode()),
+        onMouseOver: () => dispatch(highlightNode(nodeFront)),
       },
       dom.span({ className: "flex-item-index" }, index),
       getNodeRep(nodeFront)

devtools/client/inspector/flexbox/components/FlexItemList.js

@@ -21,22 +21,15 @@ const Types = require("devtools/client/inspector/flexbox/types");
 class FlexItemList extends PureComponent {
   static get propTypes() {
     return {
+      dispatch: PropTypes.func.isRequired,
       flexItems: PropTypes.arrayOf(PropTypes.shape(Types.flexItem)).isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       scrollToTop: PropTypes.func.isRequired,
       setSelectedNode: PropTypes.func.isRequired,
     };
   }
 
   render() {
-    const {
-      flexItems,
-      onHideBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
-      scrollToTop,
-      setSelectedNode,
-    } = this.props;
+    const { dispatch, flexItems, scrollToTop, setSelectedNode } = this.props;
 
     return dom.div(
       { className: "flex-item-list" },
@@ -53,10 +46,9 @@ class FlexItemList extends PureComponent {
       flexItems.map((flexItem, index) =>
         FlexItem({
           key: flexItem.actorID,
+          dispatch,
           flexItem,
           index: index + 1,
-          onHideBoxModelHighlighter,
-          onShowBoxModelHighlighterForNode,
           scrollToTop,
           setSelectedNode,
         })

devtools/client/inspector/flexbox/components/Flexbox.js

@@ -40,12 +40,11 @@ const Types = require("devtools/client/inspector/flexbox/types");
 class Flexbox extends PureComponent {
   static get propTypes() {
     return {
+      dispatch: PropTypes.func.isRequired,
       flexbox: PropTypes.shape(Types.flexbox).isRequired,
       flexContainer: PropTypes.shape(Types.flexContainer).isRequired,
       getSwatchColorPickerTooltip: PropTypes.func.isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
       onSetFlexboxOverlayColor: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       onToggleFlexboxHighlighter: PropTypes.func.isRequired,
       scrollToTop: PropTypes.func.isRequired,
       setSelectedNode: PropTypes.func.isRequired,
@@ -53,18 +52,12 @@ class Flexbox extends PureComponent {
   }
 
   renderFlexItemList() {
-    const {
-      onHideBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
-      scrollToTop,
-      setSelectedNode,
-    } = this.props;
+    const { dispatch, scrollToTop, setSelectedNode } = this.props;
     const { flexItems } = this.props.flexContainer;
 
     return FlexItemList({
+      dispatch,
       flexItems,
-      onHideBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
       scrollToTop,
       setSelectedNode,
     });
@@ -96,11 +89,11 @@ class Flexbox extends PureComponent {
 
   render() {
     const {
+      dispatch,
+      flexbox,
       flexContainer,
       getSwatchColorPickerTooltip,
-      onHideBoxModelHighlighter,
       onSetFlexboxOverlayColor,
-      onShowBoxModelHighlighterForNode,
       onToggleFlexboxHighlighter,
       setSelectedNode,
     } = this.props;
@@ -113,15 +106,17 @@ class Flexbox extends PureComponent {
     }
 
     const { flexItemShown } = flexContainer;
+    const { color, highlighted } = flexbox;
 
     return dom.div(
       { className: "layout-flexbox-wrapper" },
       Header({
+        color,
+        dispatch,
         flexContainer,
         getSwatchColorPickerTooltip,
-        onHideBoxModelHighlighter,
+        highlighted,
         onSetFlexboxOverlayColor,
-        onShowBoxModelHighlighterForNode,
         onToggleFlexboxHighlighter,
         setSelectedNode,
       }),

devtools/client/inspector/flexbox/components/Header.js

@@ -12,7 +12,6 @@ const {
 } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-const { connect } = require("devtools/client/shared/vendor/react-redux");
 const { getStr } = require("devtools/client/inspector/layout/utils/l10n");
 
 const FlexContainer = createFactory(
@@ -27,12 +26,12 @@ const Types = require("devtools/client/inspector/flexbox/types");
 class Header extends PureComponent {
   static get propTypes() {
     return {
+      color: PropTypes.string.isRequired,
+      dispatch: PropTypes.func.isRequired,
       flexContainer: PropTypes.shape(Types.flexContainer).isRequired,
       getSwatchColorPickerTooltip: PropTypes.func.isRequired,
       highlighted: PropTypes.bool.isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
       onSetFlexboxOverlayColor: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       onToggleFlexboxHighlighter: PropTypes.func.isRequired,
       setSelectedNode: PropTypes.func.isRequired,
     };
@@ -75,19 +74,19 @@ class Header extends PureComponent {
     }
 
     const {
+      color,
+      dispatch,
       flexContainer,
       getSwatchColorPickerTooltip,
-      onHideBoxModelHighlighter,
       onSetFlexboxOverlayColor,
-      onShowBoxModelHighlighterForNode,
     } = this.props;
 
     return FlexContainer({
+      color,
+      dispatch,
       flexContainer,
       getSwatchColorPickerTooltip,
-      onHideBoxModelHighlighter,
       onSetFlexboxOverlayColor,
-      onShowBoxModelHighlighterForNode,
     });
   }
 
@@ -142,10 +141,4 @@ class Header extends PureComponent {
   }
 }
 
-const mapStateToProps = state => {
-  return {
-    highlighted: state.flexbox.highlighted,
-  };
-};
-
-module.exports = connect(mapStateToProps)(Header);
+module.exports = Header;

devtools/client/inspector/inspector.js

@@ -149,7 +149,7 @@ function Inspector(toolbox) {
   this.panelWin = window;
   this.panelWin.inspector = this;
   this.telemetry = toolbox.telemetry;
-  this.store = createStore();
+  this.store = createStore(this);
   this.isReady = false;
 
   // Map [panel id => panel instance]

devtools/client/inspector/layout/components/LayoutApp.js

@@ -52,6 +52,7 @@ class LayoutApp extends PureComponent {
   static get propTypes() {
     return {
       boxModel: PropTypes.shape(BoxModelTypes.boxModel).isRequired,
+      dispatch: PropTypes.func.isRequired,
       flexbox: PropTypes.shape(FlexboxTypes.flexbox).isRequired,
       getSwatchColorPickerTooltip: PropTypes.func.isRequired,
       grids: PropTypes.arrayOf(PropTypes.shape(GridTypes.grid)).isRequired,

devtools/client/inspector/store.js

@@ -20,11 +20,13 @@ function createReducer(laterReducers = {}) {
   });
 }
 
-module.exports = () => {
+module.exports = inspector => {
   const store = createStore(createReducer(), {
     // Enable log middleware in tests
     shouldLog: true,
-    thunkOptions: {},
+    // Pass the client inspector instance so thunks (dispatched functions)
+    // can access it from their arguments
+    thunkOptions: { inspector },
   });
 
   // Map of registered reducers loaded on-demand.