Skip to content

Commit 1a2dd28

Browse files
committed
fix[Tree]: only scroll to item when panel is visible
1 parent ecb8df4 commit 1a2dd28

File tree

4 files changed

+62
-25
lines changed

4 files changed

+62
-25
lines changed

packages/react-devtools-extensions/src/main/index.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,12 @@ function createComponentsPanel() {
206206
}
207207
});
208208

209-
// TODO: we should listen to createdPanel.onHidden to unmount some listeners
210-
// and potentially stop highlighting
209+
createdPanel.onShown.addListener(() => {
210+
bridge.emit('extensionComponentsPanelShown');
211+
});
212+
createdPanel.onHidden.addListener(() => {
213+
bridge.emit('extensionComponentsPanelHidden');
214+
});
211215
},
212216
);
213217
}

packages/react-devtools-shared/src/bridge.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ type FrontendEvents = {
217217
clearWarningsForElementID: [ElementAndRendererID],
218218
copyElementPath: [CopyElementPathParams],
219219
deletePath: [DeletePath],
220+
extensionComponentsPanelShown: [],
221+
extensionComponentsPanelHidden: [],
220222
getBackendVersion: [],
221223
getBridgeProtocol: [],
222224
getIfHasUnsupportedRendererVersion: [],

packages/react-devtools-shared/src/devtools/views/Components/Tree.js

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import styles from './Tree.css';
3737
import ButtonIcon from '../ButtonIcon';
3838
import Button from '../Button';
3939
import {logEvent} from 'react-devtools-shared/src/Logger';
40+
import {useExtensionComponentsPanelVisibility} from 'react-devtools-shared/src/frontend/hooks/useExtensionComponentsPanelVisibility';
4041

4142
// Never indent more than this number of pixels (even if we have the room).
4243
const DEFAULT_INDENTATION_SIZE = 12;
@@ -78,36 +79,28 @@ export default function Tree(): React.Node {
7879
const bridge = useContext(BridgeContext);
7980
const store = useContext(StoreContext);
8081
const {hideSettings} = useContext(OptionsContext);
82+
const {lineHeight} = useContext(SettingsContext);
83+
8184
const [isNavigatingWithKeyboard, setIsNavigatingWithKeyboard] =
8285
useState(false);
8386
const {highlightHostInstance, clearHighlightHostInstance} =
8487
useHighlightHostInstance();
88+
const [treeFocused, setTreeFocused] = useState<boolean>(false);
89+
const componentsPanelVisible = useExtensionComponentsPanelVisibility(bridge);
90+
8591
const treeRef = useRef<HTMLDivElement | null>(null);
8692
const focusTargetRef = useRef<HTMLDivElement | null>(null);
93+
const listRef = useRef(null);
8794

88-
const [treeFocused, setTreeFocused] = useState<boolean>(false);
89-
90-
const {lineHeight} = useContext(SettingsContext);
95+
useEffect(() => {
96+
if (!componentsPanelVisible) {
97+
return;
98+
}
9199

92-
// Make sure a newly selected element is visible in the list.
93-
// This is helpful for things like the owners list and search.
94-
//
95-
// TRICKY:
96-
// It's important to use a callback ref for this, rather than a ref object and an effect.
97-
// As an optimization, the AutoSizer component does not render children when their size would be 0.
98-
// This means that in some cases (if the browser panel size is initially really small),
99-
// the Tree component might render without rendering an inner List.
100-
// In this case, the list ref would be null on mount (when the scroll effect runs),
101-
// meaning the scroll action would be skipped (since ref updates don't re-run effects).
102-
// Using a callback ref accounts for this case...
103-
const listCallbackRef = useCallback(
104-
(list: $FlowFixMe) => {
105-
if (list != null && inspectedElementIndex !== null) {
106-
list.scrollToItem(inspectedElementIndex, 'smart');
107-
}
108-
},
109-
[inspectedElementIndex],
110-
);
100+
if (listRef.current != null && inspectedElementIndex !== null) {
101+
listRef.current.scrollToItem(inspectedElementIndex, 'smart');
102+
}
103+
}, [inspectedElementIndex, componentsPanelVisible]);
111104

112105
// Picking an element in the inspector should put focus into the tree.
113106
// This ensures that keyboard navigation works right after picking a node.
@@ -449,7 +442,7 @@ export default function Tree(): React.Node {
449442
itemData={itemData}
450443
itemKey={itemKey}
451444
itemSize={lineHeight}
452-
ref={listCallbackRef}
445+
ref={listRef}
453446
width={width}>
454447
{Element}
455448
</FixedSizeList>
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
import {useState, useEffect} from 'react';
11+
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
12+
13+
// Events that are prefixed with `extension` will only be emitted for the browser extension implementation.
14+
// For other implementations, this hook will just return constant `true` value.
15+
export function useExtensionComponentsPanelVisibility(
16+
bridge: FrontendBridge,
17+
): boolean {
18+
const [isVisible, setIsVisible] = useState(true);
19+
20+
useEffect(() => {
21+
function onPanelShown() {
22+
setIsVisible(true);
23+
}
24+
function onPanelHidden() {
25+
setIsVisible(false);
26+
}
27+
28+
bridge.addListener('extensionComponentsPanelShown', onPanelShown);
29+
bridge.addListener('extensionComponentsPanelHidden', onPanelHidden);
30+
31+
return () => {
32+
bridge.removeListener('extensionComponentsPanelShown', onPanelShown);
33+
bridge.removeListener('extensionComponentsPanelHidden', onPanelHidden);
34+
};
35+
}, [bridge]);
36+
37+
return isVisible;
38+
}

0 commit comments

Comments
 (0)