Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Scheduling profiler: Fix tooltip wheel event regression (facebook#22130)
Browse files Browse the repository at this point in the history
Panning horizontally via mouse wheel used to allow you to scrub over snapshot images. This was accidentally broken by a recent change. The core of the fix for this was to update `useSmartTooltip()` to remove the dependencies array so that a newly rendered tooltip is positioned even if the mouseX/mouseY coordinates don't change (as they don't when panning via wheel).

I also cleaned a couple of unrelated things up while doing this:
* Consolidated hover reset logic formerly split between `CanvasPage` and `Surface` into the `Surface` `handleInteraction()` function.
* Cleaned up redundant ref setting code in EventTooltip.
Brian Vaughn authored and zhengjitf committed Apr 15, 2022

Verified

This commit was signed with the committer’s verified signature.
targos Michaël Zasso
1 parent 97a2fc1 commit cb0085e
Showing 4 changed files with 102 additions and 203 deletions.
38 changes: 0 additions & 38 deletions packages/react-devtools-scheduling-profiler/src/CanvasPage.js
Original file line number Diff line number Diff line change
@@ -419,44 +419,6 @@ function AutoSizedCanvas({
return;
}

// Wheel events should always hide the current tooltip.
switch (interaction.type) {
case 'wheel-control':
case 'wheel-meta':
case 'wheel-plain':
case 'wheel-shift':
setHoveredEvent(prevHoverEvent => {
if (prevHoverEvent === null) {
return prevHoverEvent;
} else if (
prevHoverEvent.componentMeasure !== null ||
prevHoverEvent.flamechartStackFrame !== null ||
prevHoverEvent.measure !== null ||
prevHoverEvent.nativeEvent !== null ||
prevHoverEvent.networkMeasure !== null ||
prevHoverEvent.schedulingEvent !== null ||
prevHoverEvent.snapshot !== null ||
prevHoverEvent.suspenseEvent !== null ||
prevHoverEvent.userTimingMark !== null
) {
return {
componentMeasure: null,
flamechartStackFrame: null,
measure: null,
nativeEvent: null,
networkMeasure: null,
schedulingEvent: null,
snapshot: null,
suspenseEvent: null,
userTimingMark: null,
};
} else {
return prevHoverEvent;
}
});
break;
}

const surface = surfaceRef.current;
surface.handleInteraction(interaction);

252 changes: 94 additions & 158 deletions packages/react-devtools-scheduling-profiler/src/EventTooltip.js
Original file line number Diff line number Diff line change
@@ -16,15 +16,13 @@ import type {
ReactHoverContextInfo,
ReactMeasure,
ReactProfilerData,
Return,
SchedulingEvent,
Snapshot,
SuspenseEvent,
UserTimingMark,
} from './types';

import * as React from 'react';
import {useRef} from 'react';
import {formatDuration, formatTimestamp, trimString} from './utils/formatting';
import {getBatchRange} from './utils/getBatchRange';
import useSmartTooltip from './utils/useSmartTooltip';
@@ -75,7 +73,7 @@ export default function EventTooltip({
hoveredEvent,
origin,
}: Props) {
const tooltipRef = useSmartTooltip({
const ref = useSmartTooltip({
canvasRef,
mouseX: origin.x,
mouseY: origin.y,
@@ -97,77 +95,53 @@ export default function EventTooltip({
userTimingMark,
} = hoveredEvent;

let content = null;
if (componentMeasure !== null) {
return (
<TooltipReactComponentMeasure
componentMeasure={componentMeasure}
tooltipRef={tooltipRef}
/>
content = (
<TooltipReactComponentMeasure componentMeasure={componentMeasure} />
);
} else if (nativeEvent !== null) {
return (
<TooltipNativeEvent nativeEvent={nativeEvent} tooltipRef={tooltipRef} />
);
content = <TooltipNativeEvent nativeEvent={nativeEvent} />;
} else if (networkMeasure !== null) {
return (
<TooltipNetworkMeasure
networkMeasure={networkMeasure}
tooltipRef={tooltipRef}
/>
);
content = <TooltipNetworkMeasure networkMeasure={networkMeasure} />;
} else if (schedulingEvent !== null) {
return (
<TooltipSchedulingEvent
data={data}
schedulingEvent={schedulingEvent}
tooltipRef={tooltipRef}
/>
content = (
<TooltipSchedulingEvent data={data} schedulingEvent={schedulingEvent} />
);
} else if (snapshot !== null) {
return <TooltipSnapshot snapshot={snapshot} tooltipRef={tooltipRef} />;
content = <TooltipSnapshot snapshot={snapshot} />;
} else if (suspenseEvent !== null) {
return (
<TooltipSuspenseEvent
suspenseEvent={suspenseEvent}
tooltipRef={tooltipRef}
/>
);
content = <TooltipSuspenseEvent suspenseEvent={suspenseEvent} />;
} else if (measure !== null) {
return (
<TooltipReactMeasure
data={data}
measure={measure}
tooltipRef={tooltipRef}
/>
);
content = <TooltipReactMeasure data={data} measure={measure} />;
} else if (flamechartStackFrame !== null) {
return (
<TooltipFlamechartNode
stackFrame={flamechartStackFrame}
tooltipRef={tooltipRef}
/>
);
content = <TooltipFlamechartNode stackFrame={flamechartStackFrame} />;
} else if (userTimingMark !== null) {
content = <TooltipUserTimingMark mark={userTimingMark} />;
}

if (content !== null) {
return (
<TooltipUserTimingMark mark={userTimingMark} tooltipRef={tooltipRef} />
<div className={styles.Tooltip} ref={ref}>
{content}
</div>
);
} else {
return null;
}
return null;
}

const TooltipReactComponentMeasure = ({
componentMeasure,
tooltipRef,
}: {
}: {|
componentMeasure: ReactComponentMeasure,
tooltipRef: Return<typeof useRef>,
}) => {
|}) => {
const {componentName, duration, timestamp, warning} = componentMeasure;

const label = `${componentName} rendered`;

return (
<div className={styles.Tooltip} ref={tooltipRef}>
<>
<div className={styles.TooltipSection}>
{trimString(label, 768)}
<div className={styles.Divider} />
@@ -183,52 +157,42 @@ const TooltipReactComponentMeasure = ({
<div className={styles.WarningText}>{warning}</div>
</div>
)}
</div>
</>
);
};

const TooltipFlamechartNode = ({
stackFrame,
tooltipRef,
}: {
}: {|
stackFrame: FlamechartStackFrame,
tooltipRef: Return<typeof useRef>,
}) => {
|}) => {
const {name, timestamp, duration, locationLine, locationColumn} = stackFrame;
return (
<div className={styles.Tooltip} ref={tooltipRef}>
<div className={styles.TooltipSection}>
<span className={styles.FlamechartStackFrameName}>{name}</span>
<div className={styles.DetailsGrid}>
<div className={styles.DetailsGridLabel}>Timestamp:</div>
<div>{formatTimestamp(timestamp)}</div>
<div className={styles.DetailsGridLabel}>Duration:</div>
<div>{formatDuration(duration)}</div>
{(locationLine !== undefined || locationColumn !== undefined) && (
<>
<div className={styles.DetailsGridLabel}>Location:</div>
<div>
line {locationLine}, column {locationColumn}
</div>
</>
)}
</div>
<div className={styles.TooltipSection}>
<span className={styles.FlamechartStackFrameName}>{name}</span>
<div className={styles.DetailsGrid}>
<div className={styles.DetailsGridLabel}>Timestamp:</div>
<div>{formatTimestamp(timestamp)}</div>
<div className={styles.DetailsGridLabel}>Duration:</div>
<div>{formatDuration(duration)}</div>
{(locationLine !== undefined || locationColumn !== undefined) && (
<>
<div className={styles.DetailsGridLabel}>Location:</div>
<div>
line {locationLine}, column {locationColumn}
</div>
</>
)}
</div>
</div>
);
};

const TooltipNativeEvent = ({
nativeEvent,
tooltipRef,
}: {
nativeEvent: NativeEvent,
tooltipRef: Return<typeof useRef>,
}) => {
const TooltipNativeEvent = ({nativeEvent}: {|nativeEvent: NativeEvent|}) => {
const {duration, timestamp, type, warning} = nativeEvent;

return (
<div className={styles.Tooltip} ref={tooltipRef}>
<>
<div className={styles.TooltipSection}>
<span className={styles.NativeEventName}>{trimString(type, 768)}</span>
event
@@ -245,17 +209,15 @@ const TooltipNativeEvent = ({
<div className={styles.WarningText}>{warning}</div>
</div>
)}
</div>
</>
);
};

const TooltipNetworkMeasure = ({
networkMeasure,
tooltipRef,
}: {
}: {|
networkMeasure: NetworkMeasure,
tooltipRef: Return<typeof useRef>,
}) => {
|}) => {
const {
finishTimestamp,
lastReceivedDataTimestamp,
@@ -278,24 +240,20 @@ const TooltipNetworkMeasure = ({
: '(incomplete)';

return (
<div className={styles.Tooltip} ref={tooltipRef}>
<div className={styles.SingleLineTextSection}>
{duration} <span className={styles.DimText}>{priority}</span>{' '}
{urlToDisplay}
</div>
<div className={styles.SingleLineTextSection}>
{duration} <span className={styles.DimText}>{priority}</span>{' '}
{urlToDisplay}
</div>
);
};

const TooltipSchedulingEvent = ({
data,
schedulingEvent,
tooltipRef,
}: {
}: {|
data: ReactProfilerData,
schedulingEvent: SchedulingEvent,
tooltipRef: Return<typeof useRef>,
}) => {
|}) => {
const label = getSchedulingEventLabel(schedulingEvent);
if (!label) {
if (__DEV__) {
@@ -323,7 +281,7 @@ const TooltipSchedulingEvent = ({
const {componentName, timestamp, warning} = schedulingEvent;

return (
<div className={styles.Tooltip} ref={tooltipRef}>
<>
<div className={styles.TooltipSection}>
{componentName && (
<span className={styles.ComponentName}>
@@ -350,35 +308,25 @@ const TooltipSchedulingEvent = ({
<div className={styles.WarningText}>{warning}</div>
</div>
)}
</div>
</>
);
};

const TooltipSnapshot = ({
snapshot,
tooltipRef,
}: {
snapshot: Snapshot,
tooltipRef: Return<typeof useRef>,
}) => {
const TooltipSnapshot = ({snapshot}: {|snapshot: Snapshot|}) => {
return (
<div className={styles.Tooltip} ref={tooltipRef}>
<img
className={styles.Image}
src={snapshot.imageSource}
style={{width: snapshot.width / 2, height: snapshot.height / 2}}
/>
</div>
<img
className={styles.Image}
src={snapshot.imageSource}
style={{width: snapshot.width / 2, height: snapshot.height / 2}}
/>
);
};

const TooltipSuspenseEvent = ({
suspenseEvent,
tooltipRef,
}: {
}: {|
suspenseEvent: SuspenseEvent,
tooltipRef: Return<typeof useRef>,
}) => {
|}) => {
const {
componentName,
duration,
@@ -394,7 +342,7 @@ const TooltipSuspenseEvent = ({
}

return (
<div className={styles.Tooltip} ref={tooltipRef}>
<>
<div className={styles.TooltipSection}>
{componentName && (
<span className={styles.ComponentName}>
@@ -421,19 +369,17 @@ const TooltipSuspenseEvent = ({
<div className={styles.WarningText}>{warning}</div>
</div>
)}
</div>
</>
);
};

const TooltipReactMeasure = ({
data,
measure,
tooltipRef,
}: {
}: {|
data: ReactProfilerData,
measure: ReactMeasure,
tooltipRef: Return<typeof useRef>,
}) => {
|}) => {
const label = getReactMeasureLabel(measure.type);
if (!label) {
if (__DEV__) {
@@ -450,52 +396,42 @@ const TooltipReactMeasure = ({
);

return (
<div className={styles.Tooltip} ref={tooltipRef}>
<div className={styles.TooltipSection}>
<span className={styles.ReactMeasureLabel}>{label}</span>
<div className={styles.Divider} />
<div className={styles.DetailsGrid}>
<div className={styles.DetailsGridLabel}>Timestamp:</div>
<div>{formatTimestamp(timestamp)}</div>
{measure.type !== 'render-idle' && (
<>
<div className={styles.DetailsGridLabel}>Duration:</div>
<div>{formatDuration(duration)}</div>
</>
)}
<div className={styles.DetailsGridLabel}>Batch duration:</div>
<div>{formatDuration(stopTime - startTime)}</div>
<div className={styles.DetailsGridLabel}>
Lane{lanes.length === 1 ? '' : 's'}:
</div>
<div>
{laneLabels.length > 0
? `${laneLabels.join(', ')} (${lanes.join(', ')})`
: lanes.join(', ')}
</div>
<div className={styles.TooltipSection}>
<span className={styles.ReactMeasureLabel}>{label}</span>
<div className={styles.Divider} />
<div className={styles.DetailsGrid}>
<div className={styles.DetailsGridLabel}>Timestamp:</div>
<div>{formatTimestamp(timestamp)}</div>
{measure.type !== 'render-idle' && (
<>
<div className={styles.DetailsGridLabel}>Duration:</div>
<div>{formatDuration(duration)}</div>
</>
)}
<div className={styles.DetailsGridLabel}>Batch duration:</div>
<div>{formatDuration(stopTime - startTime)}</div>
<div className={styles.DetailsGridLabel}>
Lane{lanes.length === 1 ? '' : 's'}:
</div>
<div>
{laneLabels.length > 0
? `${laneLabels.join(', ')} (${lanes.join(', ')})`
: lanes.join(', ')}
</div>
</div>
</div>
);
};

const TooltipUserTimingMark = ({
mark,
tooltipRef,
}: {
mark: UserTimingMark,
tooltipRef: Return<typeof useRef>,
}) => {
const TooltipUserTimingMark = ({mark}: {|mark: UserTimingMark|}) => {
const {name, timestamp} = mark;
return (
<div className={styles.Tooltip} ref={tooltipRef}>
<div className={styles.TooltipSection}>
<span className={styles.UserTimingLabel}>{name}</span>
<div className={styles.Divider} />
<div className={styles.DetailsGrid}>
<div className={styles.DetailsGridLabel}>Timestamp:</div>
<div>{formatTimestamp(timestamp)}</div>
</div>
<div className={styles.TooltipSection}>
<span className={styles.UserTimingLabel}>{name}</span>
<div className={styles.Divider} />
<div className={styles.DetailsGrid}>
<div className={styles.DetailsGridLabel}>Timestamp:</div>
<div>{formatTimestamp(timestamp)}</div>
</div>
</div>
);
Original file line number Diff line number Diff line change
@@ -75,7 +75,7 @@ export default function useSmartTooltip({
element.style.left = `${mouseX + TOOLTIP_OFFSET_BOTTOM}px`;
}
}
}, [mouseX, mouseY, ref]);
});

return ref;
}
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@
* @flow
*/

import type {ReactHoverContextInfo} from '../types';
import type {Interaction} from './useCanvasInteraction';
import type {Size} from './geometry';

@@ -48,9 +47,7 @@ const getCanvasContext = memoize(
},
);

type ResetHoveredEventFn = (
partialState: $Shape<ReactHoverContextInfo>,
) => void;
type ResetHoveredEventFn = () => void;

/**
* Represents the canvas surface and a view heirarchy. A surface is also the
@@ -123,7 +120,11 @@ export class Surface {
const viewRefs = this._viewRefs;
switch (interaction.type) {
case 'mousemove':
// Clean out the hovered view before processing a new mouse move interaction.
case 'wheel-control':
case 'wheel-meta':
case 'wheel-plain':
case 'wheel-shift':
// Clean out the hovered view before processing this type of interaction.
const hoveredView = viewRefs.hoveredView;
viewRefs.hoveredView = null;

@@ -134,7 +135,7 @@ export class Surface {

// If a previously hovered view is no longer hovered, update the outer state.
if (hoveredView !== null && viewRefs.hoveredView === null) {
this._resetHoveredEvent({});
this._resetHoveredEvent();
}
break;
default:

0 comments on commit cb0085e

Please sign in to comment.