From 39223239bbcea68f801d2d3607c5326099e22e91 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 8 Apr 2019 14:26:16 +0100 Subject: [PATCH 1/4] Select DOM nodes on hover --- src/backend/agent.js | 19 +++++++++++++++---- src/backend/views/Highlighter.js | 7 +++++-- src/devtools/views/Components/Element.js | 18 ++++++++++++++++++ .../views/Components/SelectedElement.js | 2 ++ src/devtools/views/Components/Tree.js | 8 +++++++- 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 3bde89b953..5e2f332b85 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -63,6 +63,10 @@ export default class Agent extends EventEmitter { this._bridge = bridge; bridge.addListener('captureScreenshot', this.captureScreenshot); + bridge.addListener( + 'clearHighlightedElementInDOM', + this.clearHighlightedElementInDOM + ); bridge.addListener('exportProfilingSummary', this.exportProfilingSummary); bridge.addListener('getCommitDetails', this.getCommitDetails); bridge.addListener('getFiberCommits', this.getFiberCommits); @@ -216,14 +220,22 @@ export default class Agent extends EventEmitter { } }; + clearHighlightedElementInDOM = () => { + hideOverlay(); + }; + highlightElementInDOM = ({ displayName, id, + isSticky, rendererID, + scrollIntoView, }: { displayName: string, id: number, + isSticky: boolean, rendererID: number, + scrollIntoView: boolean, }) => { const renderer = this._rendererInterfaces[rendererID]; if (renderer == null) { @@ -236,13 +248,12 @@ export default class Agent extends EventEmitter { } if (node != null) { - if (typeof node.scrollIntoView === 'function') { + 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); + showOverlay(((node: any): HTMLElement), displayName, isSticky); } else { hideOverlay(); } @@ -466,6 +477,6 @@ export default class Agent extends EventEmitter { // Don't pass the name explicitly. // It will be inferred from DOM tag and Fiber owner. - showOverlay(target); + showOverlay(target, null, true); }; } diff --git a/src/backend/views/Highlighter.js b/src/backend/views/Highlighter.js index fc98d4a134..8b1c17676a 100644 --- a/src/backend/views/Highlighter.js +++ b/src/backend/views/Highlighter.js @@ -18,7 +18,8 @@ export function hideOverlay() { export function showOverlay( element: HTMLElement | null, - componentName: string = '' + componentName: string | null, + isSticky: boolean ) { if (timeoutID !== null) { clearTimeout(timeoutID); @@ -34,5 +35,7 @@ export function showOverlay( overlay.inspect(element, componentName); - timeoutID = setTimeout(hideOverlay, SHOW_DURATION); + if (!isSticky) { + timeoutID = setTimeout(hideOverlay, SHOW_DURATION); + } } diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index 14acf8ae2f..f10284cc91 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -11,6 +11,7 @@ import React, { import { ElementTypeClass, ElementTypeFunction } from 'src/devtools/types'; import { createRegExp } from '../utils'; import { TreeContext } from './TreeContext'; +import { BridgeContext, StoreContext } from '../context'; import type { Element } from './types'; @@ -31,6 +32,9 @@ export default function ElementView({ index, style, data }: Props) { selectedElementID, selectElementByID, } = useContext(TreeContext); + const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); + const element = getElementAtIndex(index); const id = element === null ? null : element.id; @@ -82,6 +86,19 @@ export default function ElementView({ index, style, data }: Props) { [id, selectElementByID] ); + const rendererID = store.getRendererIDForElement(element.id) || null; + const handleMouseEnter = useCallback(() => { + if (rendererID !== null) { + bridge.send('highlightElementInDOM', { + displayName: element.displayName, + id: element.id, + rendererID, + scrollIntoView: false, + isSticky: true, + }); + } + }, [bridge, element, rendererID]); + // Handle elements that are removed from the tree while an async render is in progress. if (element == null) { console.warn(` Could not find element at index ${index}`); @@ -100,6 +117,7 @@ export default function ElementView({ index, style, data }: Props) { return (
| null>(null); const treeRef = useRef(null); @@ -112,13 +114,17 @@ export default function Tree(props: Props) { [baseDepth, numElements, getElementAtIndex, lastScrolledIDRef] ); + const handleMouseLeave = useCallback(() => { + bridge.send('clearHighlightedElementInDOM'); + }, [bridge]); + return (
{ownerStack.length > 0 ? : }
-
+
{({ height, width }) => ( Date: Mon, 8 Apr 2019 14:39:00 +0100 Subject: [PATCH 2/4] Add a comment --- src/devtools/views/Components/Element.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index f10284cc91..f188770ce9 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -87,6 +87,8 @@ export default function ElementView({ index, style, data }: Props) { ); const rendererID = store.getRendererIDForElement(element.id) || null; + // Individual elements don't have a corresponding leave handler. + // Instead, it's implemented on the tree level. const handleMouseEnter = useCallback(() => { if (rendererID !== null) { bridge.send('highlightElementInDOM', { From c6ee445ea974e133ea9974aec37c65d387fdcfd7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 8 Apr 2019 18:00:48 +0100 Subject: [PATCH 3/4] Address review --- src/backend/agent.js | 8 ++++---- src/backend/views/Highlighter.js | 4 ++-- src/devtools/views/Components/Element.js | 4 ++-- src/devtools/views/Components/SelectedElement.js | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 5e2f332b85..c96b05b0aa 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -226,14 +226,14 @@ export default class Agent extends EventEmitter { highlightElementInDOM = ({ displayName, + hideAfterTimeout, id, - isSticky, rendererID, scrollIntoView, }: { displayName: string, + hideAfterTimeout: boolean, id: number, - isSticky: boolean, rendererID: number, scrollIntoView: boolean, }) => { @@ -253,7 +253,7 @@ export default class Agent extends EventEmitter { // We may want to reconsider this; it might be a little disruptive. node.scrollIntoView({ block: 'nearest', inline: 'nearest' }); } - showOverlay(((node: any): HTMLElement), displayName, isSticky); + showOverlay(((node: any): HTMLElement), displayName, hideAfterTimeout); } else { hideOverlay(); } @@ -477,6 +477,6 @@ 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, true); + showOverlay(target, null, false); }; } diff --git a/src/backend/views/Highlighter.js b/src/backend/views/Highlighter.js index 8b1c17676a..c5339f6763 100644 --- a/src/backend/views/Highlighter.js +++ b/src/backend/views/Highlighter.js @@ -19,7 +19,7 @@ export function hideOverlay() { export function showOverlay( element: HTMLElement | null, componentName: string | null, - isSticky: boolean + hideAfterTimeout: boolean ) { if (timeoutID !== null) { clearTimeout(timeoutID); @@ -35,7 +35,7 @@ export function showOverlay( overlay.inspect(element, componentName); - if (!isSticky) { + if (hideAfterTimeout) { timeoutID = setTimeout(hideOverlay, SHOW_DURATION); } } diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index f188770ce9..4ac576c95b 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -86,17 +86,17 @@ export default function ElementView({ index, style, data }: Props) { [id, selectElementByID] ); - const rendererID = store.getRendererIDForElement(element.id) || null; + const rendererID = store.getRendererIDForElement(element.id); // Individual elements don't have a corresponding leave handler. // Instead, it's implemented on the tree level. const handleMouseEnter = useCallback(() => { if (rendererID !== null) { bridge.send('highlightElementInDOM', { displayName: element.displayName, + hideAfterTimeout: false, id: element.id, rendererID, scrollIntoView: false, - isSticky: true, }); } }, [bridge, element, rendererID]); diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js index df07d46dbf..bba8d803da 100644 --- a/src/devtools/views/Components/SelectedElement.js +++ b/src/devtools/views/Components/SelectedElement.js @@ -45,10 +45,10 @@ export default function SelectedElement(_: Props) { if (rendererID !== null) { bridge.send('highlightElementInDOM', { displayName: element.displayName, + hideAfterTimeout: true, id: selectedElementID, rendererID, scrollIntoView: true, - isSticky: false, }); } } @@ -271,7 +271,7 @@ function useInspectedElement(id: number | null): InspectedElement | null { return () => {}; } - const rendererID = store.getRendererIDForElement(id) || null; + const rendererID = store.getRendererIDForElement(id); // Update the $r variable. bridge.send('selectElement', { id, rendererID }); From 52014671bf42df73fd8764de2c18c979cad47ff8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 8 Apr 2019 18:03:48 +0100 Subject: [PATCH 4/4] Make Flow happy --- src/devtools/views/Components/Element.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index 4ac576c95b..421195d5af 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -86,20 +86,20 @@ export default function ElementView({ index, style, data }: Props) { [id, selectElementByID] ); - const rendererID = store.getRendererIDForElement(element.id); + const rendererID = id !== null ? store.getRendererIDForElement(id) : null; // Individual elements don't have a corresponding leave handler. // Instead, it's implemented on the tree level. const handleMouseEnter = useCallback(() => { - if (rendererID !== null) { + if (element !== null && id !== null && rendererID !== null) { bridge.send('highlightElementInDOM', { displayName: element.displayName, hideAfterTimeout: false, - id: element.id, + id, rendererID, scrollIntoView: false, }); } - }, [bridge, element, rendererID]); + }, [bridge, element, id, rendererID]); // Handle elements that are removed from the tree while an async render is in progress. if (element == null) {