diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 0dd3084af5aa9..e37a47daf6b6b 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -5651,6 +5651,23 @@ export function attach( } } + function findLastKnownRectsForID(id: number): null | Array { + try { + const devtoolsInstance = idToDevToolsInstanceMap.get(id); + if (devtoolsInstance === undefined) { + console.warn(`Could not find DevToolsInstance with id "${id}"`); + return null; + } + if (devtoolsInstance.suspenseNode === null) { + return null; + } + return devtoolsInstance.suspenseNode.rects; + } catch (err) { + // The fiber might have unmounted by now. + return null; + } + } + function getDisplayNameForElementID(id: number): null | string { const devtoolsInstance = idToDevToolsInstanceMap.get(id); if (devtoolsInstance === undefined) { @@ -8387,6 +8404,7 @@ export function attach( getSerializedElementValueByPath, deletePath, findHostInstancesForElementID, + findLastKnownRectsForID, flushInitialOperations, getBestMatchForTrackedPath, getDisplayNameForElementID, diff --git a/packages/react-devtools-shared/src/backend/flight/renderer.js b/packages/react-devtools-shared/src/backend/flight/renderer.js index 75763b1f18499..d0dc9094334eb 100644 --- a/packages/react-devtools-shared/src/backend/flight/renderer.js +++ b/packages/react-devtools-shared/src/backend/flight/renderer.js @@ -152,6 +152,9 @@ export function attach( findHostInstancesForElementID() { return null; }, + findLastKnownRectsForID() { + return null; + }, flushInitialOperations() {}, getBestMatchForTrackedPath() { return null; diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index b59c0292942c6..8a245155ef2cc 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1168,6 +1168,9 @@ export function attach( const hostInstance = findHostInstanceForInternalID(id); return hostInstance == null ? null : [hostInstance]; }, + findLastKnownRectsForID() { + return null; + }, getOwnersList, getPathForElement, getProfilingData, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 481bf65e210ed..e002740cb69f7 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -101,6 +101,16 @@ export type FindHostInstancesForElementID = ( id: number, ) => null | $ReadOnlyArray; +type Rect = { + x: number, + y: number, + width: number, + height: number, + ... +}; +export type FindLastKnownRectsForID = ( + id: number, +) => null | $ReadOnlyArray; export type ReactProviderType = { $$typeof: symbol | number, _context: ReactContext, @@ -411,6 +421,7 @@ export type RendererInterface = { path: Array, ) => void, findHostInstancesForElementID: FindHostInstancesForElementID, + findLastKnownRectsForID: FindLastKnownRectsForID, flushInitialOperations: () => void, getBestMatchForTrackedPath: () => PathMatch | null, getComponentStack?: GetComponentStack, diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js index 419fefc4ffcf8..2b651cdc9cc45 100644 --- a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js +++ b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js @@ -11,6 +11,7 @@ import Agent from 'react-devtools-shared/src/backend/agent'; import {hideOverlay, showOverlay} from './Highlighter'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; +import type {RendererInterface} from '../../types'; // This plug-in provides in-page highlighting of the selected element. // It is used by the browser extension and the standalone DevTools shell (when connected to a browser). @@ -25,6 +26,7 @@ export default function setupHighlighter( ): void { bridge.addListener('clearHostInstanceHighlight', clearHostInstanceHighlight); bridge.addListener('highlightHostInstance', highlightHostInstance); + bridge.addListener('scrollToHostInstance', scrollToHostInstance); bridge.addListener('shutdown', stopInspectingHost); bridge.addListener('startInspectingHost', startInspectingHost); bridge.addListener('stopInspectingHost', stopInspectingHost); @@ -111,24 +113,162 @@ export default function setupHighlighter( } const nodes = renderer.findHostInstancesForElementID(id); + if (nodes != null) { + for (let i = 0; i < nodes.length; i++) { + const node = nodes[0]; + if (node === null) { + continue; + } + const nodeRects = + // $FlowFixMe[method-unbinding] + typeof node.getClientRects === 'function' + ? node.getClientRects() + : []; + // If this is currently display: none, then try another node. + // This can happen when one of the host instances is a hoistable. + if ( + nodeRects.length > 0 && + (nodeRects.length > 2 || + nodeRects[0].width > 0 || + nodeRects[0].height > 0) + ) { + // $FlowFixMe[method-unbinding] + if (scrollIntoView && typeof node.scrollIntoView === 'function') { + if (scrollDelayTimer) { + clearTimeout(scrollDelayTimer); + scrollDelayTimer = null; + } + // If the node isn't visible show it before highlighting it. + // We may want to reconsider this; it might be a little disruptive. + node.scrollIntoView({block: 'nearest', inline: 'nearest'}); + } + + showOverlay(nodes, displayName, agent, hideAfterTimeout); + + if (openBuiltinElementsPanel) { + window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node; + bridge.send('syncSelectionToBuiltinElementsPanel'); + } + return; + } + } + } - if (nodes != null && nodes[0] != null) { - const node = nodes[0]; - // $FlowFixMe[method-unbinding] - if (scrollIntoView && typeof node.scrollIntoView === 'function') { - // If the node isn't visible show it before highlighting it. - // We may want to reconsider this; it might be a little disruptive. - node.scrollIntoView({block: 'nearest', inline: 'nearest'}); + hideOverlay(agent); + } + + function attemptScrollToHostInstance( + renderer: RendererInterface, + id: number, + ) { + const nodes = renderer.findHostInstancesForElementID(id); + if (nodes != null) { + for (let i = 0; i < nodes.length; i++) { + const node = nodes[0]; + if (node === null) { + continue; + } + const nodeRects = + // $FlowFixMe[method-unbinding] + typeof node.getClientRects === 'function' + ? node.getClientRects() + : []; + // If this is currently display: none, then try another node. + // This can happen when one of the host instances is a hoistable. + if ( + nodeRects.length > 0 && + (nodeRects.length > 2 || + nodeRects[0].width > 0 || + nodeRects[0].height > 0) + ) { + // $FlowFixMe[method-unbinding] + if (typeof node.scrollIntoView === 'function') { + node.scrollIntoView({ + block: 'nearest', + inline: 'nearest', + behavior: 'smooth', + }); + return true; + } + } } + } + return false; + } + + let scrollDelayTimer = null; + function scrollToHostInstance({ + id, + rendererID, + }: { + id: number, + rendererID: number, + }) { + // Always hide the existing overlay so it doesn't obscure the element. + // If you wanted to show the overlay, highlightHostInstance should be used instead + // with the scrollIntoView option. + hideOverlay(agent); - showOverlay(nodes, displayName, agent, hideAfterTimeout); + if (scrollDelayTimer) { + clearTimeout(scrollDelayTimer); + scrollDelayTimer = null; + } - if (openBuiltinElementsPanel) { - window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node; - bridge.send('syncSelectionToBuiltinElementsPanel'); + const renderer = agent.rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + return; + } + + // In some cases fiber may already be unmounted + if (!renderer.hasElementWithId(id)) { + return; + } + + if (attemptScrollToHostInstance(renderer, id)) { + return; + } + + // It's possible that the current state of a Suspense boundary doesn't have a position + // in the tree. E.g. because it's not yet mounted in the state we're moving to. + // Such as if it's in a null tree or inside another boundary's hidden state. + // In this case we use the last known position and try to scroll to that. + const rects = renderer.findLastKnownRectsForID(id); + if (rects !== null && rects.length > 0) { + let x = Infinity; + let y = Infinity; + for (let i = 0; i < rects.length; i++) { + const rect = rects[i]; + if (rect.x < x) { + x = rect.x; + } + if (rect.y < y) { + y = rect.y; + } } - } else { - hideOverlay(agent); + const element = document.documentElement; + if (!element) { + return; + } + // Check if the target corner is already in the viewport. + if ( + x < window.scrollX || + y < window.scrollY || + x > window.scrollX + element.clientWidth || + y > window.scrollY + element.clientHeight + ) { + window.scrollTo({ + top: y, + left: x, + behavior: 'smooth', + }); + } + // It's possible that after mount, we're able to scroll deeper once the new nodes + // have mounted. Let's try again after mount. Ideally we'd know which commit this + // is going to be but for now we just try after 100ms. + scrollDelayTimer = setTimeout(() => { + attemptScrollToHostInstance(renderer, id); + }, 100); } } diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 616f2d3d3ec23..aa9c867e1f1f2 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -93,6 +93,10 @@ type HighlightHostInstance = { scrollIntoView: boolean, }; +type ScrollToHostInstance = { + ...ElementAndRendererID, +}; + type OverrideValue = { ...ElementAndRendererID, path: Array, @@ -254,6 +258,7 @@ type FrontendEvents = { startInspectingHost: [], startProfiling: [StartProfilingParams], stopInspectingHost: [boolean], + scrollToHostInstance: [ScrollToHostInstance], stopProfiling: [], storeAsGlobal: [StoreAsGlobalParams], updateComponentFilters: [Array], diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js index 18901b45b9ffb..b7340da915b9b 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js @@ -8,10 +8,10 @@ */ import * as React from 'react'; -import {useContext, useEffect} from 'react'; +import {useContext, useEffect, useRef} from 'react'; import {BridgeContext, StoreContext} from '../context'; import {TreeDispatcherContext} from '../Components/TreeContext'; -import {useHighlightHostInstance} from '../hooks'; +import {useHighlightHostInstance, useScrollToHostInstance} from '../hooks'; import { SuspenseTreeDispatcherContext, SuspenseTreeStateContext, @@ -28,6 +28,7 @@ function SuspenseTimelineInput() { const suspenseTreeDispatch = useContext(SuspenseTreeDispatcherContext); const {highlightHostInstance, clearHighlightHostInstance} = useHighlightHostInstance(); + const scrollToHostInstance = useScrollToHostInstance(); const { selectedRootID: rootID, @@ -77,7 +78,6 @@ function SuspenseTimelineInput() { function skipPrevious() { const nextSelectedSuspenseID = timeline[timelineIndex - 1]; - highlightHostInstance(nextSelectedSuspenseID); treeDispatch({ type: 'SELECT_ELEMENT_BY_ID', payload: nextSelectedSuspenseID, @@ -90,7 +90,6 @@ function SuspenseTimelineInput() { function skipForward() { const nextSelectedSuspenseID = timeline[timelineIndex + 1]; - highlightHostInstance(nextSelectedSuspenseID); treeDispatch({ type: 'SELECT_ELEMENT_BY_ID', payload: nextSelectedSuspenseID, @@ -108,6 +107,7 @@ function SuspenseTimelineInput() { }); } + const isInitialMount = useRef(true); // TODO: useEffectEvent here once it's supported in all versions DevTools supports. // For now we just exclude it from deps since we don't lint those anyway. function changeTimelineIndex(newIndex: number) { @@ -132,6 +132,16 @@ function SuspenseTimelineInput() { rootID, suspendedSet, }); + if (isInitialMount.current) { + // Skip scrolling on initial mount. Only when we're changing the timeline. + isInitialMount.current = false; + } else { + // When we're scrubbing through the timeline, scroll the current boundary + // into view as it was just revealed. This is after we override the milestone + // to reveal it. + const selectedSuspenseID = timeline[timelineIndex]; + scrollToHostInstance(selectedSuspenseID); + } } useEffect(() => { diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index da69d6b493a19..a4ed2da526e16 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -345,13 +345,13 @@ export function useSubscription({ export function useHighlightHostInstance(): { clearHighlightHostInstance: () => void, - highlightHostInstance: (id: number) => void, + highlightHostInstance: (id: number, scrollIntoView?: boolean) => void, } { const bridge = useContext(BridgeContext); const store = useContext(StoreContext); const highlightHostInstance = useCallback( - (id: number) => { + (id: number, scrollIntoView?: boolean = false) => { const element = store.getElementByID(id); const rendererID = store.getRendererIDForElement(id); if (element !== null && rendererID !== null) { @@ -365,7 +365,7 @@ export function useHighlightHostInstance(): { id, openBuiltinElementsPanel: false, rendererID, - scrollIntoView: false, + scrollIntoView: scrollIntoView, }); } }, @@ -381,3 +381,24 @@ export function useHighlightHostInstance(): { clearHighlightHostInstance, }; } + +export function useScrollToHostInstance(): (id: number) => void { + const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); + + const scrollToHostInstance = useCallback( + (id: number) => { + const element = store.getElementByID(id); + const rendererID = store.getRendererIDForElement(id); + if (element !== null && rendererID !== null) { + bridge.send('scrollToHostInstance', { + id, + rendererID, + }); + } + }, + [store, bridge], + ); + + return scrollToHostInstance; +}