From a6bc8933cc594f50b2e3800c4b8ad81951033a48 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Wed, 19 Jul 2023 08:58:20 -0700 Subject: [PATCH] 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 --- .../public/ui-debugger/components/Tree.tsx | 55 ++++++++++++------- .../components/Visualization2D.tsx | 17 ++++-- .../components/sidebar/Inspector.tsx | 2 +- .../public/ui-debugger/hooks/useDelay.tsx | 1 + desktop/plugins/public/ui-debugger/index.tsx | 27 ++++----- .../plugins/public/ui-debugger/tracker.tsx | 2 + desktop/plugins/public/ui-debugger/types.tsx | 23 +++++++- 7 files changed, 85 insertions(+), 42 deletions(-) diff --git a/desktop/plugins/public/ui-debugger/components/Tree.tsx b/desktop/plugins/public/ui-debugger/components/Tree.tsx index d43a2300c..dcb653f1b 100644 --- a/desktop/plugins/public/ui-debugger/components/Tree.tsx +++ b/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; rootId: Id}) { nodes, focusedNode || rootId, expandedNodes, - selectedNode, + selectedNode?.id, ); const refs: React.RefObject[] = treeNodes.map(() => @@ -119,7 +124,7 @@ export function Tree2({nodes, rootId}: {nodes: Map; 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; rootId: Id}) { }); useLayoutEffect(() => { - if (selectedNode) { - const idx = treeNodes.findIndex((node) => node.id === selectedNode); + if (selectedNode != null) { + const selectedTreeNode = treeNodes.find( + (node) => node.id === selectedNode?.id, + ); - const kbIsNoLongerReservingScroll = - new Date().getTime() > (isUsingKBToScrollUtill.current ?? 0); + const ref = parentRef.current; + if ( + ref != null && + selectedTreeNode != null && + selectedNode?.source === 'visualiser' + ) { + ref.scrollLeft = + Math.max(0, selectedTreeNode.depth - 10) * renderDepthOffset; - if (idx !== -1 && kbIsNoLongerReservingScroll) { - parentRef.current!!.scrollLeft = - Math.max(0, treeNodes[idx].depth - 10) * renderDepthOffset; + let scrollToIndex = selectedTreeNode.idx; - 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; 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; 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, 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, hoveredNode: Id | undefined, selectedNode: Id | undefined, - onSelectNode: (id?: Id) => void, + onSelectNode: OnSelectNode, onHoverNode: (id?: Id) => void, isUsingKBToScrollUntill: React.MutableRefObject, ) { @@ -818,7 +833,7 @@ function moveSelectedNodeViaKeyBoard( newIdx: number, treeNodes: TreeNode[], rowVirtualizer: Virtualizer, - onSelectNode: (id?: Id) => void, + onSelectNode: OnSelectNode, onHoverNode: (id?: Id) => void, isUsingKBToScrollUntil: React.MutableRefObject, ) { @@ -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'}); diff --git a/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx b/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx index ec0a27aa5..e553b3959 100644 --- a/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx +++ b/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; - onSelectNode: (id?: Id) => void; + onSelectNode: OnSelectNode; } & React.HTMLAttributes > = ({width, nodes, onSelectNode}) => { const rootNodeRef = useRef(); @@ -129,7 +136,7 @@ export const Visualization2D: React.FC< {selectedNodeId && ( )} @@ -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'); }}> diff --git a/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx b/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx index 4c2e58023..2139caa66 100644 --- a/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx +++ b/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx @@ -41,7 +41,7 @@ export const Inspector: React.FC = ({ 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; diff --git a/desktop/plugins/public/ui-debugger/hooks/useDelay.tsx b/desktop/plugins/public/ui-debugger/hooks/useDelay.tsx index 73216cc91..81dda2aa9 100644 --- a/desktop/plugins/public/ui-debugger/hooks/useDelay.tsx +++ b/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) { diff --git a/desktop/plugins/public/ui-debugger/index.tsx b/desktop/plugins/public/ui-debugger/index.tsx index 6c13ac629..3a5756466 100644 --- a/desktop/plugins/public/ui-debugger/index.tsx +++ b/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) { highlightedNodes, - selectedNode: createState(undefined), + selectedNode: createState(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) { }; } -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>): UIActions { const onExpandNode = (node: Id) => { uiState.expandedNodes.update((draft) => { draft.add(node); }); }; - const onSelectNode = (node?: Id) => { - if (uiState.selectedNode.get() === node) { + const onSelectNode = (node: Id | undefined, source: SelectionSource) => { + 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; diff --git a/desktop/plugins/public/ui-debugger/tracker.tsx b/desktop/plugins/public/ui-debugger/tracker.tsx index 4788b83db..b0616cc5a 100644 --- a/desktop/plugins/public/ui-debugger/tracker.tsx +++ b/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 = { diff --git a/desktop/plugins/public/ui-debugger/types.tsx b/desktop/plugins/public/ui-debugger/types.tsx index 3b814b41e..35b39c94c 100644 --- a/desktop/plugins/public/ui-debugger/types.tsx +++ b/desktop/plugins/public/ui-debugger/types.tsx @@ -16,7 +16,7 @@ export type UIState = { searchTerm: Atom; isContextMenuOpen: Atom; hoveredNodes: Atom; - selectedNode: Atom; + selectedNode: Atom; highlightedNodes: Atom>; focusedNode: Atom; expandedNodes: Atom>; @@ -24,6 +24,27 @@ export type UIState = { frameworkEventMonitoring: Atom>; }; +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'}