From 0651bb27dfe3f4982f1c42e61237475145bc98da Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Fri, 17 Feb 2023 02:45:05 -0800 Subject: [PATCH] Fix automatic scrolling bug Summary: Fixes https://fb.workplace.com/groups/443457641253219/permalink/522121466720169/ For context see changelog. The issue was because when an update comes in it creates an entirely new treeList. Since we were accessing that treeList in useEffect it was added to the dependency array. Therefore when an update came in we would scroll back to the last selected node. This effect was only meant to run when the selection changed in the visualiser. To fix we have to put the data it depends on in a ref so it can access the latest value without needing this data in the dependency array changelog: UIDebugger Fix bug where if video playing on android and if element selected it would sometimes jump back to selected element when you scroll away Reviewed By: mweststrate Differential Revision: D43347501 fbshipit-source-id: f03bb32ddfa7828a4742d1a57e9be133a455ec30 --- desktop/plugins/public/ui-debugger/components/Tree.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/desktop/plugins/public/ui-debugger/components/Tree.tsx b/desktop/plugins/public/ui-debugger/components/Tree.tsx index 18958877d..5cfc1db63 100644 --- a/desktop/plugins/public/ui-debugger/components/Tree.tsx +++ b/desktop/plugins/public/ui-debugger/components/Tree.tsx @@ -13,6 +13,7 @@ import React, { Ref, RefObject, useEffect, + useLayoutEffect, useMemo, useRef, } from 'react'; @@ -96,7 +97,7 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { const scrollContainerRef = useRef(null); - useEffect(() => { + useLayoutEffect(() => { if (selectedNode) { const idx = treeNodes.findIndex((node) => node.id === selectedNode); if (idx !== -1) { @@ -108,7 +109,11 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { }); } } - }, [refs, selectedNode, treeNodes]); + // NOTE: We don't want to add refs or tree nodes to the dependency list since when new data comes in over the wire + // otherwise we will keep scrolling back to the selected node overriding the users manual scroll offset. + // We only should scroll when selection changes + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [selectedNode]); return (