From 34928e07d3d55d5b9074c219201d21645066a42e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 5 Nov 2021 16:27:47 -0400 Subject: [PATCH] Timeline: Improved snapshot view Resize snapshots canvas area more intelligently, based on native height. Resize hover tooltips better, based on aspect ratio and viewport dimensions. Show scrubber bar like Chrome does. --- .../react-devtools-timeline/src/CanvasPage.js | 2 + .../src/EventTooltip.js | 34 ++++++++++++-- .../react-devtools-timeline/src/constants.js | 2 + .../src/content-views/SnapshotsView.js | 46 +++++++++++++++++-- .../src/content-views/constants.js | 10 +++- .../__tests__/preprocessData-test.internal.js | 4 ++ .../src/import-worker/preprocessData.js | 20 +++++++- packages/react-devtools-timeline/src/types.js | 1 + .../src/view-base/View.js | 4 +- 9 files changed, 113 insertions(+), 10 deletions(-) diff --git a/packages/react-devtools-timeline/src/CanvasPage.js b/packages/react-devtools-timeline/src/CanvasPage.js index ab5c828280c0a..4084fa4eeb860 100644 --- a/packages/react-devtools-timeline/src/CanvasPage.js +++ b/packages/react-devtools-timeline/src/CanvasPage.js @@ -753,8 +753,10 @@ function AutoSizedCanvas({ )} diff --git a/packages/react-devtools-timeline/src/EventTooltip.js b/packages/react-devtools-timeline/src/EventTooltip.js index d3ca5d5625f13..1cb4b3d8bbf2e 100644 --- a/packages/react-devtools-timeline/src/EventTooltip.js +++ b/packages/react-devtools-timeline/src/EventTooltip.js @@ -34,8 +34,10 @@ const MAX_TOOLTIP_TEXT_LENGTH = 60; type Props = {| canvasRef: {|current: HTMLCanvasElement | null|}, data: ReactProfilerData, + height: number, hoveredEvent: ReactHoverContextInfo | null, origin: Point, + width: number, |}; function getSchedulingEventLabel(event: SchedulingEvent): string | null { @@ -71,8 +73,10 @@ function getReactMeasureLabel(type): string | null { export default function EventTooltip({ canvasRef, data, + height, hoveredEvent, origin, + width, }: Props) { const ref = useSmartTooltip({ canvasRef, @@ -111,7 +115,9 @@ export default function EventTooltip({ ); } else if (snapshot !== null) { - content = ; + content = ( + + ); } else if (suspenseEvent !== null) { content = ; } else if (measure !== null) { @@ -333,12 +339,34 @@ const TooltipSchedulingEvent = ({ ); }; -const TooltipSnapshot = ({snapshot}: {|snapshot: Snapshot|}) => { +const TooltipSnapshot = ({ + height, + snapshot, + width, +}: {| + height: number, + snapshot: Snapshot, + width: number, +|}) => { + const aspectRatio = snapshot.width / snapshot.height; + + // Zoomed in view should not be any bigger than the DevTools viewport. + let safeWidth = snapshot.width; + let safeHeight = snapshot.height; + if (safeWidth > width) { + safeWidth = width; + safeHeight = safeWidth / aspectRatio; + } + if (safeHeight > height) { + safeHeight = height; + safeWidth = safeHeight * aspectRatio; + } + return ( ); }; diff --git a/packages/react-devtools-timeline/src/constants.js b/packages/react-devtools-timeline/src/constants.js index e29b8901d81fd..e9b18ab65716e 100644 --- a/packages/react-devtools-timeline/src/constants.js +++ b/packages/react-devtools-timeline/src/constants.js @@ -16,3 +16,5 @@ export const REACT_TOTAL_NUM_LANES = 31; // Increment this number any time a backwards breaking change is made to the profiler metadata. export const SCHEDULING_PROFILER_VERSION = 1; + +export const SNAPSHOT_MAX_HEIGHT = 60; diff --git a/packages/react-devtools-timeline/src/content-views/SnapshotsView.js b/packages/react-devtools-timeline/src/content-views/SnapshotsView.js index fbaa7c5d83570..b027acae37693 100644 --- a/packages/react-devtools-timeline/src/content-views/SnapshotsView.js +++ b/packages/react-devtools-timeline/src/content-views/SnapshotsView.js @@ -24,11 +24,12 @@ import { rectEqualToRect, View, } from '../view-base'; -import {BORDER_SIZE, COLORS, SNAPSHOT_HEIGHT} from './constants'; +import {BORDER_SIZE, COLORS, SNAPSHOT_SCRUBBER_SIZE} from './constants'; type OnHover = (node: Snapshot | null) => void; export class SnapshotsView extends View { + _hoverLocation: Point | null = null; _intrinsicSize: Size; _profilerData: ReactProfilerData; @@ -39,7 +40,7 @@ export class SnapshotsView extends View { this._intrinsicSize = { width: profilerData.duration, - height: SNAPSHOT_HEIGHT, + height: profilerData.snapshotHeight, }; this._profilerData = profilerData; } @@ -49,6 +50,7 @@ export class SnapshotsView extends View { } draw(context: CanvasRenderingContext2D) { + const snapshotHeight = this._profilerData.snapshotHeight; const {visibleArea} = this; context.fillStyle = COLORS.BACKGROUND; @@ -72,8 +74,8 @@ export class SnapshotsView extends View { break; } - const scaledHeight = SNAPSHOT_HEIGHT; - const scaledWidth = (snapshot.width * SNAPSHOT_HEIGHT) / snapshot.height; + const scaledHeight = snapshotHeight; + const scaledWidth = (snapshot.width * snapshotHeight) / snapshot.height; const imageRect: Rect = { origin: { @@ -96,6 +98,28 @@ export class SnapshotsView extends View { x += scaledWidth + BORDER_SIZE; } + + const hoverLocation = this._hoverLocation; + if (hoverLocation !== null) { + const scrubberWidth = SNAPSHOT_SCRUBBER_SIZE + BORDER_SIZE * 2; + const scrubberOffset = scrubberWidth / 2; + + context.fillStyle = COLORS.SCRUBBER_BORDER; + context.fillRect( + hoverLocation.x - scrubberOffset, + visibleArea.origin.y, + scrubberWidth, + visibleArea.size.height, + ); + + context.fillStyle = COLORS.SCRUBBER_BACKGROUND; + context.fillRect( + hoverLocation.x - scrubberOffset + BORDER_SIZE, + visibleArea.origin.y, + SNAPSHOT_SCRUBBER_SIZE, + visibleArea.size.height, + ); + } } handleInteraction(interaction: Interaction, viewRefs: ViewRefs) { @@ -208,15 +232,29 @@ export class SnapshotsView extends View { } if (!rectContainsPoint(location, visibleArea)) { + if (this._hoverLocation !== null) { + this._hoverLocation = null; + + this.setNeedsDisplay(); + } + onHover(null); return; } const snapshot = this._findClosestSnapshot(location.x); if (snapshot !== null) { + this._hoverLocation = location; + onHover(snapshot); } else { + this._hoverLocation = null; + onHover(null); } + + // Any time the mouse moves within the boundaries of this view, we need to re-render. + // This is because we draw a scrubbing bar that shows the location corresponding to the current tooltip. + this.setNeedsDisplay(); } } diff --git a/packages/react-devtools-timeline/src/content-views/constants.js b/packages/react-devtools-timeline/src/content-views/constants.js index 216234508f5e1..f2920ac19b516 100644 --- a/packages/react-devtools-timeline/src/content-views/constants.js +++ b/packages/react-devtools-timeline/src/content-views/constants.js @@ -24,7 +24,7 @@ export const REACT_MEASURE_HEIGHT = 14; export const BORDER_SIZE = 1 / DPR; export const FLAMECHART_FRAME_HEIGHT = 14; export const TEXT_PADDING = 3; -export const SNAPSHOT_HEIGHT = 35; +export const SNAPSHOT_SCRUBBER_SIZE = 3; export const INTERVAL_TIMES = [ 1, @@ -89,6 +89,8 @@ export let COLORS = { REACT_THROWN_ERROR_HOVER: '', REACT_WORK_BORDER: '', SCROLL_CARET: '', + SCRUBBER_BACKGROUND: '', + SCRUBBER_BORDER: '', TEXT_COLOR: '', TEXT_DIM_COLOR: '', TIME_MARKER_LABEL: '', @@ -230,6 +232,12 @@ export function updateColorsToMatchTheme(element: Element): boolean { '--color-timeline-react-work-border', ), SCROLL_CARET: computedStyle.getPropertyValue('--color-scroll-caret'), + SCRUBBER_BACKGROUND: computedStyle.getPropertyValue( + '--color-timeline-react-suspense-rejected', + ), + SCRUBBER_BORDER: computedStyle.getPropertyValue( + '--color-timeline-text-color', + ), TEXT_COLOR: computedStyle.getPropertyValue('--color-timeline-text-color'), TEXT_DIM_COLOR: computedStyle.getPropertyValue( '--color-timeline-text-dim-color', diff --git a/packages/react-devtools-timeline/src/import-worker/__tests__/preprocessData-test.internal.js b/packages/react-devtools-timeline/src/import-worker/__tests__/preprocessData-test.internal.js index 758afc1e15f2c..f902ddc57e14e 100644 --- a/packages/react-devtools-timeline/src/import-worker/__tests__/preprocessData-test.internal.js +++ b/packages/react-devtools-timeline/src/import-worker/__tests__/preprocessData-test.internal.js @@ -354,6 +354,7 @@ describe('preprocessData', () => { "otherUserTimingMarks": Array [], "reactVersion": "17.0.3", "schedulingEvents": Array [], + "snapshotHeight": 0, "snapshots": Array [], "startTime": 1, "suspenseEvents": Array [], @@ -572,6 +573,7 @@ describe('preprocessData', () => { "warning": null, }, ], + "snapshotHeight": 0, "snapshots": Array [], "startTime": 1, "suspenseEvents": Array [], @@ -765,6 +767,7 @@ describe('preprocessData', () => { "warning": null, }, ], + "snapshotHeight": 0, "snapshots": Array [], "startTime": 4, "suspenseEvents": Array [], @@ -1129,6 +1132,7 @@ describe('preprocessData', () => { "warning": null, }, ], + "snapshotHeight": 0, "snapshots": Array [], "startTime": 4, "suspenseEvents": Array [], diff --git a/packages/react-devtools-timeline/src/import-worker/preprocessData.js b/packages/react-devtools-timeline/src/import-worker/preprocessData.js index caf27e707563b..98e802793f6a8 100644 --- a/packages/react-devtools-timeline/src/import-worker/preprocessData.js +++ b/packages/react-devtools-timeline/src/import-worker/preprocessData.js @@ -29,7 +29,11 @@ import type { SchedulingEvent, SuspenseEvent, } from '../types'; -import {REACT_TOTAL_NUM_LANES, SCHEDULING_PROFILER_VERSION} from '../constants'; +import { + REACT_TOTAL_NUM_LANES, + SCHEDULING_PROFILER_VERSION, + SNAPSHOT_MAX_HEIGHT, +} from '../constants'; import InvalidProfileError from './InvalidProfileError'; import {getBatchRange} from '../utils/getBatchRange'; import ErrorStackParser from 'error-stack-parser'; @@ -1066,6 +1070,7 @@ export default async function preprocessData( reactVersion: null, schedulingEvents: [], snapshots: [], + snapshotHeight: 0, startTime: 0, suspenseEvents: [], thrownErrors: [], @@ -1189,5 +1194,18 @@ export default async function preprocessData( // Since processing is done in a worker, async work must complete before data is serialized and returned. await Promise.all(state.asyncProcessingPromises); + // Now that all images have been loaded, let's figure out the display size we're going to use for our thumbnails: + // both the ones rendered to the canvas and the ones shown on hover. + if (profilerData.snapshots.length > 0) { + // NOTE We assume a static window size here, which is not necessarily true but should be for most cases. + // Regardless, Chrome also sets a single size/ratio and stick with it- so we'll do the same. + const snapshot = profilerData.snapshots[0]; + + profilerData.snapshotHeight = Math.min( + snapshot.height, + SNAPSHOT_MAX_HEIGHT, + ); + } + return profilerData; } diff --git a/packages/react-devtools-timeline/src/types.js b/packages/react-devtools-timeline/src/types.js index cf504229b01c5..5a532495187ee 100644 --- a/packages/react-devtools-timeline/src/types.js +++ b/packages/react-devtools-timeline/src/types.js @@ -202,6 +202,7 @@ export type ReactProfilerData = {| reactVersion: string | null, schedulingEvents: SchedulingEvent[], snapshots: Snapshot[], + snapshotHeight: number, startTime: number, suspenseEvents: SuspenseEvent[], thrownErrors: ThrownError[], diff --git a/packages/react-devtools-timeline/src/view-base/View.js b/packages/react-devtools-timeline/src/view-base/View.js index 0edac1120862b..3e2a5a06f1cd5 100644 --- a/packages/react-devtools-timeline/src/view-base/View.js +++ b/packages/react-devtools-timeline/src/view-base/View.js @@ -197,7 +197,9 @@ export class View { !sizeIsEmpty(this.visibleArea.size) ) { this.layoutSubviews(); - if (this._needsDisplay) this._needsDisplay = false; + if (this._needsDisplay) { + this._needsDisplay = false; + } if (this._subviewsNeedDisplay) this._subviewsNeedDisplay = false; // Clip anything drawn by the view to prevent it from overflowing its visible area.