Skip to content

Commit d763f31

Browse files
authored
[Devtools] Navigating commits performance panel hotkey (#35238)
## Summary Add keyboard shortcuts (Cmd/Ctrl + Left/Right arrow keys) to navigate between commits in the Profiler's snapshot view. Moved `filteredCommitIndices` management and commit navigation logic (`selectNextCommitIndex`, `selectPrevCommitIndex`) from `SnapshotSelector` into `useCommitFilteringAndNavigation` used by `ProfilerContext` to enable keyboard shortcuts from the top-level Profiler component. ## How did you test this change? - New tests in ProfilerContext-tests - Built browser extension: `yarn build:<browser name>` - tested in browser: `yarn run test:<browser name>` - Manually verified Left/Right arrow navigation cycles through commits - Verified navigation respects commit duration filter - Verified reload-and-profile button unaffected Chrome: https://github.com/user-attachments/assets/01d2a749-13dc-4d08-8bcb-3d4d45a5f97c Edge with duration filter: https://github.com/user-attachments/assets/a7f76ff7-2a0b-4b9c-a0ce-d4449373308b firefox mixing hotkey with clicking arrow buttons: https://github.com/user-attachments/assets/48912d68-7c75-40f2-a203-5e6d7e6b2d99
1 parent 734f1bf commit d763f31

File tree

5 files changed

+595
-93
lines changed

5 files changed

+595
-93
lines changed

packages/react-devtools-shared/src/__tests__/profilerContext-test.js

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,4 +655,288 @@ describe('ProfilerContext', () => {
655655

656656
document.body.removeChild(profilerContainer);
657657
});
658+
659+
it('should navigate between commits when the keyboard shortcut is pressed', async () => {
660+
const Parent = () => <Child />;
661+
const Child = () => null;
662+
663+
const container = document.createElement('div');
664+
const root = ReactDOMClient.createRoot(container);
665+
utils.act(() => root.render(<Parent />));
666+
667+
// Profile and record multiple commits
668+
await utils.actAsync(() => store.profilerStore.startProfiling());
669+
await utils.actAsync(() => root.render(<Parent />)); // Commit 1
670+
await utils.actAsync(() => root.render(<Parent />)); // Commit 2
671+
await utils.actAsync(() => root.render(<Parent />)); // Commit 3
672+
await utils.actAsync(() => store.profilerStore.stopProfiling());
673+
674+
const Profiler =
675+
require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default;
676+
const {
677+
TimelineContextController,
678+
} = require('react-devtools-timeline/src/TimelineContext');
679+
const {
680+
SettingsContextController,
681+
} = require('react-devtools-shared/src/devtools/views/Settings/SettingsContext');
682+
const {
683+
ModalDialogContextController,
684+
} = require('react-devtools-shared/src/devtools/views/ModalDialog');
685+
686+
let context: Context = ((null: any): Context);
687+
function ContextReader() {
688+
context = React.useContext(ProfilerContext);
689+
return null;
690+
}
691+
692+
const profilerContainer = document.createElement('div');
693+
document.body.appendChild(profilerContainer);
694+
695+
const profilerRoot = ReactDOMClient.createRoot(profilerContainer);
696+
697+
await utils.actAsync(() => {
698+
profilerRoot.render(
699+
<Contexts>
700+
<SettingsContextController browserTheme="light">
701+
<ModalDialogContextController>
702+
<TimelineContextController>
703+
<Profiler />
704+
<ContextReader />
705+
</TimelineContextController>
706+
</ModalDialogContextController>
707+
</SettingsContextController>
708+
</Contexts>,
709+
);
710+
});
711+
712+
// Verify we have profiling data with 3 commits
713+
expect(context.didRecordCommits).toBe(true);
714+
expect(context.profilingData).not.toBeNull();
715+
const rootID = context.rootID;
716+
expect(rootID).not.toBeNull();
717+
const dataForRoot = context.profilingData.dataForRoots.get(rootID);
718+
expect(dataForRoot.commitData.length).toBe(3);
719+
// Should start at the first commit
720+
expect(context.selectedCommitIndex).toBe(0);
721+
722+
const ownerWindow = profilerContainer.ownerDocument.defaultView;
723+
const isMac =
724+
typeof navigator !== 'undefined' &&
725+
navigator.platform.toUpperCase().indexOf('MAC') >= 0;
726+
727+
// Test ArrowRight navigation (forward) with correct modifier
728+
const arrowRightEvent = new KeyboardEvent('keydown', {
729+
key: 'ArrowRight',
730+
metaKey: isMac,
731+
ctrlKey: !isMac,
732+
bubbles: true,
733+
});
734+
735+
await utils.actAsync(() => {
736+
ownerWindow.dispatchEvent(arrowRightEvent);
737+
}, false);
738+
expect(context.selectedCommitIndex).toBe(1);
739+
740+
await utils.actAsync(() => {
741+
ownerWindow.dispatchEvent(arrowRightEvent);
742+
}, false);
743+
expect(context.selectedCommitIndex).toBe(2);
744+
745+
// Test wrap-around (last -> first)
746+
await utils.actAsync(() => {
747+
ownerWindow.dispatchEvent(arrowRightEvent);
748+
}, false);
749+
expect(context.selectedCommitIndex).toBe(0);
750+
751+
// Test ArrowLeft navigation (backward) with correct modifier
752+
const arrowLeftEvent = new KeyboardEvent('keydown', {
753+
key: 'ArrowLeft',
754+
metaKey: isMac,
755+
ctrlKey: !isMac,
756+
bubbles: true,
757+
});
758+
759+
await utils.actAsync(() => {
760+
ownerWindow.dispatchEvent(arrowLeftEvent);
761+
}, false);
762+
expect(context.selectedCommitIndex).toBe(2);
763+
764+
await utils.actAsync(() => {
765+
ownerWindow.dispatchEvent(arrowLeftEvent);
766+
}, false);
767+
expect(context.selectedCommitIndex).toBe(1);
768+
769+
await utils.actAsync(() => {
770+
ownerWindow.dispatchEvent(arrowLeftEvent);
771+
}, false);
772+
expect(context.selectedCommitIndex).toBe(0);
773+
774+
// Cleanup
775+
await utils.actAsync(() => profilerRoot.unmount());
776+
document.body.removeChild(profilerContainer);
777+
});
778+
779+
it('should handle commit selection edge cases when filtering commits', async () => {
780+
const Scheduler = require('scheduler');
781+
782+
// Create components that do varying amounts of work to generate different commit durations
783+
const Parent = ({count}) => {
784+
Scheduler.unstable_advanceTime(10);
785+
const items = [];
786+
for (let i = 0; i < count; i++) {
787+
items.push(<Child key={i} duration={i} />);
788+
}
789+
return <div>{items}</div>;
790+
};
791+
const Child = ({duration}) => {
792+
Scheduler.unstable_advanceTime(duration);
793+
return <span>{duration}</span>;
794+
};
795+
796+
const container = document.createElement('div');
797+
const root = ReactDOMClient.createRoot(container);
798+
utils.act(() => root.render(<Parent count={1} />));
799+
800+
// Profile and record multiple commits with different amounts of work
801+
await utils.actAsync(() => store.profilerStore.startProfiling());
802+
await utils.actAsync(() => root.render(<Parent count={5} />)); // Commit 1 - 20ms
803+
await utils.actAsync(() => root.render(<Parent count={20} />)); // Commit 2 - 200ms
804+
await utils.actAsync(() => root.render(<Parent count={50} />)); // Commit 3 - 1235ms
805+
await utils.actAsync(() => root.render(<Parent count={10} />)); // Commit 4 - 55ms
806+
await utils.actAsync(() => store.profilerStore.stopProfiling());
807+
808+
// Context providers
809+
const Profiler =
810+
require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default;
811+
const {
812+
TimelineContextController,
813+
} = require('react-devtools-timeline/src/TimelineContext');
814+
const {
815+
SettingsContextController,
816+
} = require('react-devtools-shared/src/devtools/views/Settings/SettingsContext');
817+
const {
818+
ModalDialogContextController,
819+
} = require('react-devtools-shared/src/devtools/views/ModalDialog');
820+
821+
let context: Context = ((null: any): Context);
822+
function ContextReader() {
823+
context = React.useContext(ProfilerContext);
824+
return null;
825+
}
826+
827+
const profilerContainer = document.createElement('div');
828+
document.body.appendChild(profilerContainer);
829+
830+
const profilerRoot = ReactDOMClient.createRoot(profilerContainer);
831+
832+
await utils.actAsync(() => {
833+
profilerRoot.render(
834+
<Contexts>
835+
<SettingsContextController browserTheme="light">
836+
<ModalDialogContextController>
837+
<TimelineContextController>
838+
<Profiler />
839+
<ContextReader />
840+
</TimelineContextController>
841+
</ModalDialogContextController>
842+
</SettingsContextController>
843+
</Contexts>,
844+
);
845+
});
846+
847+
// Verify we have profiling data with 4 commits
848+
expect(context.didRecordCommits).toBe(true);
849+
expect(context.profilingData).not.toBeNull();
850+
const rootID = context.rootID;
851+
expect(rootID).not.toBeNull();
852+
const dataForRoot = context.profilingData.dataForRoots.get(rootID);
853+
expect(dataForRoot.commitData.length).toBe(4);
854+
// Edge case 1: Should start at the first commit
855+
expect(context.selectedCommitIndex).toBe(0);
856+
857+
const ownerWindow = profilerContainer.ownerDocument.defaultView;
858+
const isMac =
859+
typeof navigator !== 'undefined' &&
860+
navigator.platform.toUpperCase().indexOf('MAC') >= 0;
861+
862+
const arrowRightEvent = new KeyboardEvent('keydown', {
863+
key: 'ArrowRight',
864+
metaKey: isMac,
865+
ctrlKey: !isMac,
866+
bubbles: true,
867+
});
868+
869+
await utils.actAsync(() => {
870+
ownerWindow.dispatchEvent(arrowRightEvent);
871+
}, false);
872+
expect(context.selectedCommitIndex).toBe(1);
873+
874+
await utils.actAsync(() => {
875+
context.setIsCommitFilterEnabled(true);
876+
});
877+
878+
// Edge case 2: When filtering is enabled, selected commit should remain if it's still visible
879+
expect(context.filteredCommitIndices.length).toBe(4);
880+
expect(context.selectedCommitIndex).toBe(1);
881+
expect(context.selectedFilteredCommitIndex).toBe(1);
882+
883+
await utils.actAsync(() => {
884+
context.setMinCommitDuration(1000000);
885+
});
886+
887+
// Edge case 3: When all commits are filtered out, selection should be null
888+
expect(context.filteredCommitIndices).toEqual([]);
889+
expect(context.selectedCommitIndex).toBe(null);
890+
expect(context.selectedFilteredCommitIndex).toBe(null);
891+
892+
await utils.actAsync(() => {
893+
context.setMinCommitDuration(0);
894+
});
895+
896+
// Edge case 4: After restoring commits, first commit should be auto-selected
897+
expect(context.filteredCommitIndices.length).toBe(4);
898+
expect(context.selectedCommitIndex).toBe(0);
899+
expect(context.selectedFilteredCommitIndex).toBe(0);
900+
901+
await utils.actAsync(() => {
902+
ownerWindow.dispatchEvent(arrowRightEvent);
903+
}, false);
904+
expect(context.selectedCommitIndex).toBe(1);
905+
906+
await utils.actAsync(() => {
907+
ownerWindow.dispatchEvent(arrowRightEvent);
908+
}, false);
909+
expect(context.selectedCommitIndex).toBe(2);
910+
911+
await utils.actAsync(() => {
912+
ownerWindow.dispatchEvent(arrowRightEvent);
913+
}, false);
914+
expect(context.selectedCommitIndex).toBe(3);
915+
916+
// Filter out the currently selected commit using actual commit data
917+
const commitDurations = dataForRoot.commitData.map(
918+
commit => commit.duration,
919+
);
920+
const selectedCommitDuration = commitDurations[3];
921+
const filterThreshold = selectedCommitDuration + 0.001;
922+
await utils.actAsync(() => {
923+
context.setMinCommitDuration(filterThreshold);
924+
});
925+
926+
// Edge case 5: Should auto-select first available commit when current one is filtered
927+
expect(context.selectedCommitIndex).not.toBe(null);
928+
expect(context.selectedFilteredCommitIndex).toBe(1);
929+
930+
await utils.actAsync(() => {
931+
context.setIsCommitFilterEnabled(false);
932+
});
933+
934+
// Edge case 6: When filtering is disabled, selected commit should remain
935+
expect(context.filteredCommitIndices.length).toBe(4);
936+
expect(context.selectedCommitIndex).toBe(2);
937+
expect(context.selectedFilteredCommitIndex).toBe(2);
938+
939+
await utils.actAsync(() => profilerRoot.unmount());
940+
document.body.removeChild(profilerContainer);
941+
});
658942
});

packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ function Profiler(_: {}) {
5454
supportsProfiling,
5555
startProfiling,
5656
stopProfiling,
57+
selectPrevCommitIndex,
58+
selectNextCommitIndex,
5759
} = useContext(ProfilerContext);
5860

5961
const {file: timelineTraceEventData, searchInputContainerRef} =
@@ -63,9 +65,9 @@ function Profiler(_: {}) {
6365

6466
const isLegacyProfilerSelected = selectedTabID !== 'timeline';
6567

66-
// Cmd+E to start/stop profiler recording
6768
const handleKeyDown = useEffectEvent((event: KeyboardEvent) => {
6869
const correctModifier = isMac ? event.metaKey : event.ctrlKey;
70+
// Cmd+E to start/stop profiler recording
6971
if (correctModifier && event.key === 'e') {
7072
if (isProfiling) {
7173
stopProfiling();
@@ -74,6 +76,24 @@ function Profiler(_: {}) {
7476
}
7577
event.preventDefault();
7678
event.stopPropagation();
79+
} else if (
80+
isLegacyProfilerSelected &&
81+
didRecordCommits &&
82+
selectedCommitIndex !== null
83+
) {
84+
// Cmd+Left/Right (Mac) or Ctrl+Left/Right (Windows/Linux) to navigate commits
85+
if (
86+
correctModifier &&
87+
(event.key === 'ArrowLeft' || event.key === 'ArrowRight')
88+
) {
89+
if (event.key === 'ArrowLeft') {
90+
selectPrevCommitIndex();
91+
} else {
92+
selectNextCommitIndex();
93+
}
94+
event.preventDefault();
95+
event.stopPropagation();
96+
}
7797
}
7898
});
7999

0 commit comments

Comments
 (0)