From 795d1de10d5483ac8ed0b5354acc7bdd4b82a375 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Tue, 1 Aug 2023 10:32:29 -0700 Subject: [PATCH] Dont auto expand nodes Summary: Rather than automatically collapsing siblings when using the visualiser instead we take a different approach: 1. The tree starts out fully collapsed 2. Every time you click on the visualiser we expand it and its ancestory chain to expanded nodes. This is exactly how the Dom inspector works. The previous approach of constantly collapsing all siblings when uinsg the visualiser felt too intrusive and taking control from the user. The option is still there but only in the context menu Some ultilities around autocollapsing nodes were removed as they dont make sense anymore that we now send complete frames Changelog: UIDebugger Tree starts collapsed and expands as you click from the visualiser Reviewed By: aigoncharov Differential Revision: D47949843 fbshipit-source-id: 4381d22b12874dde5a89267572bee95f084380e3 --- .../public/ui-debugger/DesktopTypes.tsx | 1 + .../components/visualizer/Visualization2D.tsx | 2 +- desktop/plugins/public/ui-debugger/index.tsx | 24 +------------- .../ui-debugger/plugin/ClientDataUtils.tsx | 15 --------- .../public/ui-debugger/plugin/uiActions.tsx | 31 +++++++------------ 5 files changed, 14 insertions(+), 59 deletions(-) diff --git a/desktop/plugins/public/ui-debugger/DesktopTypes.tsx b/desktop/plugins/public/ui-debugger/DesktopTypes.tsx index db68be5ed..d29d3344a 100644 --- a/desktop/plugins/public/ui-debugger/DesktopTypes.tsx +++ b/desktop/plugins/public/ui-debugger/DesktopTypes.tsx @@ -98,6 +98,7 @@ export type UIActions = { onExpandAllRecursively: (nodeId: Id) => void; onCollapseAllNonAncestors: (nodeId: Id) => void; onCollapseAllRecursively: (nodeId: Id) => void; + ensureAncestorsExpanded: (nodeId: Id) => void; }; export type SelectionSource = diff --git a/desktop/plugins/public/ui-debugger/components/visualizer/Visualization2D.tsx b/desktop/plugins/public/ui-debugger/components/visualizer/Visualization2D.tsx index 10670413a..c8ee3bcc7 100644 --- a/desktop/plugins/public/ui-debugger/components/visualizer/Visualization2D.tsx +++ b/desktop/plugins/public/ui-debugger/components/visualizer/Visualization2D.tsx @@ -131,7 +131,7 @@ export const Visualization2D: React.FC< const onClickOverlay = () => { instance.uiActions.onSelectNode(hoveredNodeId, 'visualiser'); if (hoveredNodeId != null) { - instance.uiActions.onCollapseAllNonAncestors(hoveredNodeId); + instance.uiActions.ensureAncestorsExpanded(hoveredNodeId); } if (targetMode.state !== 'disabled') { diff --git a/desktop/plugins/public/ui-debugger/index.tsx b/desktop/plugins/public/ui-debugger/index.tsx index 16378b5c9..6187ff096 100644 --- a/desktop/plugins/public/ui-debugger/index.tsx +++ b/desktop/plugins/public/ui-debugger/index.tsx @@ -31,10 +31,7 @@ import { } from './DesktopTypes'; import {getStreamInterceptor} from './fb-stubs/StreamInterceptor'; import {prefetchSourceFileLocation} from './components/fb-stubs/IDEContextMenu'; -import { - checkFocusedNodeStillActive, - collapseinActiveChildren, -} from './plugin/ClientDataUtils'; +import {checkFocusedNodeStillActive} from './plugin/ClientDataUtils'; import {uiActions} from './plugin/uiActions'; type PendingData = { @@ -67,10 +64,6 @@ export function plugin(client: PluginClient) { limit: 10 * 1024, }); - //this keeps track of all node ids we have seen so we dont keep reexpanding nodes when they come in again. - //Could probably be removed if we refactor the nodes to be expanded by default and only collapsed is toggled on - const seenNodes = new Set(); - //this holds pending any pending data that needs to be applied in the event of a stream interceptor error //while in the error state more metadata or a more recent frame may come in so both cases need to apply the same darta const pendingData: PendingData = {frame: null, metadata: {}}; @@ -279,21 +272,6 @@ export function plugin(client: PluginClient) { } mutableLiveClientData.nodes = nodes; - uiState.expandedNodes.update((draft) => { - for (const node of nodes.values()) { - if (!seenNodes.has(node.id)) { - draft.add(node.id); - } - seenNodes.add(node.id); - - if (!uiState.isPaused.get()) { - //we need to not do this while paused as you may move to another screen / tab - //and it would collapse the tree node for the activity you were paused on. - collapseinActiveChildren(node, draft); - } - } - }); - if (!uiState.isPaused.get()) { nodesAtom.set(mutableLiveClientData.nodes); snapshot.set(mutableLiveClientData.snapshotInfo); diff --git a/desktop/plugins/public/ui-debugger/plugin/ClientDataUtils.tsx b/desktop/plugins/public/ui-debugger/plugin/ClientDataUtils.tsx index 6f02b6137..21437b86f 100644 --- a/desktop/plugins/public/ui-debugger/plugin/ClientDataUtils.tsx +++ b/desktop/plugins/public/ui-debugger/plugin/ClientDataUtils.tsx @@ -7,24 +7,9 @@ * @format */ -import {Draft} from 'flipper-plugin'; import {ClientNode, Id} from '../ClientTypes'; import {UIState} from '../DesktopTypes'; -export function collapseinActiveChildren( - node: ClientNode, - expandedNodes: Draft>, -) { - if (node.activeChild) { - expandedNodes.add(node.activeChild); - for (const child of node.children) { - if (child !== node.activeChild) { - expandedNodes.delete(child); - } - } - } -} - export function checkFocusedNodeStillActive( uiState: UIState, nodes: Map, diff --git a/desktop/plugins/public/ui-debugger/plugin/uiActions.tsx b/desktop/plugins/public/ui-debugger/plugin/uiActions.tsx index 0d6582669..42ed7910a 100644 --- a/desktop/plugins/public/ui-debugger/plugin/uiActions.tsx +++ b/desktop/plugins/public/ui-debugger/plugin/uiActions.tsx @@ -19,10 +19,7 @@ import { WireFrameMode, } from '../DesktopTypes'; import {tracker} from '../utils/tracker'; -import { - checkFocusedNodeStillActive, - collapseinActiveChildren, -} from './ClientDataUtils'; +import {checkFocusedNodeStillActive} from './ClientDataUtils'; export function uiActions( uiState: UIState, @@ -87,19 +84,19 @@ export function uiActions( }; const onCollapseAllNonAncestors = (nodeId: Id) => { - //this is not the simplest way to achieve this but on android there is a parent pointer missing for the decor view - //due to the nested obversers. + uiState.expandedNodes.update((draft) => { + draft.clear(); + }); + ensureAncestorsExpanded(nodeId); + }; + + const ensureAncestorsExpanded = (nodeId: Id) => { uiState.expandedNodes.update((draft) => { const nodesMap = nodes.get(); - let prevNode: Id | null = null; + let curNode = nodesMap.get(nodeId); while (curNode != null) { - for (const child of curNode.children) { - if (child !== prevNode) { - draft.delete(child); - } - } - prevNode = curNode.id; + draft.add(curNode.id); curNode = nodesMap.get(curNode?.parent ?? 'Nonode'); } }); @@ -177,13 +174,6 @@ export function uiActions( tracker.track('play-pause-toggled', {paused: isPaused}); uiState.isPaused.set(isPaused); if (!isPaused) { - //When going back to play mode then set the atoms to the live state to rerender the latest - //Also need to fixed expanded state for any change in active child state - uiState.expandedNodes.update((draft) => { - liveClientData.nodes.forEach((node) => { - collapseinActiveChildren(node, draft); - }); - }); nodes.set(liveClientData.nodes); snapshot.set(liveClientData.snapshotInfo); checkFocusedNodeStillActive(uiState, nodes.get()); @@ -216,5 +206,6 @@ export function uiActions( onCollapseAllNonAncestors, onExpandAllRecursively, onCollapseAllRecursively, + ensureAncestorsExpanded, }; }