From 8f9ac0d08726706acaf1a71ab8c8b14a0953a1d3 Mon Sep 17 00:00:00 2001
From: Luke De Feo <lukedefeo@fb.com>
Date: Wed, 28 Sep 2022 06:51:13 -0700
Subject: [PATCH] New implementation of nested hover

Summary:
There were 2 issues with the previous implementation of the nested hover.
1. If you moved the mouse out of the inspector quickly we would miss the event and we would have a hover state of the root element when we shouldnt
2. The hover state was stored per node, it was possible to have mulitple children hovered at the same time if you moved the mouse fast enough in a very complex tree

The new implementation has the hovered id stored in the Datainspector root. This solves the multiple state issue since there can only be one.  Finally There is an onMouseLeave hook added to the parent div which seems to reliably fire no mouse how erractic my mouse movements :) Also the new implementation is a lot easier to understand

Reviewed By: mweststrate

Differential Revision: D39855733

fbshipit-source-id: 96b43f216deef72b81cd52001f8de26df55ea693
---
 .../src/ui/data-inspector/DataInspector.tsx   | 16 ++++-
 .../ui/data-inspector/DataInspectorNode.tsx   | 59 ++++++-------------
 2 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/desktop/flipper-plugin/src/ui/data-inspector/DataInspector.tsx b/desktop/flipper-plugin/src/ui/data-inspector/DataInspector.tsx
index 39f4c7721..872bfaa93 100644
--- a/desktop/flipper-plugin/src/ui/data-inspector/DataInspector.tsx
+++ b/desktop/flipper-plugin/src/ui/data-inspector/DataInspector.tsx
@@ -82,6 +82,7 @@ type DataInspectorState = {
   filterExpanded: DataInspectorExpanded;
   userExpanded: DataInspectorExpanded;
   filter: string;
+  hoveredNodePath: string | undefined;
 };
 
 const MAX_RESULTS = 50;
@@ -102,6 +103,7 @@ export class DataInspector extends PureComponent<
     userExpanded: {},
     filterExpanded: {},
     filter: '',
+    hoveredNodePath: undefined,
   };
 
   static getDerivedStateFromProps(
@@ -181,6 +183,16 @@ export class DataInspector extends PureComponent<
     });
   };
 
+  setHoveredNodePath = (path?: string) => {
+    this.setState({
+      hoveredNodePath: path,
+    });
+  };
+
+  removeHover = () => {
+    this.setHoveredNodePath(undefined);
+  };
+
   // make sure this fn is a stable ref to not invalidate the whole tree on new data
   getRootData = () => {
     return this.props.data;
@@ -188,10 +200,12 @@ export class DataInspector extends PureComponent<
 
   render() {
     return (
-      <Layout.Container>
+      <Layout.Container onMouseLeave={this.removeHover}>
         <RootDataContext.Provider value={this.getRootData}>
           <HighlightProvider text={this.props.filter}>
             <DataInspectorNode
+              hoveredNodePath={this.state.hoveredNodePath}
+              setHoveredNodePath={this.setHoveredNodePath}
               data={this.props.data}
               diff={this.props.diff}
               extractValue={this.props.extractValue}
diff --git a/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx b/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx
index dfcec4a21..562b25028 100644
--- a/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx
+++ b/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx
@@ -181,9 +181,9 @@ type DataInspectorProps = {
     name?: string,
   ) => ReactElement[];
 
-  onMouseEnter?: () => void;
+  hoveredNodePath?: string;
 
-  onMouseExit?: () => void;
+  setHoveredNodePath: (path: string) => void;
 };
 
 const defaultValueExtractor: DataValueExtractor = (value: any) => {
@@ -315,14 +315,13 @@ export const DataInspectorNode: React.FC<DataInspectorProps> = memo(
     tooltips,
     setValue: setValueProp,
     additionalContextMenuItems,
-    onMouseEnter,
-    onMouseExit,
+    hoveredNodePath,
+    setHoveredNodePath,
   }) {
     const highlighter = useHighlighter();
     const getRoot = useContext(RootDataContext);
     const isUnitTest = useInUnitTest();
 
-    const [hover, setHover] = useState(false);
     const shouldExpand = useRef(false);
     const expandHandle = useRef(undefined as any);
     const [renderExpanded, setRenderExpanded] = useState(false);
@@ -428,37 +427,6 @@ export const DataInspectorNode: React.FC<DataInspectorProps> = memo(
       [onDelete],
     );
 
-    const thisOnMouseEnter = useCallback(
-      (e: SyntheticEvent) => {
-        onMouseEnter?.();
-        e.stopPropagation();
-        setHover(true);
-      },
-      [onMouseEnter],
-    );
-
-    const thisOnMouseLeave = useCallback(
-      (e: SyntheticEvent) => {
-        onMouseExit?.();
-        e.stopPropagation();
-        setHover(false);
-      },
-      [onMouseExit],
-    );
-
-    const childOnMouseEnter = useCallback(() => {
-      //when a child is hovered we are in both child and parents bounds so
-      //manually disable our own hover state so we focus on child
-      setHover(false);
-    }, []);
-
-    const childOnMouseExit = useCallback(() => {
-      //If a child has been unhovered then it means we have left the childs bounds and mouse should still be in parents
-      //(current element's) bounds. However the mouse enter callback wont fire again in this element since we never left the bounds.
-      //Therefore we manually set hovered back to true
-      setHover(true);
-    }, []);
-
     /**
      * RENDERING
      */
@@ -502,8 +470,8 @@ export const DataInspectorNode: React.FC<DataInspectorProps> = memo(
           const metaKey = key + index;
           const dataInspectorNode = (
             <DataInspectorNode
-              onMouseEnter={childOnMouseEnter}
-              onMouseExit={childOnMouseExit}
+              setHoveredNodePath={setHoveredNodePath}
+              hoveredNodePath={hoveredNodePath}
               parentAncestry={ancestry}
               extractValue={extractValue}
               setValue={setValue}
@@ -662,13 +630,19 @@ export const DataInspectorNode: React.FC<DataInspectorProps> = memo(
       );
     }
 
+    const nodePath = path.join('.');
+
     return (
       <Dropdown overlay={getContextMenu} trigger={contextMenuTrigger}>
         <BaseContainer
           onContextMenu={stopPropagation}
-          hovered={hover}
-          onMouseEnter={thisOnMouseEnter}
-          onMouseLeave={thisOnMouseLeave}
+          hovered={hoveredNodePath === nodePath}
+          onMouseEnter={() => {
+            setHoveredNodePath(nodePath);
+          }}
+          onMouseLeave={() => {
+            setHoveredNodePath(parentPath.join('.'));
+          }}
           depth={depth}
           disabled={!!setValueProp && !!setValue === false}>
           <PropertyContainer onClick={isExpandable ? handleClick : undefined}>
@@ -755,7 +729,8 @@ function dataInspectorPropsAreEqual(
     nextProps.onDelete === props.onDelete &&
     nextProps.setValue === props.setValue &&
     nextProps.collapsed === props.collapsed &&
-    nextProps.expandRoot === props.expandRoot
+    nextProps.expandRoot === props.expandRoot &&
+    nextProps.hoveredNodePath === props.hoveredNodePath
   );
 }