[Devtools] Navigating commits performance panel hotkey #35238
Conversation
| selectedCommitIndex !== null | ||
| ) { | ||
| // Left/Right to navigate commits | ||
| if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { |
There was a problem hiding this comment.
We should probably check for Cmd key as well. I think we should keep just arrow navigations for changes between nodes in tree view (outside of the scope for this PR), and Cmd + ->, Cmd + <- for navigation between commits. What do you think?
There was a problem hiding this comment.
I agree it's nicer to have them unique but the arrow navigation for the tree view nodes only works when one has just been selected and when that has happened even having them as different shortcuts you can't use a shortcut to navigate between commits. I'm not convinced how much the tab navigation one will be used if your mouse has already selected a tab in the area there just before, and therefore it could be nicer to leave the shortcut as arrow key for simplicity
There was a problem hiding this comment.
Maybe it would confuse people they are switching tabs when they expect to switch commits if they don't realise they need to select it
There was a problem hiding this comment.
I am not sure I am following. We don't have keyboard navigation for the flamegraph in Profiler tree, right? But if we would add one, it would probably be done by using only arrow keys.
If so, then it would conflict with the navigation between commits. That's my current thinking, what is the alternative?
packages/react-devtools-shared/src/__tests__/profilerContext-test.js
Outdated
Show resolved
Hide resolved
packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js
Outdated
Show resolved
Hide resolved
packages/react-devtools-shared/src/__tests__/profilerContext-test.js
Outdated
Show resolved
Hide resolved
…ance-panel-hot-key
https://github.com/emily8rown/react into devtools-navigating-commits-performance-panel-hot-key
packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js
Outdated
Show resolved
Hide resolved
hoxyq
left a comment
hoxyq
left a comment
There was a problem hiding this comment.
Looks good, left a minor suggestion to avoid cascading update.
Also, could you please validate that reload-and-profile works after these changes?
| // Clear selections when starting a new profiling session | ||
| useEffect(() => { | ||
| if (isProfiling) { | ||
| selectCommitIndex(null); | ||
| selectFiberID(null); | ||
| selectFiberName(null); | ||
| } | ||
| } | ||
| }, [isProfiling, selectCommitIndex]); |
There was a problem hiding this comment.
Can this logic be moved to startProfiling() callback?
2 participants
Summary
Add keyboard shortcuts (Cmd/Ctrl + Left/Right arrow keys) to navigate between commits in the Profiler's snapshot view.
Moved
filteredCommitIndicesmanagement and commit navigation logic (selectNextCommitIndex,selectPrevCommitIndex) fromSnapshotSelectorintouseCommitFilteringAndNavigationused byProfilerContextto enable keyboard shortcuts from the top-level Profiler component.How did you test this change?
yarn build:<browser name>yarn run test:<browser name>Chrome:
Screen.Recording.2025-11-27.at.18.19.45.mov
Edge with duration filter:
Screen.Recording.2025-11-28.at.12.23.23.mov
firefox mixing hotkey with clicking arrow buttons:
Screen.Recording.2025-11-28.at.12.34.07.mov