From 36447d550a3ea1b78b45f19460dee37626aadbfa Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Mon, 10 Jul 2023 09:22:39 -0700 Subject: [PATCH] Keyboard controls moved selected and hovered node together Summary: Following feedback when using keyboard controls its a little bit awkward to have to move with arrows and then select with enter. Now when using keyboard controls you are manipulating the selected state. Enter still selects / unselects but its not really needed anymore When using the mouse the hover state is still there Changelog: [UIDebugger] Using keyboard arrow control changes the selected and hovered state together for faster / easier navigation Reviewed By: lblasa Differential Revision: D47212492 fbshipit-source-id: 996196880d623885b4d4b7d1a70954201f809d28 --- .../public/ui-debugger/components/Tree.tsx | 55 +++++++++++++------ desktop/plugins/public/ui-debugger/index.tsx | 10 +++- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/desktop/plugins/public/ui-debugger/components/Tree.tsx b/desktop/plugins/public/ui-debugger/components/Tree.tsx index 3211c0ed1..687b9d94e 100644 --- a/desktop/plugins/public/ui-debugger/components/Tree.tsx +++ b/desktop/plugins/public/ui-debugger/components/Tree.tsx @@ -122,6 +122,7 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { selectedNode, hoveredNode, instance.uiActions.onSelectNode, + instance.uiActions.onHoverNode, instance.uiActions.onExpandNode, instance.uiActions.onCollapseNode, isUsingKBToScrollUtill, @@ -163,7 +164,11 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { useLayoutEffect(() => { if (selectedNode) { const idx = treeNodes.findIndex((node) => node.id === selectedNode); - if (idx !== -1) { + + const kbIsNoLongerReservingScroll = + new Date().getTime() > (isUsingKBToScrollUtill.current ?? 0); + + if (idx !== -1 && kbIsNoLongerReservingScroll) { parentRef.current!!.scrollLeft = Math.max(0, treeNodes[idx].depth - 10) * renderDepthOffset; @@ -680,6 +685,7 @@ function useKeyboardShortcuts( selectedNode: Id | undefined, hoveredNodeId: Id | undefined, onSelectNode: (id?: Id) => void, + onHoverNode: (id?: Id) => void, onExpandNode: (id: Id) => void, onCollapseNode: (id: Id) => void, isUsingKBToScrollUntill: React.MutableRefObject, @@ -692,7 +698,6 @@ function useKeyboardShortcuts( switch (event.key) { case 'Enter': { if (hoveredNodeId != null) { - extendKBControlLease(isUsingKBToScrollUntill); onSelectNode(hoveredNodeId); } @@ -703,11 +708,14 @@ function useKeyboardShortcuts( event.preventDefault(); if (hoveredNode) { if (hoveredNode.isExpanded) { - moveHoveredNodeUpOrDown( + moveSelectedNodeUpOrDown( 'ArrowDown', treeNodes, rowVirtualizer, - instance.uiState.hoveredNodes, + hoveredNodeId, + selectedNode, + onSelectNode, + onHoverNode, isUsingKBToScrollUntill, ); } else { @@ -724,11 +732,12 @@ function useKeyboardShortcuts( const parentIdx = treeNodes.findIndex( (treeNode) => treeNode.id === hoveredNode.parent, ); - moveHoveredNodeViaKeyBoard( + moveSelectedNodeViaKeyBoard( parentIdx, treeNodes, rowVirtualizer, - instance.uiState.hoveredNodes, + onSelectNode, + onHoverNode, isUsingKBToScrollUntill, ); } @@ -740,11 +749,14 @@ function useKeyboardShortcuts( case 'ArrowDown': event.preventDefault(); - moveHoveredNodeUpOrDown( + moveSelectedNodeUpOrDown( event.key, treeNodes, rowVirtualizer, - instance.uiState.hoveredNodes, + hoveredNodeId, + selectedNode, + onSelectNode, + onHoverNode, isUsingKBToScrollUntill, ); @@ -765,47 +777,54 @@ function useKeyboardShortcuts( instance.uiState.hoveredNodes, hoveredNodeId, rowVirtualizer, + onHoverNode, ]); } export type UpOrDown = 'ArrowDown' | 'ArrowUp'; -function moveHoveredNodeUpOrDown( +function moveSelectedNodeUpOrDown( direction: UpOrDown, treeNodes: TreeNode[], rowVirtualizer: Virtualizer, - hoveredNodes: Atom, + hoveredNode: Id | undefined, + selectedNode: Id | undefined, + onSelectNode: (id?: Id) => void, + onHoverNode: (id?: Id) => void, isUsingKBToScrollUntill: React.MutableRefObject, ) { - const curIdx = treeNodes.findIndex( - (item) => item.id === head(hoveredNodes.get()), - ); + const nodeToUse = selectedNode != null ? selectedNode : hoveredNode; + const curIdx = treeNodes.findIndex((item) => item.id === nodeToUse); if (curIdx != -1) { const increment = direction === 'ArrowDown' ? 1 : -1; const newIdx = curIdx + increment; - moveHoveredNodeViaKeyBoard( + moveSelectedNodeViaKeyBoard( newIdx, treeNodes, rowVirtualizer, - hoveredNodes, + onSelectNode, + onHoverNode, isUsingKBToScrollUntill, ); } } -function moveHoveredNodeViaKeyBoard( +function moveSelectedNodeViaKeyBoard( newIdx: number, treeNodes: TreeNode[], rowVirtualizer: Virtualizer, - hoveredNodes: Atom, + onSelectNode: (id?: Id) => void, + onHoverNode: (id?: Id) => void, isUsingKBToScrollUntil: React.MutableRefObject, ) { if (newIdx >= 0 && newIdx < treeNodes.length) { const newNode = treeNodes[newIdx]; - hoveredNodes.set([newNode.id]); extendKBControlLease(isUsingKBToScrollUntil); + onSelectNode(newNode.id); + onHoverNode(newNode.id); + rowVirtualizer.scrollToIndex(newIdx, {align: 'auto'}); } } diff --git a/desktop/plugins/public/ui-debugger/index.tsx b/desktop/plugins/public/ui-debugger/index.tsx index 8024b5507..6c13ac629 100644 --- a/desktop/plugins/public/ui-debugger/index.tsx +++ b/desktop/plugins/public/ui-debugger/index.tsx @@ -372,7 +372,7 @@ export function plugin(client: PluginClient) { } type UIActions = { - onHoverNode: (node: Id) => void; + onHoverNode: (node?: Id) => void; onFocusNode: (focused?: Id) => void; onContextMenuOpen: (open: boolean) => void; onSelectNode: (node?: Id) => void; @@ -418,8 +418,12 @@ function uiActions(uiState: UIState, nodes: Atom>): UIActions { }); }; - const onHoverNode = (node: Id) => { - uiState.hoveredNodes.set([node]); + const onHoverNode = (node?: Id) => { + if (node != null) { + uiState.hoveredNodes.set([node]); + } else { + uiState.hoveredNodes.set([]); + } }; const onContextMenuOpen = (open: boolean) => {