Repository: facebook/react
File: packages/react-devtools-shared/src/devtools/views/Components/Tree.js (JavaScript)

Code before:
    searchResults,
    inspectedElementID,
    inspectedElementIndex,
  } = useContext(TreeStateContext);
  const bridge = useContext(BridgeContext);
  const store = useContext(StoreContext);
  const {hideSettings} = useContext(OptionsContext);
  const {lineHeight} = useContext(SettingsContext);

  const [isNavigatingWithKeyboard, setIsNavigatingWithKeyboard] =
    useState(false);
  const {highlightHostInstance, clearHighlightHostInstance} =
    useHighlightHostInstance();
  const [treeFocused, setTreeFocused] = useState<boolean>(false);
  const componentsPanelVisible = useExtensionComponentsPanelVisibility(bridge);

  const treeRef = useRef<HTMLDivElement | null>(null);
  const focusTargetRef = useRef<HTMLDivElement | null>(null);
  const listRef = useRef(null);

  useEffect(() => {
    if (!componentsPanelVisible) {
      return;
    }

    if (listRef.current != null && inspectedElementIndex !== null) {
      listRef.current.scrollToItem(inspectedElementIndex, 'smart');
    }
  }, [inspectedElementIndex, componentsPanelVisible]);

  // Picking an element in the inspector should put focus into the tree.
  // This ensures that keyboard navigation works right after picking a node.
  useEffect(() => {
    function handleStopInspectingHost(didSelectNode: boolean) {
      if (didSelectNode && focusTargetRef.current !== null) {
        focusTargetRef.current.focus();
        logEvent({
          event_name: 'select-element',
          metadata: {source: 'inspector'},
        });
      }
    }
    bridge.addListener('stopInspectingHost', handleStopInspectingHost);
    return () =>
      bridge.removeListener('stopInspectingHost', handleStopInspectingHost);
  }, [bridge]);

  // This ref is passed down the context to elements.
  // It lets them avoid autoscrolling to the same item many times
  // when a selected virtual row goes in and out of the viewport.
  const lastScrolledIDRef = useRef<number | null>(null);

Review comment (bug):
I see the need to respond to `componentsPanelVisible` changing, but I don't see how the concerns of the previous impl is addressed, i.e. this impl would miss a call to `scrollToItem` if `listRef` failed a null check earlier.

Conceptually, I think we want `scrollToItem` to run in response to three changes:
1. `listRef` itself getting assigned an element ref
2. `inspectedElementIndex` changing
3. `componentsPanelVisible` changing

The previous impl triggers on 1) only. The cached function is updated in response to 2), but it's not actually called.
The current impl adds triggers on both 2) and 3). Is responding to 2) part of this PR?

If we want all three, we'd need both callback ref and effect, e.g.

```
const listRef = useRef(null);

const maybeScrollToItem = useCallback(() => {
    if (componentsPanelVisible && inspectedElementIndex !== null) {
      listRef.current?.scrollToItem(inspectedElementIndex, 'smart');
    }
}, [inspectedElementIndex, componentsPanelVisible]);

// triggered by 2) and 3) when `useCallback` busts the cache
useEffect(() => maybeScrollToItem(), [maybeScrollToItem]);

// triggered by 1)
const listCallbackRef = useCallback((list: HTMLElement) => {
  listRef.current = list;
  maybeScrollToItem();
}, [maybeScrollToItem]);
```

Code after:
    inspectedElementIndex,
  } = useContext(TreeStateContext);
  const bridge = useContext(BridgeContext);
  const store = useContext(StoreContext);
  const {hideSettings} = useContext(OptionsContext);
  const {lineHeight} = useContext(SettingsContext);

  const [isNavigatingWithKeyboard, setIsNavigatingWithKeyboard] =
    useState(false);
  const {highlightHostInstance, clearHighlightHostInstance} =
    useHighlightHostInstance();
  const [treeFocused, setTreeFocused] = useState<boolean>(false);
  const componentsPanelVisible = useExtensionComponentsPanelVisibility(bridge);

  const treeRef = useRef<HTMLDivElement | null>(null);
  const focusTargetRef = useRef<HTMLDivElement | null>(null);
  const listRef = useRef(null);

  useEffect(() => {
    if (!componentsPanelVisible) {
      return;
    }

    if (listRef.current != null && inspectedElementIndex !== null) {
      listRef.current.scrollToItem(inspectedElementIndex, 'smart');
    }
  }, [inspectedElementIndex, componentsPanelVisible]);

  // Picking an element in the inspector should put focus into the tree.
  // If possible, navigation works right after picking a node.
  // NOTE: This is not guaranteed to work, because browser extension panels are hosted inside an iframe.
  useEffect(() => {
    function handleStopInspectingHost(didSelectNode: boolean) {
      if (didSelectNode && focusTargetRef.current !== null) {
        focusTargetRef.current.focus();
        logEvent({
          event_name: 'select-element',
          metadata: {source: 'inspector'},
        });
      }
    }
    bridge.addListener('stopInspectingHost', handleStopInspectingHost);
    return () =>
      bridge.removeListener('stopInspectingHost', handleStopInspectingHost);
  }, [bridge]);

  // Navigate the tree with up/down arrow keys.
  useEffect(() => {
    if (treeRef.current === null) {
      return () => {};
    }