Fixed a selection-sync edge case for Elements/Components tabs
Brian Vaughn committed
Aug 12, 2019 at 10:31 UTC
8fa608d9ab881b1a6287044c0f7d80e189c1b408
1 file changed
+19
-14
src/devtools/views/Components/Tree.js
+19
-14
@@ -3,13 +3,12 @@
3
import React, {
4
Fragment,
5
Suspense,
6
- useState,
6
useCallback,
7
useContext,
8
useEffect,
9
useMemo,
11
- useLayoutEffect,
10
useRef,
11
+ useState,
12
} from 'react';
13
import AutoSizer from 'react-virtualized-auto-sizer';
14
import { FixedSizeList } from 'react-window';
@@ -54,8 +53,6 @@ export default function Tree(props: Props) {
53
const [isNavigatingWithKeyboard, setIsNavigatingWithKeyboard] = useState(
54
false
55
);
57
- // $FlowFixMe https://github.com/facebook/flow/issues/7341
58
- const listRef = useRef<FixedSizeList<ItemData> | null>(null);
56
const treeRef = useRef<HTMLDivElement | null>(null);
57
const focusTargetRef = useRef<HTMLDivElement | null>(null);
58
@@ -65,15 +62,23 @@ export default function Tree(props: Props) {
62
63
// Make sure a newly selected element is visible in the list.
64
// This is helpful for things like the owners list and search.
68
- useLayoutEffect(() => {
69
- if (selectedElementIndex !== null && listRef.current != null) {
70
- listRef.current.scrollToItem(selectedElementIndex, 'smart');
71
- // Note this autoscroll only works for rows.
72
- // There's another autoscroll inside the elements
73
- // that ensures the component name is visible horizontally.
74
- // It's too early to do it now because the row might not exist yet.
75
- }
76
- }, [listRef, selectedElementIndex]);
65
+ //
66
+ // TRICKY:
67
+ // It's important to use a callback ref for this, rather than a ref object and an effect.
68
+ // As an optimization, the AutoSizer component does not render children when their size would be 0.
69
+ // This means that in some cases (if the browser panel size is initially really small),
70
+ // the Tree component might render without rendering an inner List.
71
+ // In this case, the list ref would be null on mount (when the scroll effect runs),
72
+ // meaning the scroll action would be skipped (since ref updates don't re-run effects).
73
+ // Using a callback ref accounts for this case...
74
+ const listCallbackRef = useCallback(
75
+ list => {
76
+ if (list != null && selectedElementIndex !== null) {
77
+ list.scrollToItem(selectedElementIndex, 'smart');
78
+ }
79
+ },
80
+ [selectedElementIndex]
81
+ );
82
83
// Picking an element in the inspector should put focus into the tree.
84
// This ensures that keyboard navigation works right after picking a node.
@@ -322,7 +327,7 @@ export default function Tree(props: Props) {
327
itemData={itemData}
328
itemKey={itemKey}
329
itemSize={lineHeight}
325
- ref={listRef}
330
+ ref={listCallbackRef}
331
width={width}
332
>
333
{ElementView}