Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeline: Improved snapshot view #22706

Merged
merged 1 commit into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/react-devtools-timeline/src/CanvasPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -753,8 +753,10 @@ function AutoSizedCanvas({
<EventTooltip
canvasRef={canvasRef}
data={data}
height={height}
hoveredEvent={hoveredEvent}
origin={mouseLocation}
width={width}
/>
)}
</Fragment>
Expand Down
34 changes: 31 additions & 3 deletions packages/react-devtools-timeline/src/EventTooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -71,8 +73,10 @@ function getReactMeasureLabel(type): string | null {
export default function EventTooltip({
canvasRef,
data,
height,
hoveredEvent,
origin,
width,
}: Props) {
const ref = useSmartTooltip({
canvasRef,
Expand Down Expand Up @@ -111,7 +115,9 @@ export default function EventTooltip({
<TooltipSchedulingEvent data={data} schedulingEvent={schedulingEvent} />
);
} else if (snapshot !== null) {
content = <TooltipSnapshot snapshot={snapshot} />;
content = (
<TooltipSnapshot height={height} snapshot={snapshot} width={width} />
);
} else if (suspenseEvent !== null) {
content = <TooltipSuspenseEvent suspenseEvent={suspenseEvent} />;
} else if (measure !== null) {
Expand Down Expand Up @@ -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 (
<img
className={styles.Image}
src={snapshot.imageSource}
style={{width: snapshot.width / 2, height: snapshot.height / 2}}
style={{height: safeHeight, width: safeWidth}}
/>
);
};
Expand Down
2 changes: 2 additions & 0 deletions packages/react-devtools-timeline/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -39,7 +40,7 @@ export class SnapshotsView extends View {

this._intrinsicSize = {
width: profilerData.duration,
height: SNAPSHOT_HEIGHT,
height: profilerData.snapshotHeight,
};
this._profilerData = profilerData;
}
Expand All @@ -49,6 +50,7 @@ export class SnapshotsView extends View {
}

draw(context: CanvasRenderingContext2D) {
const snapshotHeight = this._profilerData.snapshotHeight;
const {visibleArea} = this;

context.fillStyle = COLORS.BACKGROUND;
Expand All @@ -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: {
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
}
}
10 changes: 9 additions & 1 deletion packages/react-devtools-timeline/src/content-views/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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: '',
Expand Down Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ describe('preprocessData', () => {
"otherUserTimingMarks": Array [],
"reactVersion": "17.0.3",
"schedulingEvents": Array [],
"snapshotHeight": 0,
"snapshots": Array [],
"startTime": 1,
"suspenseEvents": Array [],
Expand Down Expand Up @@ -572,6 +573,7 @@ describe('preprocessData', () => {
"warning": null,
},
],
"snapshotHeight": 0,
"snapshots": Array [],
"startTime": 1,
"suspenseEvents": Array [],
Expand Down Expand Up @@ -765,6 +767,7 @@ describe('preprocessData', () => {
"warning": null,
},
],
"snapshotHeight": 0,
"snapshots": Array [],
"startTime": 4,
"suspenseEvents": Array [],
Expand Down Expand Up @@ -1129,6 +1132,7 @@ describe('preprocessData', () => {
"warning": null,
},
],
"snapshotHeight": 0,
"snapshots": Array [],
"startTime": 4,
"suspenseEvents": Array [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1066,6 +1070,7 @@ export default async function preprocessData(
reactVersion: null,
schedulingEvents: [],
snapshots: [],
snapshotHeight: 0,
startTime: 0,
suspenseEvents: [],
thrownErrors: [],
Expand Down Expand Up @@ -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;
}
1 change: 1 addition & 0 deletions packages/react-devtools-timeline/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ export type ReactProfilerData = {|
reactVersion: string | null,
schedulingEvents: SchedulingEvent[],
snapshots: Snapshot[],
snapshotHeight: number,
startTime: number,
suspenseEvents: SuspenseEvent[],
thrownErrors: ThrownError[],
Expand Down
4 changes: 3 additions & 1 deletion packages/react-devtools-timeline/src/view-base/View.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down