No longer autoscroll when selecting via tree

Summary:
Added selection source concept to onSelect callback. This allows us to only autoscroll the tree when selection source is the visualiser. We had feedback that the horizontal autoscrolling whilst using the tree was unhelpful.

A side benefit of selection source is better tracking of how people use kb, tree vs visualiser to select things

Changelog: UIDebugger only autoscroll horizontally when selecting via the visualiser

Reviewed By: lblasa

Differential Revision: D47334078

fbshipit-source-id: d7eadddb8d3d0fd428d5c294b2dccc2f1efa5a95

desktop/plugins/public/ui-debugger/components/Tree.tsx

@@ -7,7 +7,13 @@
  * @format
  */
 
-import {FrameworkEvent, FrameworkEventType, Id, UINode} from '../types';
+import {
+  FrameworkEvent,
+  FrameworkEventType,
+  Id,
+  OnSelectNode,
+  UINode,
+} from '../types';
 import React, {
   ReactNode,
   Ref,
@@ -18,7 +24,6 @@ import React, {
   useRef,
 } from 'react';
 import {
-  Atom,
   getFlipperLib,
   HighlightManager,
   HighlightProvider,
@@ -86,7 +91,7 @@ export function Tree2({nodes, rootId}: {nodes: Map<Id, UINode>; rootId: Id}) {
       nodes,
       focusedNode || rootId,
       expandedNodes,
-      selectedNode,
+      selectedNode?.id,
     );
 
     const refs: React.RefObject<HTMLLIElement>[] = treeNodes.map(() =>
@@ -119,7 +124,7 @@ export function Tree2({nodes, rootId}: {nodes: Map<Id, UINode>; rootId: Id}) {
   useKeyboardShortcuts(
     treeNodes,
     rowVirtualizer,
-    selectedNode,
+    selectedNode?.id,
     hoveredNode,
     instance.uiActions.onSelectNode,
     instance.uiActions.onHoverNode,
@@ -162,17 +167,27 @@ export function Tree2({nodes, rootId}: {nodes: Map<Id, UINode>; rootId: Id}) {
   });
 
   useLayoutEffect(() => {
-    if (selectedNode) {
+    if (selectedNode != null) {
-      const idx = treeNodes.findIndex((node) => node.id === selectedNode);
+      const selectedTreeNode = treeNodes.find(
+        (node) => node.id === selectedNode?.id,
+      );
 
-      const kbIsNoLongerReservingScroll =
+      const ref = parentRef.current;
-        new Date().getTime() > (isUsingKBToScrollUtill.current ?? 0);
+      if (
+        ref != null &&
+        selectedTreeNode != null &&
+        selectedNode?.source === 'visualiser'
+      ) {
+        ref.scrollLeft =
+          Math.max(0, selectedTreeNode.depth - 10) * renderDepthOffset;
 
-      if (idx !== -1 && kbIsNoLongerReservingScroll) {
+        let scrollToIndex = selectedTreeNode.idx;
-        parentRef.current!!.scrollLeft =
-          Math.max(0, treeNodes[idx].depth - 10) * renderDepthOffset;
 
-        rowVirtualizer.scrollToIndex(idx, {align: 'auto'});
+        if (selectedTreeNode.idx > rowVirtualizer.range.endIndex) {
+          //when scrolling down the scrollbar gets in the way if you scroll to the precise node
+          scrollToIndex = Math.min(scrollToIndex + 1, treeNodes.length);
+        }
+        rowVirtualizer.scrollToIndex(scrollToIndex, {align: 'auto'});
       }
     }
     // NOTE: We don't want to add refs or tree nodes to the dependency list since when new data comes in over the wire
@@ -226,7 +241,7 @@ export function Tree2({nodes, rootId}: {nodes: Map<Id, UINode>; rootId: Id}) {
                   frameworkEvents={frameworkEvents}
                   frameworkEventsMonitoring={frameworkEventsMonitoring}
                   highlightedNodes={highlightedNodes}
-                  selectedNode={selectedNode}
+                  selectedNode={selectedNode?.id}
                   hoveredNode={hoveredNode}
                   isUsingKBToScroll={isUsingKBToScrollUtill}
                   isContextMenuOpen={isContextMenuOpen}
@@ -296,7 +311,7 @@ function TreeItemContainer({
   hoveredNode?: Id;
   isUsingKBToScroll: RefObject<MillisSinceEpoch>;
   isContextMenuOpen: boolean;
-  onSelectNode: (node?: Id) => void;
+  onSelectNode: OnSelectNode;
   onExpandNode: (node: Id) => void;
   onCollapseNode: (node: Id) => void;
   onHoverNode: (node: Id) => void;
@@ -333,7 +348,7 @@ function TreeItemContainer({
           }
         }}
         onClick={() => {
-          onSelectNode(treeNode.id);
+          onSelectNode(treeNode.id, 'tree');
         }}
         item={treeNode}
         style={{overflow: 'visible'}}>
@@ -688,7 +703,7 @@ function useKeyboardShortcuts(
   rowVirtualizer: Virtualizer<HTMLDivElement, Element>,
   selectedNode: Id | undefined,
   hoveredNodeId: Id | undefined,
-  onSelectNode: (id?: Id) => void,
+  onSelectNode: OnSelectNode,
   onHoverNode: (id?: Id) => void,
   onExpandNode: (id: Id) => void,
   onCollapseNode: (id: Id) => void,
@@ -702,7 +717,7 @@ function useKeyboardShortcuts(
       switch (event.key) {
         case 'Enter': {
           if (hoveredNodeId != null) {
-            onSelectNode(hoveredNodeId);
+            onSelectNode(hoveredNodeId, 'keyboard');
           }
 
           break;
@@ -793,7 +808,7 @@ function moveSelectedNodeUpOrDown(
   rowVirtualizer: Virtualizer<HTMLDivElement, Element>,
   hoveredNode: Id | undefined,
   selectedNode: Id | undefined,
-  onSelectNode: (id?: Id) => void,
+  onSelectNode: OnSelectNode,
   onHoverNode: (id?: Id) => void,
   isUsingKBToScrollUntill: React.MutableRefObject<MillisSinceEpoch>,
 ) {
@@ -818,7 +833,7 @@ function moveSelectedNodeViaKeyBoard(
   newIdx: number,
   treeNodes: TreeNode[],
   rowVirtualizer: Virtualizer<HTMLDivElement, Element>,
-  onSelectNode: (id?: Id) => void,
+  onSelectNode: OnSelectNode,
   onHoverNode: (id?: Id) => void,
   isUsingKBToScrollUntil: React.MutableRefObject<number>,
 ) {
@@ -826,7 +841,7 @@ function moveSelectedNodeViaKeyBoard(
     const newNode = treeNodes[newIdx];
 
     extendKBControlLease(isUsingKBToScrollUntil);
-    onSelectNode(newNode.id);
+    onSelectNode(newNode.id, 'keyboard');
     onHoverNode(newNode.id);
 
     rowVirtualizer.scrollToIndex(newIdx, {align: 'auto'});

desktop/plugins/public/ui-debugger/components/Visualization2D.tsx

@@ -8,7 +8,14 @@
  */
 
 import React, {useEffect, useMemo, useRef} from 'react';
-import {Bounds, Coordinate, Id, NestedNode, UINode} from '../types';
+import {
+  Bounds,
+  Coordinate,
+  Id,
+  NestedNode,
+  OnSelectNode,
+  UINode,
+} from '../types';
 
 import {produce, styled, theme, usePlugin, useValue} from 'flipper-plugin';
 import {plugin} from '../index';
@@ -21,7 +28,7 @@ export const Visualization2D: React.FC<
   {
     width: number;
     nodes: Map<Id, UINode>;
-    onSelectNode: (id?: Id) => void;
+    onSelectNode: OnSelectNode;
   } & React.HTMLAttributes<HTMLDivElement>
 > = ({width, nodes, onSelectNode}) => {
   const rootNodeRef = useRef<HTMLDivElement>();
@@ -129,7 +136,7 @@ export const Visualization2D: React.FC<
         {selectedNodeId && (
           <OverlayBorder
             type="selected"
-            nodeId={selectedNodeId}
+            nodeId={selectedNodeId.id}
             nodes={nodes}
           />
         )}
@@ -185,7 +192,7 @@ function Visualization2DNode({
   onSelectNode,
 }: {
   node: NestedNode;
-  onSelectNode: (id?: Id) => void;
+  onSelectNode: OnSelectNode;
 }) {
   const instance = usePlugin(plugin);
 
@@ -236,7 +243,7 @@ function Visualization2DNode({
 
         const hoveredNodes = instance.uiState.hoveredNodes.get();
 
-        onSelectNode(hoveredNodes[0]);
+        onSelectNode(hoveredNodes[0], 'visualiser');
       }}>
       <NodeBorder />
 

desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx

@@ -41,7 +41,7 @@ export const Inspector: React.FC<Props> = ({
   showExtra,
 }) => {
   const instance = usePlugin(plugin);
-  const selectedNodeId = useValue(instance.uiState.selectedNode);
+  const selectedNodeId = useValue(instance.uiState.selectedNode)?.id;
   const frameworkEvents = useValue(instance.frameworkEvents);
 
   const selectedNode = selectedNodeId ? nodes.get(selectedNodeId) : undefined;

desktop/plugins/public/ui-debugger/hooks/useDelay.tsx

@@ -7,6 +7,7 @@
  * @format
  */
 
+import {Atom} from 'flipper-plugin';
 import {useEffect, useRef, useState} from 'react';
 
 export function useDelay(delayTimeMs: number) {

desktop/plugins/public/ui-debugger/index.tsx

@@ -22,10 +22,13 @@ import {
   Id,
   Metadata,
   MetadataId,
+  NodeSelection,
   PerformanceStatsEvent,
+  SelectionSource,
   SnapshotInfo,
   StreamInterceptorError,
   StreamState,
+  UIActions,
   UINode,
   UIState,
 } from './types';
@@ -204,7 +207,7 @@ export function plugin(client: PluginClient<Events>) {
 
     highlightedNodes,
 
-    selectedNode: createState<Id | undefined>(undefined),
+    selectedNode: createState<NodeSelection | undefined>(undefined),
     //used to indicate whether we will higher the visualizer / tree when a matching event comes in
     //also whether or not will show running total  in the tree
     frameworkEventMonitoring: createState(
@@ -371,34 +374,28 @@ export function plugin(client: PluginClient<Events>) {
   };
 }
 
-type UIActions = {
-  onHoverNode: (node?: Id) => void;
-  onFocusNode: (focused?: Id) => void;
-  onContextMenuOpen: (open: boolean) => void;
-  onSelectNode: (node?: Id) => void;
-  onExpandNode: (node: Id) => void;
-  onCollapseNode: (node: Id) => void;
-  setVisualiserWidth: (width: number) => void;
-};
-
 function uiActions(uiState: UIState, nodes: Atom<Map<Id, UINode>>): UIActions {
   const onExpandNode = (node: Id) => {
     uiState.expandedNodes.update((draft) => {
       draft.add(node);
     });
   };
-  const onSelectNode = (node?: Id) => {
+  const onSelectNode = (node: Id | undefined, source: SelectionSource) => {
-    if (uiState.selectedNode.get() === node) {
+    if (node == null || uiState.selectedNode.get()?.id === node) {
       uiState.selectedNode.set(undefined);
     } else {
-      uiState.selectedNode.set(node);
+      uiState.selectedNode.set({id: node, source});
     }
 
     if (node) {
       const selectedNode = nodes.get().get(node);
       const tags = selectedNode?.tags;
       if (tags) {
-        tracker.track('node-selected', {name: selectedNode.name, tags});
+        tracker.track('node-selected', {
+          name: selectedNode.name,
+          tags,
+          source: source,
+        });
       }
 
       let current = selectedNode?.parent;

desktop/plugins/public/ui-debugger/tracker.tsx

@@ -8,6 +8,7 @@
  */
 
 import {getFlipperLib} from 'flipper-plugin';
+import {SelectionSource} from '.';
 import {FrameworkEventType, Tag} from './types';
 
 const UI_DEBUGGER_IDENTIFIER = 'ui-debugger';
@@ -15,6 +16,7 @@ const UI_DEBUGGER_IDENTIFIER = 'ui-debugger';
 type NodeEventPayload = {
   name: string;
   tags: Tag[];
+  source?: SelectionSource;
 };
 
 type TrackerEvents = {

desktop/plugins/public/ui-debugger/types.tsx

@@ -16,7 +16,7 @@ export type UIState = {
   searchTerm: Atom<string>;
   isContextMenuOpen: Atom<boolean>;
   hoveredNodes: Atom<Id[]>;
-  selectedNode: Atom<Id | undefined>;
+  selectedNode: Atom<NodeSelection | undefined>;
   highlightedNodes: Atom<Set<Id>>;
   focusedNode: Atom<Id | undefined>;
   expandedNodes: Atom<Set<Id>>;
@@ -24,6 +24,27 @@ export type UIState = {
   frameworkEventMonitoring: Atom<Map<FrameworkEventType, boolean>>;
 };
 
+export type NodeSelection = {
+  id: Id;
+  source: SelectionSource;
+};
+
+export type OnSelectNode = (
+  node: Id | undefined,
+  source: SelectionSource,
+) => void;
+
+export type UIActions = {
+  onHoverNode: (node?: Id) => void;
+  onFocusNode: (focused?: Id) => void;
+  onContextMenuOpen: (open: boolean) => void;
+  onSelectNode: OnSelectNode;
+  onExpandNode: (node: Id) => void;
+  onCollapseNode: (node: Id) => void;
+  setVisualiserWidth: (width: number) => void;
+};
+export type SelectionSource = 'visualiser' | 'tree' | 'keyboard';
+
 export type StreamState =
   | {state: 'Ok'}
   | {state: 'RetryingAfterError'}