@samitouri / QOS-React-2 / commits / aa25824f3e

Scheduling profiler: Fix tooltip wheel event regression (#22130)

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 committed Aug 18, 2021 at 19:51 UTC aa25824f3ebbdbbea01be48417f2f99251be1a12
4 files changed +102 -203
packages/react-devtools-scheduling-profiler/src/CanvasPage.js
-38
@@ -419,44 +419,6 @@ function AutoSizedCanvas({
419 return;
420 }
421
422 - // Wheel events should always hide the current tooltip.
423 - switch (interaction.type) {
424 - case 'wheel-control':
425 - case 'wheel-meta':
426 - case 'wheel-plain':
427 - case 'wheel-shift':
428 - setHoveredEvent(prevHoverEvent => {
429 - if (prevHoverEvent === null) {
430 - return prevHoverEvent;
431 - } else if (
432 - prevHoverEvent.componentMeasure !== null ||
433 - prevHoverEvent.flamechartStackFrame !== null ||
434 - prevHoverEvent.measure !== null ||
435 - prevHoverEvent.nativeEvent !== null ||
436 - prevHoverEvent.networkMeasure !== null ||
437 - prevHoverEvent.schedulingEvent !== null ||
438 - prevHoverEvent.snapshot !== null ||
439 - prevHoverEvent.suspenseEvent !== null ||
440 - prevHoverEvent.userTimingMark !== null
441 - ) {
442 - return {
443 - componentMeasure: null,
444 - flamechartStackFrame: null,
445 - measure: null,
446 - nativeEvent: null,
447 - networkMeasure: null,
448 - schedulingEvent: null,
449 - snapshot: null,
450 - suspenseEvent: null,
451 - userTimingMark: null,
452 - };
453 - } else {
454 - return prevHoverEvent;
455 - }
456 - });
457 - break;
458 - }
459 -
422 const surface = surfaceRef.current;
423 surface.handleInteraction(interaction);
424
packages/react-devtools-scheduling-profiler/src/EventTooltip.js
+94 -158
@@ -16,7 +16,6 @@ import type {
16 ReactHoverContextInfo,
17 ReactMeasure,
18 ReactProfilerData,
19 - Return,
19 SchedulingEvent,
20 Snapshot,
21 SuspenseEvent,
@@ -24,7 +23,6 @@ import type {
23 } from './types';
24
25 import * as React from 'react';
27 -import {useRef} from 'react';
26 import {formatDuration, formatTimestamp, trimString} from './utils/formatting';
27 import {getBatchRange} from './utils/getBatchRange';
28 import useSmartTooltip from './utils/useSmartTooltip';
@@ -75,7 +73,7 @@ export default function EventTooltip({
73 hoveredEvent,
74 origin,
75 }: Props) {
78 - const tooltipRef = useSmartTooltip({
76 + const ref = useSmartTooltip({
77 canvasRef,
78 mouseX: origin.x,
79 mouseY: origin.y,
@@ -97,77 +95,53 @@ export default function EventTooltip({
95 userTimingMark,
96 } = hoveredEvent;
97
98 + let content = null;
99 if (componentMeasure !== null) {
101 - return (
102 - <TooltipReactComponentMeasure
103 - componentMeasure={componentMeasure}
104 - tooltipRef={tooltipRef}
105 - />
100 + content = (
101 + <TooltipReactComponentMeasure componentMeasure={componentMeasure} />
102 );
103 } else if (nativeEvent !== null) {
108 - return (
109 - <TooltipNativeEvent nativeEvent={nativeEvent} tooltipRef={tooltipRef} />
110 - );
104 + content = <TooltipNativeEvent nativeEvent={nativeEvent} />;
105 } else if (networkMeasure !== null) {
112 - return (
113 - <TooltipNetworkMeasure
114 - networkMeasure={networkMeasure}
115 - tooltipRef={tooltipRef}
116 - />
117 - );
106 + content = <TooltipNetworkMeasure networkMeasure={networkMeasure} />;
107 } else if (schedulingEvent !== null) {
119 - return (
120 - <TooltipSchedulingEvent
121 - data={data}
122 - schedulingEvent={schedulingEvent}
123 - tooltipRef={tooltipRef}
124 - />
108 + content = (
109 + <TooltipSchedulingEvent data={data} schedulingEvent={schedulingEvent} />
110 );
111 } else if (snapshot !== null) {
127 - return <TooltipSnapshot snapshot={snapshot} tooltipRef={tooltipRef} />;
112 + content = <TooltipSnapshot snapshot={snapshot} />;
113 } else if (suspenseEvent !== null) {
129 - return (
130 - <TooltipSuspenseEvent
131 - suspenseEvent={suspenseEvent}
132 - tooltipRef={tooltipRef}
133 - />
134 - );
114 + content = <TooltipSuspenseEvent suspenseEvent={suspenseEvent} />;
115 } else if (measure !== null) {
136 - return (
137 - <TooltipReactMeasure
138 - data={data}
139 - measure={measure}
140 - tooltipRef={tooltipRef}
141 - />
142 - );
116 + content = <TooltipReactMeasure data={data} measure={measure} />;
117 } else if (flamechartStackFrame !== null) {
144 - return (
145 - <TooltipFlamechartNode
146 - stackFrame={flamechartStackFrame}
147 - tooltipRef={tooltipRef}
148 - />
149 - );
118 + content = <TooltipFlamechartNode stackFrame={flamechartStackFrame} />;
119 } else if (userTimingMark !== null) {
120 + content = <TooltipUserTimingMark mark={userTimingMark} />;
121 + }
122 +
123 + if (content !== null) {
124 return (
152 - <TooltipUserTimingMark mark={userTimingMark} tooltipRef={tooltipRef} />
125 + <div className={styles.Tooltip} ref={ref}>
126 + {content}
127 + </div>
128 );
129 + } else {
130 + return null;
131 }
155 - return null;
132 }
133
134 const TooltipReactComponentMeasure = ({
135 componentMeasure,
160 - tooltipRef,
161 -}: {
136 +}: {|
137 componentMeasure: ReactComponentMeasure,
163 - tooltipRef: Return<typeof useRef>,
164 -}) => {
138 +|}) => {
139 const {componentName, duration, timestamp, warning} = componentMeasure;
140
141 const label = `${componentName} rendered`;
142
143 return (
170 - <div className={styles.Tooltip} ref={tooltipRef}>
144 + <>
145 <div className={styles.TooltipSection}>
146 {trimString(label, 768)}
147 <div className={styles.Divider} />
@@ -183,52 +157,42 @@ const TooltipReactComponentMeasure = ({
157 <div className={styles.WarningText}>{warning}</div>
158 </div>
159 )}
186 - </div>
160 + </>
161 );
162 };
163
164 const TooltipFlamechartNode = ({
165 stackFrame,
192 - tooltipRef,
193 -}: {
166 +}: {|
167 stackFrame: FlamechartStackFrame,
195 - tooltipRef: Return<typeof useRef>,
196 -}) => {
168 +|}) => {
169 const {name, timestamp, duration, locationLine, locationColumn} = stackFrame;
170 return (
199 - <div className={styles.Tooltip} ref={tooltipRef}>
200 - <div className={styles.TooltipSection}>
201 - <span className={styles.FlamechartStackFrameName}>{name}</span>
202 - <div className={styles.DetailsGrid}>
203 - <div className={styles.DetailsGridLabel}>Timestamp:</div>
204 - <div>{formatTimestamp(timestamp)}</div>
205 - <div className={styles.DetailsGridLabel}>Duration:</div>
206 - <div>{formatDuration(duration)}</div>
207 - {(locationLine !== undefined || locationColumn !== undefined) && (
208 - <>
209 - <div className={styles.DetailsGridLabel}>Location:</div>
210 - <div>
211 - line {locationLine}, column {locationColumn}
212 - </div>
213 - </>
214 - )}
215 - </div>
171 + <div className={styles.TooltipSection}>
172 + <span className={styles.FlamechartStackFrameName}>{name}</span>
173 + <div className={styles.DetailsGrid}>
174 + <div className={styles.DetailsGridLabel}>Timestamp:</div>
175 + <div>{formatTimestamp(timestamp)}</div>
176 + <div className={styles.DetailsGridLabel}>Duration:</div>
177 + <div>{formatDuration(duration)}</div>
178 + {(locationLine !== undefined || locationColumn !== undefined) && (
179 + <>
180 + <div className={styles.DetailsGridLabel}>Location:</div>
181 + <div>
182 + line {locationLine}, column {locationColumn}
183 + </div>
184 + </>
185 + )}
186 </div>
187 </div>
188 );
189 };
190
221 -const TooltipNativeEvent = ({
222 - nativeEvent,
223 - tooltipRef,
224 -}: {
225 - nativeEvent: NativeEvent,
226 - tooltipRef: Return<typeof useRef>,
227 -}) => {
191 +const TooltipNativeEvent = ({nativeEvent}: {|nativeEvent: NativeEvent|}) => {
192 const {duration, timestamp, type, warning} = nativeEvent;
193
194 return (
231 - <div className={styles.Tooltip} ref={tooltipRef}>
195 + <>
196 <div className={styles.TooltipSection}>
197 <span className={styles.NativeEventName}>{trimString(type, 768)}</span>
198 event
@@ -245,17 +209,15 @@ const TooltipNativeEvent = ({
209 <div className={styles.WarningText}>{warning}</div>
210 </div>
211 )}
248 - </div>
212 + </>
213 );
214 };
215
216 const TooltipNetworkMeasure = ({
217 networkMeasure,
254 - tooltipRef,
255 -}: {
218 +}: {|
219 networkMeasure: NetworkMeasure,
257 - tooltipRef: Return<typeof useRef>,
258 -}) => {
220 +|}) => {
221 const {
222 finishTimestamp,
223 lastReceivedDataTimestamp,
@@ -278,11 +240,9 @@ const TooltipNetworkMeasure = ({
240 : '(incomplete)';
241
242 return (
281 - <div className={styles.Tooltip} ref={tooltipRef}>
282 - <div className={styles.SingleLineTextSection}>
283 - {duration} <span className={styles.DimText}>{priority}</span>{' '}
284 - {urlToDisplay}
285 - </div>
243 + <div className={styles.SingleLineTextSection}>
244 + {duration} <span className={styles.DimText}>{priority}</span>{' '}
245 + {urlToDisplay}
246 </div>
247 );
248 };
@@ -290,12 +250,10 @@ const TooltipNetworkMeasure = ({
250 const TooltipSchedulingEvent = ({
251 data,
252 schedulingEvent,
293 - tooltipRef,
294 -}: {
253 +}: {|
254 data: ReactProfilerData,
255 schedulingEvent: SchedulingEvent,
297 - tooltipRef: Return<typeof useRef>,
298 -}) => {
256 +|}) => {
257 const label = getSchedulingEventLabel(schedulingEvent);
258 if (!label) {
259 if (__DEV__) {
@@ -323,7 +281,7 @@ const TooltipSchedulingEvent = ({
281 const {componentName, timestamp, warning} = schedulingEvent;
282
283 return (
326 - <div className={styles.Tooltip} ref={tooltipRef}>
284 + <>
285 <div className={styles.TooltipSection}>
286 {componentName && (
287 <span className={styles.ComponentName}>
@@ -350,35 +308,25 @@ const TooltipSchedulingEvent = ({
308 <div className={styles.WarningText}>{warning}</div>
309 </div>
310 )}
353 - </div>
311 + </>
312 );
313 };
314
357 -const TooltipSnapshot = ({
358 - snapshot,
359 - tooltipRef,
360 -}: {
361 - snapshot: Snapshot,
362 - tooltipRef: Return<typeof useRef>,
363 -}) => {
315 +const TooltipSnapshot = ({snapshot}: {|snapshot: Snapshot|}) => {
316 return (
365 - <div className={styles.Tooltip} ref={tooltipRef}>
366 - <img
367 - className={styles.Image}
368 - src={snapshot.imageSource}
369 - style={{width: snapshot.width / 2, height: snapshot.height / 2}}
370 - />
371 - </div>
317 + <img
318 + className={styles.Image}
319 + src={snapshot.imageSource}
320 + style={{width: snapshot.width / 2, height: snapshot.height / 2}}
321 + />
322 );
323 };
324
325 const TooltipSuspenseEvent = ({
326 suspenseEvent,
377 - tooltipRef,
378 -}: {
327 +}: {|
328 suspenseEvent: SuspenseEvent,
380 - tooltipRef: Return<typeof useRef>,
381 -}) => {
329 +|}) => {
330 const {
331 componentName,
332 duration,
@@ -394,7 +342,7 @@ const TooltipSuspenseEvent = ({
342 }
343
344 return (
397 - <div className={styles.Tooltip} ref={tooltipRef}>
345 + <>
346 <div className={styles.TooltipSection}>
347 {componentName && (
348 <span className={styles.ComponentName}>
@@ -421,19 +369,17 @@ const TooltipSuspenseEvent = ({
369 <div className={styles.WarningText}>{warning}</div>
370 </div>
371 )}
424 - </div>
372 + </>
373 );
374 };
375
376 const TooltipReactMeasure = ({
377 data,
378 measure,
431 - tooltipRef,
432 -}: {
379 +}: {|
380 data: ReactProfilerData,
381 measure: ReactMeasure,
435 - tooltipRef: Return<typeof useRef>,
436 -}) => {
382 +|}) => {
383 const label = getReactMeasureLabel(measure.type);
384 if (!label) {
385 if (__DEV__) {
@@ -450,52 +396,42 @@ const TooltipReactMeasure = ({
396 );
397
398 return (
453 - <div className={styles.Tooltip} ref={tooltipRef}>
454 - <div className={styles.TooltipSection}>
455 - <span className={styles.ReactMeasureLabel}>{label}</span>
456 - <div className={styles.Divider} />
457 - <div className={styles.DetailsGrid}>
458 - <div className={styles.DetailsGridLabel}>Timestamp:</div>
459 - <div>{formatTimestamp(timestamp)}</div>
460 - {measure.type !== 'render-idle' && (
461 - <>
462 - <div className={styles.DetailsGridLabel}>Duration:</div>
463 - <div>{formatDuration(duration)}</div>
464 - </>
465 - )}
466 - <div className={styles.DetailsGridLabel}>Batch duration:</div>
467 - <div>{formatDuration(stopTime - startTime)}</div>
468 - <div className={styles.DetailsGridLabel}>
469 - Lane{lanes.length === 1 ? '' : 's'}:
470 - </div>
471 - <div>
472 - {laneLabels.length > 0
473 - ? `${laneLabels.join(', ')} (${lanes.join(', ')})`
474 - : lanes.join(', ')}
475 - </div>
399 + <div className={styles.TooltipSection}>
400 + <span className={styles.ReactMeasureLabel}>{label}</span>
401 + <div className={styles.Divider} />
402 + <div className={styles.DetailsGrid}>
403 + <div className={styles.DetailsGridLabel}>Timestamp:</div>
404 + <div>{formatTimestamp(timestamp)}</div>
405 + {measure.type !== 'render-idle' && (
406 + <>
407 + <div className={styles.DetailsGridLabel}>Duration:</div>
408 + <div>{formatDuration(duration)}</div>
409 + </>
410 + )}
411 + <div className={styles.DetailsGridLabel}>Batch duration:</div>
412 + <div>{formatDuration(stopTime - startTime)}</div>
413 + <div className={styles.DetailsGridLabel}>
414 + Lane{lanes.length === 1 ? '' : 's'}:
415 + </div>
416 + <div>
417 + {laneLabels.length > 0
418 + ? `${laneLabels.join(', ')} (${lanes.join(', ')})`
419 + : lanes.join(', ')}
420 </div>
421 </div>
422 </div>
423 );
424 };
425
482 -const TooltipUserTimingMark = ({
483 - mark,
484 - tooltipRef,
485 -}: {
486 - mark: UserTimingMark,
487 - tooltipRef: Return<typeof useRef>,
488 -}) => {
426 +const TooltipUserTimingMark = ({mark}: {|mark: UserTimingMark|}) => {
427 const {name, timestamp} = mark;
428 return (
491 - <div className={styles.Tooltip} ref={tooltipRef}>
492 - <div className={styles.TooltipSection}>
493 - <span className={styles.UserTimingLabel}>{name}</span>
494 - <div className={styles.Divider} />
495 - <div className={styles.DetailsGrid}>
496 - <div className={styles.DetailsGridLabel}>Timestamp:</div>
497 - <div>{formatTimestamp(timestamp)}</div>
498 - </div>
429 + <div className={styles.TooltipSection}>
430 + <span className={styles.UserTimingLabel}>{name}</span>
431 + <div className={styles.Divider} />
432 + <div className={styles.DetailsGrid}>
433 + <div className={styles.DetailsGridLabel}>Timestamp:</div>
434 + <div>{formatTimestamp(timestamp)}</div>
435 </div>
436 </div>
437 );
packages/react-devtools-scheduling-profiler/src/utils/useSmartTooltip.js
+1 -1
@@ -75,7 +75,7 @@ export default function useSmartTooltip({
75 element.style.left = `${mouseX + TOOLTIP_OFFSET_BOTTOM}px`;
76 }
77 }
78 - }, [mouseX, mouseY, ref]);
78 + });
79
80 return ref;
81 }
packages/react-devtools-scheduling-profiler/src/view-base/Surface.js
+7 -6
@@ -7,7 +7,6 @@
7 * @flow
8 */
9
10 -import type {ReactHoverContextInfo} from '../types';
10 import type {Interaction} from './useCanvasInteraction';
11 import type {Size} from './geometry';
12
@@ -48,9 +47,7 @@ const getCanvasContext = memoize(
47 },
48 );
49
51 -type ResetHoveredEventFn = (
52 - partialState: $Shape<ReactHoverContextInfo>,
53 -) => void;
50 +type ResetHoveredEventFn = () => void;
51
52 /**
53 * Represents the canvas surface and a view heirarchy. A surface is also the
@@ -123,7 +120,11 @@ export class Surface {
120 const viewRefs = this._viewRefs;
121 switch (interaction.type) {
122 case 'mousemove':
126 - // Clean out the hovered view before processing a new mouse move interaction.
123 + case 'wheel-control':
124 + case 'wheel-meta':
125 + case 'wheel-plain':
126 + case 'wheel-shift':
127 + // Clean out the hovered view before processing this type of interaction.
128 const hoveredView = viewRefs.hoveredView;
129 viewRefs.hoveredView = null;
130
@@ -134,7 +135,7 @@ export class Surface {
135
136 // If a previously hovered view is no longer hovered, update the outer state.
137 if (hoveredView !== null && viewRefs.hoveredView === null) {
137 - this._resetHoveredEvent({});
138 + this._resetHoveredEvent();
139 }
140 break;
141 default: