From 281616f462866f382c0fead366c7b2b5172a1ce5 Mon Sep 17 00:00:00 2001 From: Ivan Babak Date: Sun, 21 Apr 2019 01:06:02 -0700 Subject: [PATCH] Support highlighting of all DOM elements of Fragments, not just first Fixes https://github.com/bvaughn/react-devtools-experimental/issues/131 Uses the new function `findAllCurrentHostFibers`. Removes dependency on React's `renderer.findHostInstanceByFiber` function which used to highlight only the first DOM element of a Fragment. Reworked `src/backend/views/Overlay` to support highlighting more than one element rectangle annotated with one tooltip. Fixed minor issues with the tooltip position calculation. --- src/backend/agent.js | 11 +- src/backend/renderer.js | 52 +++++- src/backend/types.js | 3 +- src/backend/views/Highlighter.js | 6 +- src/backend/views/Overlay.js | 300 +++++++++++++++++++++---------- 5 files changed, 261 insertions(+), 111 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 976b838c88..40afd10c4b 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -276,18 +276,19 @@ export default class Agent extends EventEmitter { console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); } - let node: HTMLElement | null = null; + let nodes: ?Array = null; if (renderer !== null) { - node = ((renderer.findNativeByFiberID(id): any): HTMLElement); + nodes = ((renderer.findNativeByFiberID(id): any): ?Array); } - if (node != null) { + if (nodes != null && nodes[0] != null) { + const node = nodes[0]; 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' }); } - showOverlay(((node: any): HTMLElement), displayName, hideAfterTimeout); + showOverlay(nodes, displayName, hideAfterTimeout); if (openNativeElementsPanel) { window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node; this._bridge.send('syncSelectionToNativeElementsPanel'); @@ -585,7 +586,7 @@ export default class Agent extends EventEmitter { // Don't pass the name explicitly. // It will be inferred from DOM tag and Fiber owner. - showOverlay(target, null, false); + showOverlay([target], null, false); this._selectFiberForNode(target); }; diff --git a/src/backend/renderer.js b/src/backend/renderer.js index c4e515829b..eddcafd0b1 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -1258,24 +1258,58 @@ export function attach( currentRootID = -1; } + function findAllCurrentHostFibers(parent: Fiber): $ReadOnlyArray { + const fibers = []; + const currentParent = findCurrentFiberUsingSlowPath(parent); + if (!currentParent) { + return fibers; + } + + // Next we'll drill down this component to find all HostComponent/Text. + let node: Fiber = currentParent; + while (true) { + if (node.tag === HostComponent || node.tag === HostText) { + fibers.push(node); + } else if (node.child) { + node.child.return = node; + node = node.child; + continue; + } + if (node === currentParent) { + return fibers; + } + while (!node.sibling) { + if (!node.return || node.return === currentParent) { + return fibers; + } + node = node.return; + } + node.sibling.return = node.return; + node = node.sibling; + } + // Flow needs the return here, but ESLint complains about it. + // eslint-disable-next-line no-unreachable + return fibers; + } + function findNativeByFiberID(id: number) { try { - const fiber = findCurrentFiberUsingSlowPath(idToFiberMap.get(id)); + let fiber = findCurrentFiberUsingSlowPath(idToFiberMap.get(id)); if (fiber === null) { return null; } + // Special case for a timed-out Suspense. const isTimedOutSuspense = fiber.tag === SuspenseComponent && fiber.memoizedState !== null; - if (!isTimedOutSuspense) { - // Normal case. - return renderer.findHostInstanceByFiber(fiber); - } else { + if (isTimedOutSuspense) { // A timed-out Suspense's findDOMNode is useless. // Try our best to find the fallback directly. const maybeFallbackFiber = (fiber.child && fiber.child.sibling) || fiber; - return renderer.findHostInstanceByFiber(maybeFallbackFiber); + fiber = maybeFallbackFiber; } + const hostFibers = findAllCurrentHostFibers(fiber); + return hostFibers.map(hostFiber => hostFiber.stateNode).filter(Boolean); } catch (err) { // The fiber might have unmounted by now. return null; @@ -1715,9 +1749,9 @@ export function attach( if (result.hooks !== null) { console.log('Hooks:', result.hooks); } - const nativeNode = findNativeByFiberID(id); - if (nativeNode !== null) { - console.log('Node:', nativeNode); + const nativeNodes = findNativeByFiberID(id); + if (nativeNodes !== null) { + console.log('Nodes:', nativeNodes); } if (window.chrome || /firefox/i.test(navigator.userAgent)) { console.log( diff --git a/src/backend/types.js b/src/backend/types.js index 0bd1648233..8b0a759e51 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -25,7 +25,6 @@ export type RendererID = number; type Dispatcher = any; export type ReactRenderer = { - findHostInstanceByFiber: (fiber: Object) => ?NativeType, findFiberByHostInstance: (hostInstance: NativeType) => ?Fiber, version: string, bundleType: BundleType, @@ -103,7 +102,7 @@ export type PathMatch = {| export type RendererInterface = { cleanup: () => void, - findNativeByFiberID: (id: number) => ?NativeType, + findNativeByFiberID: (id: number) => ?Array, flushInitialOperations: () => void, getBestMatchForTrackedPath: () => PathMatch | null, getCommitDetails: (rootID: number, commitIndex: number) => CommitDetails, diff --git a/src/backend/views/Highlighter.js b/src/backend/views/Highlighter.js index c5339f6763..9445d838e3 100644 --- a/src/backend/views/Highlighter.js +++ b/src/backend/views/Highlighter.js @@ -17,7 +17,7 @@ export function hideOverlay() { } export function showOverlay( - element: HTMLElement | null, + elements: Array | null, componentName: string | null, hideAfterTimeout: boolean ) { @@ -25,7 +25,7 @@ export function showOverlay( clearTimeout(timeoutID); } - if (element == null) { + if (elements == null) { return; } @@ -33,7 +33,7 @@ export function showOverlay( overlay = new Overlay(); } - overlay.inspect(element, componentName); + overlay.inspect(elements, componentName); if (hideAfterTimeout) { timeoutID = setTimeout(hideOverlay, SHOW_DURATION); diff --git a/src/backend/views/Overlay.js b/src/backend/views/Overlay.js index 276055f830..55ef361dc6 100644 --- a/src/backend/views/Overlay.js +++ b/src/backend/views/Overlay.js @@ -11,30 +11,17 @@ type Rect = { width: number, }; -// Note that this component is not affected by the active Theme, -// because it highlights elements in the main Chrome window (outside of devtools). +// Note that the Overlay components are not affected by the active Theme, +// because they highlight elements in the main Chrome window (outside of devtools). // The colors below were chosen to roughly match those used by Chrome devtools. -export default class Overlay { - window: window; - container: HTMLElement; + +class OverlayRect { node: HTMLElement; border: HTMLElement; padding: HTMLElement; content: HTMLElement; - tip: HTMLElement; - nameSpan: HTMLElement; - dimSpan: HTMLElement; - constructor() { - // Find the root window, because overlays are positioned relative to it. - let currentWindow = window; - while (currentWindow !== currentWindow.parent) { - currentWindow = currentWindow.parent; - } - - const doc = currentWindow.document; - this.window = currentWindow; - this.container = doc.createElement('div'); + constructor(doc, container) { this.node = doc.createElement('div'); this.border = doc.createElement('div'); this.padding = doc.createElement('div'); @@ -50,59 +37,21 @@ export default class Overlay { position: 'fixed', }); - this.tip = doc.createElement('div'); - assign(this.tip.style, { - backgroundColor: '#333740', - borderRadius: '2px', - fontFamily: - '"SFMono-Regular", Consolas, "Liberation Mono", Menlo, Courier, monospace', - fontWeight: 'bold', - padding: '3px 5px', - pointerEvents: 'none', - position: 'fixed', - fontSize: '12px', - }); - - this.nameSpan = doc.createElement('span'); - this.tip.appendChild(this.nameSpan); - assign(this.nameSpan.style, { - color: '#ee78e6', - borderRight: '1px solid #aaaaaa', - paddingRight: '0.5rem', - marginRight: '0.5rem', - }); - this.dimSpan = doc.createElement('span'); - this.tip.appendChild(this.dimSpan); - assign(this.dimSpan.style, { - color: '#d7d7d7', - }); - - this.container.style.zIndex = '10000000'; this.node.style.zIndex = '10000000'; - this.tip.style.zIndex = '10000000'; - this.container.appendChild(this.node); - this.container.appendChild(this.tip); + this.node.appendChild(this.border); this.border.appendChild(this.padding); this.padding.appendChild(this.content); - doc.body.appendChild(this.container); + container.appendChild(this.node); } remove() { - if (this.container.parentNode) { - this.container.parentNode.removeChild(this.container); + if (this.node.parentNode) { + this.node.parentNode.removeChild(this.node); } } - inspect(node: HTMLElement, name?: ?string) { - // We can't get the size of text nodes or comment nodes. React as of v15 - // heavily uses comment nodes to delimit text. - if (node.nodeType !== Node.ELEMENT_NODE) { - return; - } - const box = getNestedBoundingClientRect(node, this.window); - const dims = getElementDimensions(node); - + update(box, dims) { boxWrap(dims, 'margin', this.node); boxWrap(dims, 'border', this.border); boxWrap(dims, 'padding', this.padding); @@ -128,29 +77,190 @@ export default class Overlay { top: box.top - dims.marginTop + 'px', left: box.left - dims.marginLeft + 'px', }); + } +} + +class OverlayTip { + tip: HTMLElement; + nameSpan: HTMLElement; + dimSpan: HTMLElement; + + constructor(doc, container) { + this.tip = doc.createElement('div'); + assign(this.tip.style, { + display: 'flex', + flexFlow: 'row nowrap', + backgroundColor: '#333740', + borderRadius: '2px', + fontFamily: + '"SFMono-Regular", Consolas, "Liberation Mono", Menlo, Courier, monospace', + fontWeight: 'bold', + padding: '3px 5px', + pointerEvents: 'none', + position: 'fixed', + fontSize: '12px', + whiteSpace: 'nowrap', + }); + + this.nameSpan = doc.createElement('span'); + this.tip.appendChild(this.nameSpan); + assign(this.nameSpan.style, { + color: '#ee78e6', + borderRight: '1px solid #aaaaaa', + paddingRight: '0.5rem', + marginRight: '0.5rem', + }); + this.dimSpan = doc.createElement('span'); + this.tip.appendChild(this.dimSpan); + assign(this.dimSpan.style, { + color: '#d7d7d7', + }); + + this.tip.style.zIndex = '10000000'; + container.appendChild(this.tip); + } + + remove() { + if (this.tip.parentNode) { + this.tip.parentNode.removeChild(this.tip); + } + } + + updateText(name, width, height) { + this.nameSpan.textContent = name; + this.dimSpan.textContent = + Math.round(width) + 'px × ' + Math.round(height) + 'px'; + } + + updatePosition(dims, bounds) { + const tipRect = this.tip.getBoundingClientRect(); + const tipPos = findTipPos(dims, bounds, { + width: tipRect.width, + height: tipRect.height, + }); + assign(this.tip.style, tipPos.style); + } +} + +export default class Overlay { + window: window; + tipBoundsWindow: window; + container: HTMLElement; + tip: OverlayTip; + rects: Array; + + constructor() { + // Find the root window, because overlays are positioned relative to it. + let currentWindow = window; + while (currentWindow !== currentWindow.parent) { + currentWindow = currentWindow.parent; + } + this.window = currentWindow; + + // When opened in shells/dev, the tooltip should be bound by the app iframe, not by the topmost window. + let tipBoundsWindow = window; + while ( + tipBoundsWindow !== tipBoundsWindow.parent && + !tipBoundsWindow.hasOwnProperty('__REACT_DEVTOOLS_GLOBAL_HOOK__') + ) { + tipBoundsWindow = tipBoundsWindow.parent; + } + this.tipBoundsWindow = tipBoundsWindow; + + const doc = currentWindow.document; + this.container = doc.createElement('div'); + this.container.style.zIndex = '10000000'; + + this.tip = new OverlayTip(doc, this.container); + this.rects = []; + + doc.body.appendChild(this.container); + } + + remove() { + this.tip.remove(); + this.rects.forEach(rect => { + rect.remove(); + }); + this.rects.length = 0; + if (this.container.parentNode) { + this.container.parentNode.removeChild(this.container); + } + } + + inspect(nodes: Array, name?: ?string) { + // We can't get the size of text nodes or comment nodes. React as of v15 + // heavily uses comment nodes to delimit text. + const elements = nodes.filter(node => node.nodeType === Node.ELEMENT_NODE); + + while (this.rects.length > elements.length) { + const rect = this.rects.pop(); + rect.remove(); + } + if (elements.length === 0) { + return; + } + + while (this.rects.length < elements.length) { + this.rects.push(new OverlayRect(this.window.document, this.container)); + } + + const outerBox = { + top: Number.POSITIVE_INFINITY, + right: Number.NEGATIVE_INFINITY, + bottom: Number.NEGATIVE_INFINITY, + left: Number.POSITIVE_INFINITY, + }; + elements.forEach((element, index) => { + const box = getNestedBoundingClientRect(element, this.window); + const dims = getElementDimensions(element); + + outerBox.top = Math.min(outerBox.top, box.top - dims.marginTop); + outerBox.right = Math.max( + outerBox.right, + box.left + box.width + dims.marginRight + ); + outerBox.bottom = Math.max( + outerBox.bottom, + box.top + box.height + dims.marginBottom + ); + outerBox.left = Math.min(outerBox.left, box.left - dims.marginLeft); + + const rect = this.rects[index]; + rect.update(box, dims); + }); if (!name) { - name = node.nodeName.toLowerCase(); - const ownerName = getOwnerDisplayName(node); + name = elements[0].nodeName.toLowerCase(); + const ownerName = getOwnerDisplayName(elements[0]); if (ownerName) { name += ' (in ' + ownerName + ')'; } } - this.nameSpan.textContent = name; - this.dimSpan.textContent = - Math.round(box.width) + 'px × ' + Math.round(box.height) + 'px'; - - const tipPos = findTipPos( - { - top: box.top - dims.marginTop, - left: box.left - dims.marginLeft, - height: box.height + dims.marginTop + dims.marginBottom, - width: box.width + dims.marginLeft + dims.marginRight, - }, + this.tip.updateText( + name, + outerBox.right - outerBox.left, + outerBox.bottom - outerBox.top + ); + const tipBounds = getNestedBoundingClientRect( + this.tipBoundsWindow.document.documentElement, this.window ); - assign(this.tip.style, tipPos); + this.tip.updatePosition( + { + top: outerBox.top, + left: outerBox.left, + height: outerBox.bottom - outerBox.top, + width: outerBox.right - outerBox.left, + }, + { + top: tipBounds.top + this.tipBoundsWindow.scrollY, + left: tipBounds.left + this.tipBoundsWindow.scrollX, + height: this.tipBoundsWindow.innerHeight, + width: this.tipBoundsWindow.innerWidth, + } + ); } } @@ -185,35 +295,41 @@ function getFiber(node) { return null; } -function findTipPos(dims, win) { - const tipHeight = 20; +function findTipPos(dims, bounds, tipSize) { + const tipHeight = Math.max(tipSize.height, 20); + const tipWidth = Math.max(tipSize.width, 60); const margin = 5; + let top; - if (dims.top + dims.height + tipHeight <= win.innerHeight) { - if (dims.top + dims.height < 0) { - top = margin; + if (dims.top + dims.height + tipHeight <= bounds.top + bounds.height) { + if (dims.top + dims.height < bounds.top + 0) { + top = bounds.top + margin; } else { top = dims.top + dims.height + margin; } - } else if (dims.top - tipHeight <= win.innerHeight) { - if (dims.top - tipHeight - margin < margin) { - top = margin; + } else if (dims.top - tipHeight <= bounds.top + bounds.height) { + if (dims.top - tipHeight - margin < bounds.top + margin) { + top = bounds.top + margin; } else { top = dims.top - tipHeight - margin; } } else { - top = win.innerHeight - tipHeight - margin; + top = bounds.top + bounds.height - tipHeight - margin; + } + + let left = dims.left + margin; + if (dims.left < bounds.left) { + left = bounds.left + margin; + } + if (dims.left + tipWidth > bounds.left + bounds.width) { + left = bounds.left + bounds.width - tipWidth - margin; } top += 'px'; - - if (dims.left < 0) { - return { top, left: margin }; - } - if (dims.left + 200 > win.innerWidth) { - return { top, right: margin }; - } - return { top, left: dims.left + margin + 'px' }; + left += 'px'; + return { + style: { top, left }, + }; } export function getElementDimensions(domElement: Element) {