Newly selected components always auto-expand their ancestors
Brian Vaughn committed
Apr 10, 2019 at 08:57 UTC
dd84e8ff2bbb8193f585ec23d870716a0e2151c0
3 files changed
+30
-21
src/devtools/store.js
+6
-1
@@ -353,7 +353,12 @@ export default class Store extends EventEmitter {
353
break;
354
}
355
const child = ((this._idToElement.get(childID): any): Element);
356
- index += child.isCollapsed ? 1 : child.weight;
356
+
357
+ // We intentionally ignore collapsed state when determining an item's index.
358
+ // We only do this for the direct path to an element.
359
+ // That's because the index of the element is meaningless if it's inside of a collapsed tree.
360
+ // If this index is used to display the element, the caller should also un-collapse its ancestors.
361
+ index += child.weight;
362
}
363
364
previousID = current.id;
src/devtools/views/Components/TreeContext.js
+18
-15
@@ -22,6 +22,7 @@ import React, {
22
useCallback,
23
useContext,
24
useEffect,
25
+ useLayoutEffect,
26
useMemo,
27
useReducer,
28
useRef,
@@ -110,6 +111,8 @@ function reduceTreeState(store: Store, state: State, action: Action): State {
111
selectedElementID,
112
} = state;
113
114
+ let lookupIDForIndex = true;
115
+
116
// Base tree should ignore selected element changes when the owner's tree is active.
117
if (ownerStack.length === 0) {
118
switch (type) {
@@ -128,6 +131,11 @@ function reduceTreeState(store: Store, state: State, action: Action): State {
131
selectedElementIndex = ((payload: any): number | null);
132
break;
133
case 'SELECT_ELEMENT_BY_ID':
134
+ // Skip lookup in this case; it would be redundant.
135
+ // It might also cause problems if the specified element was inside of a (not yet expanded) subtree.
136
+ lookupIDForIndex = false;
137
+
138
+ selectedElementID = payload;
139
selectedElementIndex =
140
payload === null
141
? null
@@ -168,7 +176,7 @@ function reduceTreeState(store: Store, state: State, action: Action): State {
176
}
177
178
// Keep selected item ID and index in sync.
171
- if (selectedElementIndex !== state.selectedElementIndex) {
179
+ if (lookupIDForIndex && selectedElementIndex !== state.selectedElementIndex) {
180
if (selectedElementIndex === null) {
181
selectedElementID = null;
182
} else {
@@ -687,19 +695,14 @@ function TreeContextController({ children, viewElementSource }: Props) {
695
return () => bridge.removeListener('selectFiber', handleSelectFiber);
696
}, [bridge, dispatch]);
697
690
- // If a newly-selected search result is inside of a collapsed subtree, auto expand it.
691
- // We also need to handle when the search text changed (selecting a new element) without changing the index.
692
- const prevSearchIndex = useRef<number | null>(null);
693
- const prevSearchText = useRef<string>('');
694
- useEffect(() => {
695
- if (
696
- state.searchIndex !== prevSearchIndex.current ||
697
- state.searchText !== prevSearchText.current
698
- ) {
699
- prevSearchIndex.current = state.searchIndex;
700
- prevSearchText.current = state.searchText;
701
-
702
- if (state.searchIndex !== null && state.selectedElementID !== null) {
698
+ // If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it.
699
+ // This needs to be a layout effect to avoid temporarily flashing an incorrect selection.
700
+ const prevSelectedElementID = useRef<number | null>(null);
701
+ useLayoutEffect(() => {
702
+ if (state.selectedElementID !== prevSelectedElementID.current) {
703
+ prevSelectedElementID.current = state.selectedElementID;
704
+
705
+ if (state.selectedElementID !== null) {
706
let element = store.getElementByID(state.selectedElementID);
707
while (element !== null && element.parentID > 0) {
708
element = ((store.getElementByID(element.parentID): any): Element);
@@ -709,7 +712,7 @@ function TreeContextController({ children, viewElementSource }: Props) {
712
}
713
}
714
}
712
- }, [state.searchIndex, state.searchText, state.selectedElementID, store]);
715
+ }, [state.selectedElementID, store]);
716
717
// Mutations to the underlying tree may impact this context (e.g. search results, selection state).
718
useEffect(() => {
src/devtools/views/Profiler/ProfilerContext.js
+6
-5
@@ -79,7 +79,7 @@ type Props = {|
79
80
function ProfilerContextController({ children }: Props) {
81
const store = useContext(StoreContext);
82
- const { selectElementAtIndex, selectedElementID } = useContext(TreeContext);
82
+ const { selectElementByID, selectedElementID } = useContext(TreeContext);
83
84
const subscription = useMemo(
85
() => ({
@@ -152,13 +152,14 @@ function ProfilerContextController({ children }: Props) {
152
selectFiberID(id);
153
selectFiberName(name);
154
if (id !== null) {
155
- const index = store.getIndexOfElementID(id);
156
- if (index !== null) {
157
- selectElementAtIndex(index);
155
+ // If this element is still in the store, then select it in the Components tab as well.
156
+ const element = store.getElementByID(id);
157
+ if (element !== null) {
158
+ selectElementByID(id);
159
}
160
}
161
},
161
- [selectElementAtIndex, selectFiberID, selectFiberName, store]
162
+ [selectElementByID, selectFiberID, selectFiberName, store]
163
);
164
165
if (isProfiling) {