feat: add playback speed controls and looping to TimeTravelTimeline#2458
feat: add playback speed controls and looping to TimeTravelTimeline#245825032007 wants to merge 1 commit into
Conversation
|
@25032007 is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel. A member of the Team first needs to authorize it. |
🎉 Thanks for your contribution, @25032007!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
📝 WalkthroughWalkthrough
ChangesConfigurable Playback Speed and Looping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎉 Thanks for your contribution, @25032007!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/repository/TimeTravelTimeline.tsx`:
- Around line 133-146: The loop toggle button (containing the Repeat icon) and
playback speed select element rely only on title attributes for accessibility,
which is insufficient for screen reader users. Add aria-label attributes to both
controls to explicitly describe their purpose, and add an aria-pressed attribute
to the loop button that reflects the current isLooping state (true when
isLooping is true, false otherwise). This ensures assistive technologies can
properly communicate the control's function and current state to users.
- Around line 63-75: The playback logic in the interval callback does not guard
against empty commit lists when looping is enabled. When isLooping is true and
nextIndex is set to 0, the code proceeds to access
chronologicalCommits[nextIndex].hash without verifying that chronologicalCommits
has any elements, which can cause a runtime crash if the list is empty. Add a
guard check to ensure chronologicalCommits is not empty before attempting to
access chronologicalCommits[nextIndex].hash in the onCommitSelect call. If the
list is empty while looping, either stop playback or skip the selection call to
prevent undefined access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00a2524f-d8ed-43a6-a758-e1b3fcf71171
📒 Files selected for processing (1)
src/components/repository/TimeTravelTimeline.tsx
| if (isLooping) { | ||
| nextIndex = 0; | ||
| } else { | ||
| setIsPlaying(false); | ||
| return; | ||
| } | ||
| } | ||
| if (nextIndex === chronologicalCommits.length - 1) { | ||
| onCommitSelect(null); | ||
| } else { | ||
| onCommitSelect(chronologicalCommits[nextIndex].hash); | ||
| } | ||
| }, 1000); // 1 second per step | ||
| }, 1000 / playbackSpeed); // dynamic speed |
There was a problem hiding this comment.
Guard loop playback against empty commit lists to avoid runtime crashes.
If commits becomes empty while playback is active and looping is enabled, this path can still execute and access chronologicalCommits[0].hash, which is undefined and can throw.
Proposed fix
useEffect(() => {
+ if (chronologicalCommits.length === 0) {
+ setIsPlaying(false);
+ return;
+ }
+
if (isPlaying) {
playIntervalRef.current = setInterval(() => {
let nextIndex = currentIndex + 1;
if (nextIndex >= chronologicalCommits.length) {
if (isLooping) {
nextIndex = 0;
} else {
setIsPlaying(false);
return;
}
}
if (nextIndex === chronologicalCommits.length - 1) {
onCommitSelect(null);
} else {
onCommitSelect(chronologicalCommits[nextIndex].hash);
}
}, 1000 / playbackSpeed); // dynamic speed🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/repository/TimeTravelTimeline.tsx` around lines 63 - 75, The
playback logic in the interval callback does not guard against empty commit
lists when looping is enabled. When isLooping is true and nextIndex is set to 0,
the code proceeds to access chronologicalCommits[nextIndex].hash without
verifying that chronologicalCommits has any elements, which can cause a runtime
crash if the list is empty. Add a guard check to ensure chronologicalCommits is
not empty before attempting to access chronologicalCommits[nextIndex].hash in
the onCommitSelect call. If the list is empty while looping, either stop
playback or skip the selection call to prevent undefined access.
| <button | ||
| onClick={() => setIsLooping(!isLooping)} | ||
| className={`p-2 rounded-full transition-colors ${isLooping ? 'bg-primary/20 text-primary' : 'hover:bg-white/10 text-muted-foreground hover:text-white'}`} | ||
| title="Toggle Loop" | ||
| > | ||
| <Repeat className="h-4 w-4" /> | ||
| </button> | ||
|
|
||
| <select | ||
| value={playbackSpeed} | ||
| onChange={(e) => setPlaybackSpeed(Number(e.target.value))} | ||
| className="bg-transparent text-xs text-muted-foreground hover:text-white cursor-pointer outline-none border border-white/10 rounded px-1 py-1" | ||
| title="Playback Speed" | ||
| > |
There was a problem hiding this comment.
Add explicit accessibility labels to new playback controls.
The new icon-only loop button and speed select should expose explicit labels/state (aria-label, aria-pressed) instead of relying on title.
Proposed fix
<button
onClick={() => setIsLooping(!isLooping)}
+ aria-label="Toggle loop playback"
+ aria-pressed={isLooping}
className={`p-2 rounded-full transition-colors ${isLooping ? 'bg-primary/20 text-primary' : 'hover:bg-white/10 text-muted-foreground hover:text-white'}`}
title="Toggle Loop"
>
<Repeat className="h-4 w-4" />
</button>
<select
value={playbackSpeed}
onChange={(e) => setPlaybackSpeed(Number(e.target.value))}
+ aria-label="Playback speed"
className="bg-transparent text-xs text-muted-foreground hover:text-white cursor-pointer outline-none border border-white/10 rounded px-1 py-1"
title="Playback Speed"
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/repository/TimeTravelTimeline.tsx` around lines 133 - 146, The
loop toggle button (containing the Repeat icon) and playback speed select
element rely only on title attributes for accessibility, which is insufficient
for screen reader users. Add aria-label attributes to both controls to
explicitly describe their purpose, and add an aria-pressed attribute to the loop
button that reflects the current isLooping state (true when isLooping is true,
false otherwise). This ensures assistive technologies can properly communicate
the control's function and current state to users.
Description
This pull request implements user-configurable playback controls for the
TimeTravelTimelinecomponent. It introduces a dropdown to dynamically adjust the playback speed multiplier (0.5x, 1x, 2x, 5x) and adds a toggle button to enable auto-looping. When looping is enabled, the timeline automatically restarts from the oldest commit once the latest state is reached, rather than just pausing.Related Issue
Closes #2453
Type of Change
Screenshots
N/A
Testing
Verified the manual selection of speeds, UI layout of new playback controls using
lucide-reacticons, and loop functionality behavior.npm run lintnpm run buildnpm run formatgit diff --checkN/Afor documentation-only changesN/Awith a reason (N/A - UI only changes without business logic changes)Checklist
Summary by CodeRabbit
Release Notes