Merge pull request #126 from bvaughn/js-hover

Ignore hover when navigating with keyboard

src/devtools/views/Components/Element.css

@@ -1,6 +1,7 @@
 .Element,
 .InactiveSelectedElement,
-.SelectedElement {
+.SelectedElement,
+.HoveredElement {
   border-radius: 0.25em;
   white-space: nowrap;
   line-height: var(--line-height-data);
@@ -9,7 +10,7 @@
   cursor: default;
   user-select: none;
 }
-.Element:hover {
+.HoveredElement {
   background-color: var(--color-hover-background);
 }
 .InactiveSelectedElement {

src/devtools/views/Components/Element.js

@@ -7,13 +7,14 @@ import React, {
   useLayoutEffect,
   useMemo,
   useRef,
+  useState,
 } from 'react';
 import { ElementTypeClass, ElementTypeFunction } from 'src/devtools/types';
 import Store from 'src/devtools/store';
 import ButtonIcon from '../ButtonIcon';
 import { createRegExp } from '../utils';
 import { TreeContext } from './TreeContext';
-import { BridgeContext, StoreContext } from '../context';
+import { StoreContext } from '../context';
 
 import type { ItemData } from './Tree';
 import type { Element } from './types';
@@ -27,6 +28,7 @@ type Props = {
 };
 
 export default function ElementView({ data, index, style }: Props) {
+  const [isHovered, setIsHovered] = useState(false);
   const {
     baseDepth,
     getElementAtIndex,
@@ -35,15 +37,18 @@ export default function ElementView({ data, index, style }: Props) {
     selectedElementID,
     selectElementByID,
   } = useContext(TreeContext);
-  const bridge = useContext(BridgeContext);
   const store = useContext(StoreContext);
 
   const element = getElementAtIndex(index);
 
+  const {
+    lastScrolledIDRef,
+    treeFocused,
+    isNavigatingWithKeyboard,
+    onElementMouseEnter,
+  } = data;
   const id = element === null ? null : element.id;
   const isSelected = selectedElementID === id;
-  const lastScrolledIDRef = data.lastScrolledIDRef;
-  const treeFocused = data.treeFocused;
 
   const handleDoubleClick = useCallback(() => {
     if (id !== null) {
@@ -88,20 +93,16 @@ export default function ElementView({ data, index, style }: Props) {
     [id, selectElementByID]
   );
 
-  const rendererID = id !== null ? store.getRendererIDForElement(id) : null;
-  // Individual elements don't have a corresponding leave handler.
-  // Instead, it's implemented on the tree level.
   const handleMouseEnter = useCallback(() => {
-    if (element !== null && id !== null && rendererID !== null) {
-      bridge.send('highlightElementInDOM', {
-        displayName: element.displayName,
-        hideAfterTimeout: false,
-        id,
-        rendererID,
-        scrollIntoView: false,
-      });
+    setIsHovered(true);
+    if (id !== null) {
+      onElementMouseEnter(id);
     }
-  }, [bridge, element, id, rendererID]);
+  }, [onElementMouseEnter, id]);
+
+  const handleMouseLeave = useCallback(() => {
+    setIsHovered(false);
+  }, []);
 
   // Handle elements that are removed from the tree while an async render is in progress.
   if (element == null) {
@@ -121,12 +122,15 @@ export default function ElementView({ data, index, style }: Props) {
     className = treeFocused
       ? styles.SelectedElement
       : styles.InactiveSelectedElement;
+  } else if (isHovered && !isNavigatingWithKeyboard) {
+    className = styles.HoveredElement;
   }
 
   return (
     <div
       className={className}
       onMouseEnter={handleMouseEnter}
+      onMouseLeave={handleMouseLeave}
       onMouseDown={handleMouseDown}
       onDoubleClick={handleDoubleClick}
       style={{

src/devtools/views/Components/Tree.js

@@ -27,7 +27,9 @@ export type ItemData = {|
   baseDepth: number,
   numElements: number,
   getElementAtIndex: (index: number) => Element | null,
+  isNavigatingWithKeyboard: boolean,
   lastScrolledIDRef: { current: number | null },
+  onElementMouseEnter: (id: number) => void,
   treeFocused: boolean,
 |};
 
@@ -50,6 +52,9 @@ export default function Tree(props: Props) {
   } = useContext(TreeContext);
   const bridge = useContext(BridgeContext);
   const store = useContext(StoreContext);
+  const [isNavigatingWithKeyboard, setIsNavigatingWithKeyboard] = useState(
+    false
+  );
   // $FlowFixMe https://github.com/facebook/flow/issues/7341
   const listRef = useRef<FixedSizeList<ItemData> | null>(null);
   const treeRef = useRef<HTMLDivElement | null>(null);
@@ -101,8 +106,6 @@ export default function Tree(props: Props) {
       }
 
       let element;
-
-      // eslint-disable-next-line default-case
       switch (event.key) {
         case 'ArrowDown':
           event.preventDefault();
@@ -140,7 +143,10 @@ export default function Tree(props: Props) {
           event.preventDefault();
           selectPreviousElementInTree();
           break;
+        default:
+          return;
       }
+      setIsNavigatingWithKeyboard(true);
     };
 
     // It's important to listen to the ownerDocument to support the browser extension.
@@ -161,8 +167,8 @@ export default function Tree(props: Props) {
     store,
   ]);
 
+  // Focus management.
   const handleBlur = useCallback(() => setTreeFocused(false));
-
   const handleFocus = useCallback(() => {
     setTreeFocused(true);
 
@@ -187,6 +193,53 @@ export default function Tree(props: Props) {
     [selectedElementID, selectOwner]
   );
 
+  const highlightElementInDOM = useCallback(
+    (id: number) => {
+      const element = store.getElementByID(id);
+      const rendererID = store.getRendererIDForElement(id);
+      if (element !== null) {
+        bridge.send('highlightElementInDOM', {
+          displayName: element.displayName,
+          hideAfterTimeout: false,
+          id,
+          rendererID,
+          scrollIntoView: false,
+        });
+      }
+    },
+    [store, bridge]
+  );
+
+  // If we switch the selected element while using the keyboard,
+  // start highlighting it in the DOM instead of the last hovered node.
+  useEffect(() => {
+    if (isNavigatingWithKeyboard && selectedElementID !== null) {
+      highlightElementInDOM(selectedElementID);
+    }
+  }, [isNavigatingWithKeyboard, highlightElementInDOM, selectedElementID]);
+
+  // Highlight last hovered element.
+  const handleElementMouseEnter = useCallback(
+    id => {
+      // Ignore hover while we're navigating with keyboard.
+      // This avoids flicker from the hovered nodes under the mouse.
+      if (!isNavigatingWithKeyboard) {
+        highlightElementInDOM(id);
+      }
+    },
+    [isNavigatingWithKeyboard, highlightElementInDOM]
+  );
+
+  const handleMouseMove = useCallback(() => {
+    // We started using the mouse again.
+    // This will enable hover styles in individual rows.
+    setIsNavigatingWithKeyboard(false);
+  }, []);
+
+  const handleMouseLeave = useCallback(() => {
+    bridge.send('clearHighlightedElementInDOM');
+  }, [bridge]);
+
   // Let react-window know to re-render any time the underlying tree data changes.
   // This includes the owner context, since it controls a filtered view of the tree.
   const itemData = useMemo<ItemData>(
@@ -194,16 +247,22 @@ export default function Tree(props: Props) {
       baseDepth,
       numElements,
       getElementAtIndex,
+      isNavigatingWithKeyboard,
+      onElementMouseEnter: handleElementMouseEnter,
       lastScrolledIDRef,
       treeFocused,
     }),
-    [baseDepth, numElements, getElementAtIndex, lastScrolledIDRef, treeFocused]
+    [
+      baseDepth,
+      numElements,
+      getElementAtIndex,
+      isNavigatingWithKeyboard,
+      handleElementMouseEnter,
+      lastScrolledIDRef,
+      treeFocused,
+    ]
   );
 
-  const handleMouseLeave = useCallback(() => {
-    bridge.send('clearHighlightedElementInDOM');
-  }, [bridge]);
-
   return (
     <div className={styles.Tree} ref={treeRef}>
       <div className={styles.SearchInput}>
@@ -215,6 +274,7 @@ export default function Tree(props: Props) {
         onBlur={handleBlur}
         onFocus={handleFocus}
         onKeyPress={handleKeyPress}
+        onMouseMove={handleMouseMove}
         onMouseLeave={handleMouseLeave}
         ref={focusTargetRef}
         tabIndex={0}