feat/timeline-scrubbing : Mouse now scrubs timeline.#437
feat/timeline-scrubbing : Mouse now scrubs timeline.#437webadderall merged 5 commits intowebadderallorg:mainfrom
Conversation
… intended from multiple video editor. scrubbing stops the playhead. as per usual editors
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughTimeline scrubbing now pauses playback and seeks via a unified handleSeek(time, {pause?}); timeline dragging and cursor dragging use rAF-throttled mouse tracking, TimelineAxis accepts an onMouseDown, background mousedown clears selection unless target is a timeline item, and items mark themselves with data-timeline-item. ChangesTimeline Seeking with RequestAnimationFrame Throttling
Sequence Diagram(s)sequenceDiagram
participant User
participant TimelineAxis
participant Timeline
participant VideoEditor.handleSeek
participant VideoElement
User->>TimelineAxis: mousedown / drag
TimelineAxis->>Timeline: onMouseDown -> handleTimelineMouseDown
Timeline->>VideoEditor.handleSeek: compute time, handleSeek(time, {pause:true})
VideoEditor.handleSeek->>VideoElement: pause() (if needed) then set currentTime
Timeline->>Timeline: register window mousemove/mouseup (rAF) for continuous seeks
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/video-editor/timeline/TimelineEditor.tsx (1)
326-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlush the last drag position on mouseup before tearing down the rAF loop.
Both new seek loops cancel the pending animation frame during cleanup. If the user releases quickly, the last
mousemovenever reachesonSeek, so scrubbing can land behind the cursor on release.Suggested fix pattern
Apply the same change to both handlers:
const handleMouseUp = () => { + if (frameId !== null) { + cancelAnimationFrame(frameId); + frameId = null; + } + if (lastMouseEvent) { + updateSeek(); + } setIsDragging(false); document.body.style.cursor = ""; };For the timeline-axis variant, mirror the same flush before
setIsSeeking(false).Also applies to: 723-763
🤖 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/video-editor/timeline/TimelineEditor.tsx` around lines 326 - 379, On mouseup we must flush the pending drag position so the last mousemove is applied before cancelling rAF; call updateSeek() (only if lastMouseEvent is set) from handleMouseUp before clearing frameId/cancelling animation and before calling setIsDragging(false) and resetting the cursor. Apply the same pattern to the timeline-axis mouseup handler: invoke the corresponding flush function (the same updateSeek or its axis-variant) before setIsSeeking(false) and before cancelling any pending frame.
🤖 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/video-editor/VideoEditor.tsx`:
- Around line 3277-3286: The handleSeek helper currently always pauses playback
(via videoPlaybackRef.current.pause()), causing non-scrub seeks to stop
playback; change handleSeek(time: number) to accept an optional flag like
pausePlayback: boolean = false, use that flag to conditionally call
playback.pause(), and keep setting currentTime via mapTimelineTimeToSourceTime;
update the timeline scrub caller to call handleSeek(time, true) while leaving
skip/jump button callers using the default (no flag) so they won't pause
playback.
---
Outside diff comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 326-379: On mouseup we must flush the pending drag position so the
last mousemove is applied before cancelling rAF; call updateSeek() (only if
lastMouseEvent is set) from handleMouseUp before clearing frameId/cancelling
animation and before calling setIsDragging(false) and resetting the cursor.
Apply the same pattern to the timeline-axis mouseup handler: invoke the
corresponding flush function (the same updateSeek or its axis-variant) before
setIsSeeking(false) and before cancelling any pending frame.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 86e68e5d-4f96-4f22-b6cc-557047335433
📒 Files selected for processing (2)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/TimelineEditor.tsx
There was a problem hiding this comment.
Pull request overview
Adds timeline “scrubbing” so users can drag along the timeline axis to seek precisely, with playback stopping while scrubbing.
Changes:
- Pause playback on seek so scrubbing doesn’t play audio and stops playback when currently playing (
VideoEditor.tsx). - Throttle seek updates during drag using
requestAnimationFrameto reduce seek spam (TimelineEditor.tsx). - Move seeking initiation to the timeline axis via
onMouseDown+ global mousemove tracking, and update cursor styling (TimelineEditor.tsx).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/components/video-editor/VideoEditor.tsx |
Pauses playback before applying seeks (used by timeline + other seek entrypoints). |
src/components/video-editor/timeline/TimelineEditor.tsx |
Implements axis-based scrubbing with rAF throttling and window-level drag tracking; adjusts axis props/styling. |
ExtraBinoss
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/video-editor/timeline/TimelineEditor.tsx (1)
722-765: 💤 Low valueClean drag-to-seek implementation.
The rAF throttling pattern is correctly implemented with proper cleanup.
One minor inconsistency:
PlaybackCursorsetsdocument.body.style.cursor = "ew-resize"during drag (line 374), but this effect doesn't. Since the TimelineAxis already hascursor-ew-resizeclass, this is likely fine, but if the user drags outside the axis bounds, the cursor might revert to default. Consider adding the same cursor override for visual consistency during drag.🤖 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/video-editor/timeline/TimelineEditor.tsx` around lines 722 - 765, When isSeeking is true the drag handler registered in useLayoutEffect doesn't set the global cursor style, so dragging outside the axis can revert the cursor; update the useLayoutEffect (the effect that registers handleMouseMove/handleMouseUp) to set document.body.style.cursor = "ew-resize" when isSeeking becomes true and restore the previous cursor on cleanup (reverse the change in the returned cleanup function or store the original cursor), ensuring you touch the same effect that uses setIsSeeking, localTimelineRef, pixelsToValue, and onSeek so the cursor override is applied for the full drag operation and removed when the effect unmounts.
🤖 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.
Nitpick comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 722-765: When isSeeking is true the drag handler registered in
useLayoutEffect doesn't set the global cursor style, so dragging outside the
axis can revert the cursor; update the useLayoutEffect (the effect that
registers handleMouseMove/handleMouseUp) to set document.body.style.cursor =
"ew-resize" when isSeeking becomes true and restore the previous cursor on
cleanup (reverse the change in the returned cleanup function or store the
original cursor), ensuring you touch the same effect that uses setIsSeeking,
localTimelineRef, pixelsToValue, and onSeek so the cursor override is applied
for the full drag operation and removed when the effect unmounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c4983700-6e54-4d2a-8cfa-079830d17b77
📒 Files selected for processing (2)
src/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/TimelineEditor.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/video-editor/timeline/Item.tsx
…o feat/timeline-scrubbing
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/video-editor/VideoEditor.tsx`:
- Around line 3424-3427: handleTimelineSeek is memoized with an empty deps array
and closes over handleSeek, causing a stale closure when
mapTimelineTimeToSourceTime (which depends on clipRegions) changes; update the
useCallback deps for handleTimelineSeek to include the changing references
(e.g., include handleSeek or mapTimelineTimeToSourceTime / clipRegions) so the
callback is recreated when timeline-to-source mapping changes, ensuring
handleTimelineSeek uses the latest mapping.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 28c99b3d-c0de-4535-9b33-7575b155c8a8
📒 Files selected for processing (2)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/TimelineEditor.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/video-editor/timeline/TimelineEditor.tsx
|
Triage pass on latest head |
Description
This PR introduces a scrubbing ability to the timeline. It means that if you are in the Timeline, you can drag around to search the perfect spot. If your video is playing, it stops. Sound is not played while scrubbing.
Keep in mind ;
Type of Change
Related Issue(s)
None
Screenshots / Video
New Recordly :
https://github.com/user-attachments/assets/eb7a5581-ee65-4052-9781-8df6101bde46
Capcut behavior :
https://github.com/user-attachments/assets/a2e3b631-296e-4b6c-ad7a-557117a0e6cb
Testing Guide
Checklist
Summary by CodeRabbit