Bug 1623906 - [devtools] Dispatch thunks from Box Model panel React components to show/hide the Box Model Highlighter r=gl

Depends on D79556

This patch is using the approach introduced in D79556 to dispatch thunks (functions) to the Redux store to show/hide the box model highlighter from individual React components instead of passing down the methods through deeply nested trees of components that don't use them.

The Redux `dispatch()` method is passed down via the component tree instead. This enables further usage of dispatching thunks for other scenarios avoiding prop drilling.
The `showBoxModelHighlighterForNode`, `hideBoxModelHighlighter` and `showBoxModelHighlighter` props and methods are removed in favor of the corresponding thunks.

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

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

@@ -18,18 +18,52 @@
  */
 
 module.exports = {
+  /**
+   * Show the box model highlighter for the currently selected node front.
+   * The selected node is obtained from the Selection instance on the Inspector.
+   *
+   * @param {Object} options
+   *        Optional configuration options passed to the box model highlighter
+   */
+  highlightSelectedNode(options = {}) {
+    return async thunkOptions => {
+      const { inspector } = thunkOptions;
+      if (!inspector || inspector._destroyed) {
+        return;
+      }
+
+      const { nodeFront } = inspector.selection;
+      if (!nodeFront) {
+        return;
+      }
+
+      await inspector.highlighters.showHighlighterTypeForNode(
+        inspector.highlighters.TYPES.BOXMODEL,
+        nodeFront,
+        options
+      );
+    };
+  },
+
   /**
    * Show the box model highlighter for the given node front.
    *
    * @param {NodeFront} nodeFront
    *        Node that should be highlighted.
+   * @param {Object} options
+   *        Optional configuration options passed to the box model highlighter
    */
-  highlightNode(nodeFront) {
+  highlightNode(nodeFront, options = {}) {
     return async thunkOptions => {
       const { inspector } = thunkOptions;
+      if (!inspector || inspector._destroyed) {
+        return;
+      }
+
       await inspector.highlighters.showHighlighterTypeForNode(
         inspector.highlighters.TYPES.BOXMODEL,
-        nodeFront
+        nodeFront,
+        options
       );
     };
   },
@@ -40,6 +74,10 @@ module.exports = {
   unhighlightNode() {
     return async thunkOptions => {
       const { inspector } = thunkOptions;
+      if (!inspector || inspector._destroyed) {
+        return;
+      }
+
       await inspector.highlighters.hideHighlighterType(
         inspector.highlighters.TYPES.BOXMODEL
       );

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

@@ -47,13 +47,11 @@ function BoxModel(inspector, window) {
 
   this.updateBoxModel = this.updateBoxModel.bind(this);
 
-  this.onHideBoxModelHighlighter = this.onHideBoxModelHighlighter.bind(this);
   this.onHideGeometryEditor = this.onHideGeometryEditor.bind(this);
   this.onMarkupViewLeave = this.onMarkupViewLeave.bind(this);
   this.onMarkupViewNodeHover = this.onMarkupViewNodeHover.bind(this);
   this.onNewSelection = this.onNewSelection.bind(this);
   this.onShowBoxModelEditor = this.onShowBoxModelEditor.bind(this);
-  this.onShowBoxModelHighlighter = this.onShowBoxModelHighlighter.bind(this);
   this.onShowRulePreviewTooltip = this.onShowRulePreviewTooltip.bind(this);
   this.onSidebarSelect = this.onSidebarSelect.bind(this);
   this.onToggleGeometryEditor = this.onToggleGeometryEditor.bind(this);
@@ -106,9 +104,7 @@ BoxModel.prototype = {
    */
   getComponentProps() {
     return {
-      onHideBoxModelHighlighter: this.onHideBoxModelHighlighter,
       onShowBoxModelEditor: this.onShowBoxModelEditor,
-      onShowBoxModelHighlighter: this.onShowBoxModelHighlighter,
       onShowRulePreviewTooltip: this.onShowRulePreviewTooltip,
       onToggleGeometryEditor: this.onToggleGeometryEditor,
     };
@@ -229,20 +225,6 @@ BoxModel.prototype = {
     this._lastRequest = lastRequest;
   },
 
-  /**
-   * Hides the box-model highlighter on the currently selected element.
-   */
-  onHideBoxModelHighlighter() {
-    // As React components aren't destroyed when the panel closes,
-    // this function may still be called and throw because of destroyed fronts.
-    if (!this.inspector) {
-      return;
-    }
-    this.inspector.highlighters.hideHighlighterType(
-      this.inspector.highlighters.TYPES.BOXMODEL
-    );
-  },
-
   /**
    * Hides the geometry editor and updates the box moodel store with the new
    * geometry editor enabled state.
@@ -394,25 +376,6 @@ BoxModel.prototype = {
     );
   },
 
-  /**
-   * Shows the box-model highlighter on the currently selected element.
-   *
-   * @param  {Object} options
-   *         Options passed to the highlighter actor.
-   */
-  onShowBoxModelHighlighter(options = {}) {
-    if (!this.inspector) {
-      return;
-    }
-
-    const { nodeFront } = this.inspector.selection;
-    this.inspector.highlighters.showHighlighterTypeForNode(
-      this.inspector.highlighters.TYPES.BOXMODEL,
-      nodeFront,
-      options
-    );
-  },
-
   /**
    * Handler for the inspector sidebar select event. Starts tracking reflows if the
    * layout panel is visible. Otherwise, stop tracking reflows. Finally, refresh the box

devtools/client/inspector/boxmodel/components/BoxModel.js

@@ -27,10 +27,8 @@ class BoxModel extends PureComponent {
   static get propTypes() {
     return {
       boxModel: PropTypes.shape(Types.boxModel).isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
+      dispatch: PropTypes.func.isRequired,
       onShowBoxModelEditor: PropTypes.func.isRequired,
-      onShowBoxModelHighlighter: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       onShowRulePreviewTooltip: PropTypes.func.isRequired,
       onToggleGeometryEditor: PropTypes.func.isRequired,
       showBoxModelProperties: PropTypes.bool.isRequired,
@@ -54,10 +52,8 @@ class BoxModel extends PureComponent {
   render() {
     const {
       boxModel,
-      onHideBoxModelHighlighter,
+      dispatch,
       onShowBoxModelEditor,
-      onShowBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
       onShowRulePreviewTooltip,
       onToggleGeometryEditor,
       setSelectedNode,
@@ -76,13 +72,12 @@ class BoxModel extends PureComponent {
       BoxModelMain({
         boxModel,
         boxModelContainer: this.boxModelContainer,
+        dispatch,
+        onShowBoxModelEditor,
+        onShowRulePreviewTooltip,
         ref: boxModelMain => {
           this.boxModelMain = boxModelMain;
         },
-        onHideBoxModelHighlighter,
-        onShowBoxModelEditor,
-        onShowBoxModelHighlighter,
-        onShowRulePreviewTooltip,
       }),
       BoxModelInfo({
         boxModel,
@@ -91,9 +86,8 @@ class BoxModel extends PureComponent {
       showBoxModelProperties
         ? BoxModelProperties({
             boxModel,
+            dispatch,
             setSelectedNode,
-            onHideBoxModelHighlighter,
-            onShowBoxModelHighlighterForNode,
           })
         : null
     );

devtools/client/inspector/boxmodel/components/BoxModelMain.js

@@ -19,6 +19,11 @@ const BoxModelEditable = createFactory(
 
 const Types = require("devtools/client/inspector/boxmodel/types");
 
+const {
+  highlightSelectedNode,
+  unhighlightNode,
+} = require("devtools/client/inspector/boxmodel/actions/box-model-highlighter");
+
 const SHARED_STRINGS_URI = "devtools/client/locales/shared.properties";
 const SHARED_L10N = new LocalizationHelper(SHARED_STRINGS_URI);
 
@@ -27,9 +32,8 @@ class BoxModelMain extends PureComponent {
     return {
       boxModel: PropTypes.shape(Types.boxModel).isRequired,
       boxModelContainer: PropTypes.object,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
+      dispatch: PropTypes.func.isRequired,
       onShowBoxModelEditor: PropTypes.func.isRequired,
-      onShowBoxModelHighlighter: PropTypes.func.isRequired,
       onShowRulePreviewTooltip: PropTypes.func.isRequired,
     };
   }
@@ -285,14 +289,16 @@ class BoxModelMain extends PureComponent {
         }
       } while (el.parentNode);
 
-      this.props.onHideBoxModelHighlighter();
+      this.props.dispatch(unhighlightNode());
     }
 
-    this.props.onShowBoxModelHighlighter({
-      region,
-      showOnly: region,
-      onlyRegionArea: true,
-    });
+    this.props.dispatch(
+      highlightSelectedNode({
+        region,
+        showOnly: region,
+        onlyRegionArea: true,
+      })
+    );
 
     event.preventDefault();
   }
@@ -401,6 +407,7 @@ class BoxModelMain extends PureComponent {
   render() {
     const {
       boxModel,
+      dispatch,
       onShowBoxModelEditor,
       onShowRulePreviewTooltip,
     } = this.props;
@@ -480,7 +487,7 @@ class BoxModelMain extends PureComponent {
         onClick: this.onLevelClick,
         onKeyDown: this.onKeyDown,
         onMouseOver: this.onHighlightMouseOver,
-        onMouseOut: this.props.onHideBoxModelHighlighter,
+        onMouseOut: () => dispatch(unhighlightNode()),
       },
       displayPosition
         ? dom.span(

devtools/client/inspector/boxmodel/components/BoxModelProperties.js

@@ -25,8 +25,7 @@ class BoxModelProperties extends PureComponent {
   static get propTypes() {
     return {
       boxModel: PropTypes.shape(Types.boxModel).isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
-      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
+      dispatch: PropTypes.func.isRequired,
       setSelectedNode: PropTypes.func.isRequired,
     };
   }
@@ -79,12 +78,7 @@ class BoxModelProperties extends PureComponent {
   }
 
   render() {
-    const {
-      boxModel,
-      onHideBoxModelHighlighter,
-      onShowBoxModelHighlighterForNode,
-      setSelectedNode,
-    } = this.props;
+    const { boxModel, dispatch, setSelectedNode } = this.props;
     const { layout } = boxModel;
 
     const layoutInfo = [
@@ -103,10 +97,9 @@ class BoxModelProperties extends PureComponent {
       } = this.getReferenceElement(info);
 
       return ComputedProperty({
+        dispatch,
         key: info,
         name: info,
-        onHideBoxModelHighlighter,
-        onShowBoxModelHighlighterForNode,
         referenceElement,
         referenceElementType,
         setSelectedNode,

devtools/client/inspector/boxmodel/components/ComputedProperty.js

@@ -15,15 +15,19 @@ loader.lazyRequireGetter(
   "devtools/client/inspector/shared/node-reps"
 );
 
+const {
+  highlightNode,
+  unhighlightNode,
+} = require("devtools/client/inspector/boxmodel/actions/box-model-highlighter");
+
 const BOXMODEL_STRINGS_URI = "devtools/client/locales/boxmodel.properties";
 const BOXMODEL_L10N = new LocalizationHelper(BOXMODEL_STRINGS_URI);
 
 class ComputedProperty extends PureComponent {
   static get propTypes() {
     return {
+      dispatch: PropTypes.func.isRequired,
       name: PropTypes.string.isRequired,
-      onHideBoxModelHighlighter: PropTypes.func,
-      onShowBoxModelHighlighterForNode: PropTypes.func,
       referenceElement: PropTypes.object,
       referenceElementType: PropTypes.string,
       setSelectedNode: PropTypes.func,
@@ -41,8 +45,7 @@ class ComputedProperty extends PureComponent {
 
   renderReferenceElementPreview() {
     const {
-      onShowBoxModelHighlighterForNode,
-      onHideBoxModelHighlighter,
+      dispatch,
       referenceElement,
       referenceElementType,
       setSelectedNode,
@@ -65,9 +68,8 @@ class ComputedProperty extends PureComponent {
       getNodeRep(referenceElement, {
         onInspectIconClick: () =>
           setSelectedNode(referenceElement, { reason: "box-model" }),
-        onDOMNodeMouseOver: () =>
-          onShowBoxModelHighlighterForNode(referenceElement),
-        onDOMNodeMouseOut: () => onHideBoxModelHighlighter(),
+        onDOMNodeMouseOver: () => dispatch(highlightNode(referenceElement)),
+        onDOMNodeMouseOut: () => dispatch(unhighlightNode()),
       })
     );
   }

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

@@ -58,11 +58,9 @@ class LayoutApp extends PureComponent {
       grids: PropTypes.arrayOf(PropTypes.shape(GridTypes.grid)).isRequired,
       highlighterSettings: PropTypes.shape(GridTypes.highlighterSettings)
         .isRequired,
-      onHideBoxModelHighlighter: PropTypes.func.isRequired,
       onSetFlexboxOverlayColor: PropTypes.func.isRequired,
       onSetGridOverlayColor: PropTypes.func.isRequired,
       onShowBoxModelEditor: PropTypes.func.isRequired,
-      onShowBoxModelHighlighter: PropTypes.func.isRequired,
       onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
       onShowGridOutlineHighlight: PropTypes.func.isRequired,
       onToggleFlexboxHighlighter: PropTypes.func.isRequired,

devtools/client/inspector/layout/layout.js

@@ -47,9 +47,7 @@ class LayoutView {
     } = this.inspector.getCommonComponentProps();
 
     const {
-      onHideBoxModelHighlighter,
       onShowBoxModelEditor,
-      onShowBoxModelHighlighter,
       onShowRulePreviewTooltip,
       onToggleGeometryEditor,
     } = this.inspector.getPanel("boxmodel").getComponentProps();
@@ -78,11 +76,9 @@ class LayoutView {
 
     const layoutApp = LayoutApp({
       getSwatchColorPickerTooltip: () => this.swatchColorPickerTooltip,
-      onHideBoxModelHighlighter,
       onSetFlexboxOverlayColor,
       onSetGridOverlayColor,
       onShowBoxModelEditor,
-      onShowBoxModelHighlighter,
       onShowBoxModelHighlighterForNode,
       onShowRulePreviewTooltip,
       onShowGridOutlineHighlight,