Skip to content

Commit b467c6e

Browse files
authored
[DevTools] Explicitly say which id to scroll to and only once (#34823)
This ensures that we don't scroll on changes to the timeline such as when loading a new page or while the timeline is still loading. We only auto scroll to a boundary when we perform an explicit operation from the user.
1 parent 93d4458 commit b467c6e

File tree

2 files changed

+19
-15
lines changed

2 files changed

+19
-15
lines changed

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import * as React from 'react';
11-
import {useContext, useEffect, useRef} from 'react';
11+
import {useContext, useEffect} from 'react';
1212
import {BridgeContext} from '../context';
1313
import {TreeDispatcherContext} from '../Components/TreeContext';
1414
import {useHighlightHostInstance, useScrollToHostInstance} from '../hooks';
@@ -29,9 +29,8 @@ function SuspenseTimelineInput() {
2929
useHighlightHostInstance();
3030
const scrollToHostInstance = useScrollToHostInstance();
3131

32-
const {timeline, timelineIndex, hoveredTimelineIndex, playing} = useContext(
33-
SuspenseTreeStateContext,
34-
);
32+
const {timeline, timelineIndex, hoveredTimelineIndex, playing, autoScroll} =
33+
useContext(SuspenseTreeStateContext);
3534

3635
const min = 0;
3736
const max = timeline.length > 0 ? timeline.length - 1 : 0;
@@ -102,7 +101,6 @@ function SuspenseTimelineInput() {
102101
});
103102
}
104103

105-
const isInitialMount = useRef(true);
106104
// TODO: useEffectEvent here once it's supported in all versions DevTools supports.
107105
// For now we just exclude it from deps since we don't lint those anyway.
108106
function changeTimelineIndex(newIndex: number) {
@@ -115,22 +113,21 @@ function SuspenseTimelineInput() {
115113
bridge.send('overrideSuspenseMilestone', {
116114
suspendedSet,
117115
});
118-
if (isInitialMount.current) {
119-
// Skip scrolling on initial mount. Only when we're changing the timeline.
120-
isInitialMount.current = false;
121-
} else {
122-
// When we're scrubbing through the timeline, scroll the current boundary
123-
// into view as it was just revealed. This is after we override the milestone
124-
// to reveal it.
125-
const selectedSuspenseID = timeline[timelineIndex];
126-
scrollToHostInstance(selectedSuspenseID);
127-
}
128116
}
129117

130118
useEffect(() => {
131119
changeTimelineIndex(timelineIndex);
132120
}, [timelineIndex]);
133121

122+
useEffect(() => {
123+
if (autoScroll.id > 0) {
124+
const scrollToId = autoScroll.id;
125+
// Consume the scroll ref so that we only trigger this scroll once.
126+
autoScroll.id = 0;
127+
scrollToHostInstance(scrollToId);
128+
}
129+
}, [autoScroll]);
130+
134131
useEffect(() => {
135132
if (!playing) {
136133
return undefined;

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export type SuspenseTreeState = {
3131
uniqueSuspendersOnly: boolean,
3232
playing: boolean,
3333
autoSelect: boolean,
34+
autoScroll: {id: number}, // Ref that's set to 0 after scrolling once.
3435
};
3536

3637
type ACTION_SUSPENSE_TREE_MUTATION = {
@@ -125,6 +126,7 @@ function getInitialState(store: Store): SuspenseTreeState {
125126
uniqueSuspendersOnly,
126127
playing: false,
127128
autoSelect: true,
129+
autoScroll: {id: 0}, // Don't auto-scroll initially
128130
};
129131

130132
return initialState;
@@ -218,6 +220,7 @@ function SuspenseTreeContextController({children}: Props): React.Node {
218220
selectedSuspenseID,
219221
playing: false, // pause
220222
autoSelect: false,
223+
autoScroll: {id: selectedSuspenseID}, // scroll
221224
};
222225
}
223226
case 'SET_SUSPENSE_LINEAGE': {
@@ -285,6 +288,7 @@ function SuspenseTreeContextController({children}: Props): React.Node {
285288
timelineIndex: nextTimelineIndex,
286289
playing: false, // pause
287290
autoSelect: false,
291+
autoScroll: {id: nextSelectedSuspenseID}, // scroll
288292
};
289293
}
290294
case 'SUSPENSE_SKIP_TIMELINE_INDEX': {
@@ -308,6 +312,7 @@ function SuspenseTreeContextController({children}: Props): React.Node {
308312
timelineIndex: nextTimelineIndex,
309313
playing: false, // pause
310314
autoSelect: false,
315+
autoScroll: {id: nextSelectedSuspenseID}, // scroll
311316
};
312317
}
313318
case 'SUSPENSE_PLAY_PAUSE': {
@@ -359,6 +364,7 @@ function SuspenseTreeContextController({children}: Props): React.Node {
359364
selectedSuspenseID: nextSelectedSuspenseID,
360365
timelineIndex: nextTimelineIndex,
361366
playing: nextPlaying,
367+
autoScroll: {id: nextSelectedSuspenseID}, // scroll
362368
};
363369
}
364370
case 'TOGGLE_TIMELINE_FOR_ID': {
@@ -392,6 +398,7 @@ function SuspenseTreeContextController({children}: Props): React.Node {
392398
timelineIndex: nextTimelineIndex,
393399
playing: false, // pause
394400
autoSelect: false,
401+
autoScroll: {id: nextSelectedSuspenseID},
395402
};
396403
}
397404
case 'HOVER_TIMELINE_FOR_ID': {

0 commit comments

Comments
 (0)