From 42251331d8f7a997a018d75370f5216bb7552819 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 2 Aug 2021 14:30:43 -0400 Subject: [PATCH] DevTools: Scheduling profiler (#22006) --- .../src/CanvasPage.js | 31 +++++++++++-------- .../src/content-views/NativeEventsView.js | 6 ++-- .../src/content-views/ReactMeasuresView.js | 5 +-- .../src/content-views/SuspenseEventsView.js | 4 +-- .../src/content-views/UserTimingMarksView.js | 1 - .../src/view-base/ResizableView.js | 27 +++++++++++++--- .../src/view-base/View.js | 4 +-- .../src/view-base/geometry.js | 8 ++++- .../react-devtools-shared/src/constants.js | 24 +++++++++----- .../src/devtools/ContextMenu/ContextMenu.js | 14 ++++++--- .../src/devtools/views/DevTools.js | 5 ++- .../src/devtools/views/portaledContent.js | 10 +++++- .../src/devtools/views/root.css | 5 +++ 13 files changed, 100 insertions(+), 44 deletions(-) diff --git a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js index b35628a705d01..05700d6431d01 100644 --- a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js +++ b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js @@ -230,18 +230,21 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) { schedulingEventsViewRef.current = schedulingEventsView; const schedulingEventsViewWrapper = createViewHelper(schedulingEventsView); - const suspenseEventsView = new SuspenseEventsView( - surface, - defaultFrame, - data, - ); - suspenseEventsViewRef.current = suspenseEventsView; - const suspenseEventsViewWrapper = createViewHelper( - suspenseEventsView, - 'suspense', - true, - true, - ); + let suspenseEventsViewWrapper = null; + if (data.suspenseEvents.length > 0) { + const suspenseEventsView = new SuspenseEventsView( + surface, + defaultFrame, + data, + ); + suspenseEventsViewRef.current = suspenseEventsView; + suspenseEventsViewWrapper = createViewHelper( + suspenseEventsView, + 'suspense', + true, + true, + ); + } const reactMeasuresView = new ReactMeasuresView( surface, @@ -286,7 +289,9 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) { } rootView.addSubview(nativeEventsViewWrapper); rootView.addSubview(schedulingEventsViewWrapper); - rootView.addSubview(suspenseEventsViewWrapper); + if (suspenseEventsViewWrapper !== null) { + rootView.addSubview(suspenseEventsViewWrapper); + } rootView.addSubview(reactMeasuresViewWrapper); rootView.addSubview(flamechartViewWrapper); diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/NativeEventsView.js b/packages/react-devtools-scheduling-profiler/src/content-views/NativeEventsView.js index 98e388c52a815..34424e7b5b81d 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/NativeEventsView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/NativeEventsView.js @@ -10,9 +10,9 @@ import type {NativeEvent, ReactProfilerData} from '../types'; import type { Interaction, + IntrinsicSize, MouseMoveInteraction, Rect, - Size, ViewRefs, } from '../view-base'; @@ -38,7 +38,7 @@ const ROW_WITH_BORDER_HEIGHT = NATIVE_EVENT_HEIGHT + BORDER_SIZE; export class NativeEventsView extends View { _depthToNativeEvent: Map; _hoveredEvent: NativeEvent | null = null; - _intrinsicSize: Size; + _intrinsicSize: IntrinsicSize; _maxDepth: number = 0; _profilerData: ReactProfilerData; @@ -73,6 +73,7 @@ export class NativeEventsView extends View { this._intrinsicSize = { width: duration, height: (this._maxDepth + 1) * ROW_WITH_BORDER_HEIGHT, + hideScrollBarIfLessThanHeight: ROW_WITH_BORDER_HEIGHT, }; } @@ -239,7 +240,6 @@ export class NativeEventsView extends View { hoverTimestamp <= timestamp + duration ) { viewRefs.hoveredView = this; - onHover(nativeEvent); return; } diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/ReactMeasuresView.js b/packages/react-devtools-scheduling-profiler/src/content-views/ReactMeasuresView.js index 96790b2a282e5..f8e4cb99b94c1 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/ReactMeasuresView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/ReactMeasuresView.js @@ -10,9 +10,9 @@ import type {ReactLane, ReactMeasure, ReactProfilerData} from '../types'; import type { Interaction, + IntrinsicSize, MouseMoveInteraction, Rect, - SizeWithMaxHeight, ViewRefs, } from '../view-base'; @@ -45,7 +45,7 @@ function getMeasuresForLane( export class ReactMeasuresView extends View { _profilerData: ReactProfilerData; - _intrinsicSize: SizeWithMaxHeight; + _intrinsicSize: IntrinsicSize; _lanesToRender: ReactLane[]; _laneToMeasures: Map; @@ -78,6 +78,7 @@ export class ReactMeasuresView extends View { this._intrinsicSize = { width: this._profilerData.duration, height: this._lanesToRender.length * REACT_LANE_HEIGHT, + hideScrollBarIfLessThanHeight: REACT_LANE_HEIGHT, maxInitialHeight: MAX_ROWS_TO_SHOW_INITIALLY * REACT_LANE_HEIGHT, }; } diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js b/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js index 3d85ffd4e2dc0..5b0d0f2d51c38 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js @@ -10,9 +10,9 @@ import type {SuspenseEvent, ReactProfilerData} from '../types'; import type { Interaction, + IntrinsicSize, MouseMoveInteraction, Rect, - SizeWithMaxHeight, ViewRefs, } from '../view-base'; @@ -45,7 +45,7 @@ const MAX_ROWS_TO_SHOW_INITIALLY = 3; export class SuspenseEventsView extends View { _depthToSuspenseEvent: Map; _hoveredEvent: SuspenseEvent | null = null; - _intrinsicSize: SizeWithMaxHeight; + _intrinsicSize: IntrinsicSize; _maxDepth: number = 0; _profilerData: ReactProfilerData; diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js b/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js index 2249d3841ad7a..36c24091ac0ad 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js @@ -224,7 +224,6 @@ export class UserTimingMarksView extends View { timestamp - timestampAllowance <= hoverTimestamp && hoverTimestamp <= timestamp + timestampAllowance ) { - this.currentCursor = 'context-menu'; viewRefs.hoveredView = this; onHover(mark); return; diff --git a/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js b/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js index 54a3b05c1a9cf..80527a193cb59 100644 --- a/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js +++ b/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js @@ -255,12 +255,21 @@ export class ResizableView extends View { } desiredSize() { - const resizeBarDesiredSize = this._resizeBar.desiredSize(); + const subviewDesiredSize = this._subview.desiredSize(); - return { - width: this.frame.size.width, - height: this._layoutState.barOffsetY + resizeBarDesiredSize.height, - }; + if (this._shouldRenderResizeBar()) { + const resizeBarDesiredSize = this._resizeBar.desiredSize(); + + return { + width: this.frame.size.width, + height: this._layoutState.barOffsetY + resizeBarDesiredSize.height, + }; + } else { + return { + width: this.frame.size.width, + height: subviewDesiredSize.height, + }; + } } layoutSubviews() { @@ -270,6 +279,14 @@ export class ResizableView extends View { super.layoutSubviews(); } + _shouldRenderResizeBar() { + const subviewDesiredSize = this._subview.desiredSize(); + return subviewDesiredSize.hideScrollBarIfLessThanHeight != null + ? subviewDesiredSize.height > + subviewDesiredSize.hideScrollBarIfLessThanHeight + : true; + } + _updateLayoutStateAndResizeBar(barOffsetY: number) { if (barOffsetY <= RESIZE_BAR_WITH_LABEL_HEIGHT - RESIZE_BAR_HEIGHT) { barOffsetY = 0; diff --git a/packages/react-devtools-scheduling-profiler/src/view-base/View.js b/packages/react-devtools-scheduling-profiler/src/view-base/View.js index a43a733fd4762..206721cdd39c3 100644 --- a/packages/react-devtools-scheduling-profiler/src/view-base/View.js +++ b/packages/react-devtools-scheduling-profiler/src/view-base/View.js @@ -8,7 +8,7 @@ */ import type {Interaction} from './useCanvasInteraction'; -import type {Rect, Size, SizeWithMaxHeight} from './geometry'; +import type {IntrinsicSize, Rect, Size} from './geometry'; import type {Layouter} from './layouter'; import type {ViewRefs} from './Surface'; @@ -140,7 +140,7 @@ export class View { * * Can be overridden by subclasses. */ - desiredSize(): Size | SizeWithMaxHeight { + desiredSize(): Size | IntrinsicSize { if (this._needsDisplay) { this.layoutSubviews(); } diff --git a/packages/react-devtools-scheduling-profiler/src/view-base/geometry.js b/packages/react-devtools-scheduling-profiler/src/view-base/geometry.js index 49c0d3981e721..054d8c3e75feb 100644 --- a/packages/react-devtools-scheduling-profiler/src/view-base/geometry.js +++ b/packages/react-devtools-scheduling-profiler/src/view-base/geometry.js @@ -9,8 +9,14 @@ export type Point = $ReadOnly<{|x: number, y: number|}>; export type Size = $ReadOnly<{|width: number, height: number|}>; -export type SizeWithMaxHeight = {| +export type IntrinsicSize = {| ...Size, + + // If content is this height or less, hide the scrollbar entirely, + // so that it doesn't take up vertical space unnecessarily (e.g. for a single row of content). + hideScrollBarIfLessThanHeight?: number, + + // The initial height should be the height of the content, or this, whichever is less. maxInitialHeight?: number, |}; export type Rect = $ReadOnly<{|origin: Point, size: Size|}>; diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index 1e062b23a641c..a4b742a3fb64a 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -165,8 +165,6 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any} = { '--color-scheduling-profiler-react-suspense-unresolved-hover': '#93959a', '--color-scheduling-profiler-text-color': '#000000', '--color-scheduling-profiler-react-work-border': '#ffffff', - '--color-scroll-thumb': '#c2c2c2', - '--color-scroll-track': '#fafafa', '--color-search-match': 'yellow', '--color-search-match-current': '#f7923b', '--color-selected-tree-highlight-active': 'rgba(0, 136, 250, 0.1)', @@ -180,12 +178,18 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any} = { '--color-toggle-background-on': '#0088fa', '--color-toggle-background-off': '#cfd1d5', '--color-toggle-text': '#ffffff', - '--color-tooltip-background': 'rgba(0, 0, 0, 0.9)', - '--color-tooltip-text': '#ffffff', '--color-warning-background': '#fb3655', '--color-warning-background-hover': '#f82042', '--color-warning-text-color': '#ffffff', '--color-warning-text-color-inverted': '#fd4d69', + + // The styles below should be kept in sync with 'root.css' + // They are repeated there because they're used by e.g. tooltips or context menus + // which get rendered outside of the DOM subtree (where normal theme/styles are written). + '--color-scroll-thumb': '#c2c2c2', + '--color-scroll-track': '#fafafa', + '--color-tooltip-background': 'rgba(0, 0, 0, 0.9)', + '--color-tooltip-text': '#ffffff', }, dark: { '--color-attribute-name': '#9d87d2', @@ -291,8 +295,6 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any} = { '--color-scheduling-profiler-react-suspense-unresolved-hover': '#93959a', '--color-scheduling-profiler-text-color': '#000000', '--color-scheduling-profiler-react-work-border': '#ffffff', - '--color-scroll-thumb': '#afb3b9', - '--color-scroll-track': '#313640', '--color-search-match': 'yellow', '--color-search-match-current': '#f7923b', '--color-selected-tree-highlight-active': 'rgba(23, 143, 185, 0.15)', @@ -307,12 +309,18 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any} = { '--color-toggle-background-on': '#178fb9', '--color-toggle-background-off': '#777d88', '--color-toggle-text': '#ffffff', - '--color-tooltip-background': 'rgba(255, 255, 255, 0.95)', - '--color-tooltip-text': '#000000', '--color-warning-background': '#ee1638', '--color-warning-background-hover': '#da1030', '--color-warning-text-color': '#ffffff', '--color-warning-text-color-inverted': '#ee1638', + + // The styles below should be kept in sync with 'root.css' + // They are repeated there because they're used by e.g. tooltips or context menus + // which get rendered outside of the DOM subtree (where normal theme/styles are written). + '--color-scroll-thumb': '#afb3b9', + '--color-scroll-track': '#313640', + '--color-tooltip-background': 'rgba(255, 255, 255, 0.95)', + '--color-tooltip-text': '#000000', }, compact: { '--font-size-monospace-small': '9px', diff --git a/packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.js b/packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.js index 466c4fdad6aa3..90be1883612ab 100644 --- a/packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.js +++ b/packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.js @@ -68,11 +68,15 @@ export default function ContextMenu({children, id}: Props) { const element = bodyAccessorRef.current; if (element !== null) { const ownerDocument = element.ownerDocument; - containerRef.current = ownerDocument.createElement('div'); - ownerDocument.body.appendChild(containerRef.current); - return () => { - ownerDocument.body.removeChild(containerRef.current); - }; + containerRef.current = ownerDocument.querySelector( + '[data-react-devtools-portal-root]', + ); + + if (containerRef.current == null) { + console.warn( + 'DevTools tooltip root node not found; context menus will be disabled.', + ); + } } }, []); diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.js b/packages/react-devtools-shared/src/devtools/views/DevTools.js index 190fa7917a572..8f1ccc5540376 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.js +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.js @@ -222,7 +222,10 @@ export default function DevTools({ -
+
{showTabBar && (
diff --git a/packages/react-devtools-shared/src/devtools/views/portaledContent.js b/packages/react-devtools-shared/src/devtools/views/portaledContent.js index b95fa6e22c098..1a59ff59d212d 100644 --- a/packages/react-devtools-shared/src/devtools/views/portaledContent.js +++ b/packages/react-devtools-shared/src/devtools/views/portaledContent.js @@ -32,7 +32,15 @@ export default function portaledContent( // The ThemeProvider works by writing DOM style variables to an HTMLDivElement. // Because Portals render in a different DOM subtree, these variables don't propagate. // So in this case, we need to re-wrap portaled content in a second ThemeProvider. - children = {children}; + children = ( + +
+ {children} +
+
+ ); } return portalContainer != null diff --git a/packages/react-devtools-shared/src/devtools/views/root.css b/packages/react-devtools-shared/src/devtools/views/root.css index c6c7ffd586e7b..9bb749ff23703 100644 --- a/packages/react-devtools-shared/src/devtools/views/root.css +++ b/packages/react-devtools-shared/src/devtools/views/root.css @@ -1,4 +1,9 @@ :root { + /** + * The light and dark theme styles below should be kept in sync with 'react-devtools-shared/src/constants' + * They are repeated here because they're used by e.g. tooltips or context menus + * which get rendered outside of the DOM subtree (where normal theme/styles are written). + */ /* Light theme */ --light-color-scroll-thumb: #c2c2c2;