Allow splitting zoom and scene segments #1222
Conversation
WalkthroughThe PR introduces segment-splitting functionality across the editor timeline by adding Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TrackUI as Track Component
participant Context as Editor Context
participant State as Timeline State
rect rgb(240, 248, 255)
Note over User,State: Split Segment Flow (interactMode = "split")
User->>TrackUI: mouseDown on segment
TrackUI->>TrackUI: Compute splitTime from click position
TrackUI->>Context: projectActions.splitSegment(index, splitTime)
Context->>State: Insert new segment at index+1<br/>Update boundaries of both segments
State-->>Context: Segments array updated
Context-->>TrackUI: Listener notifies of state change
TrackUI->>TrackUI: Re-render with new segments
end
rect rgb(255, 250, 240)
Note over User,State: Normal Seek Mode (interactMode = "seek")
User->>TrackUI: mouseDown on segment
TrackUI->>TrackUI: Check interactMode guard
alt interactMode === "seek"
TrackUI->>TrackUI: Execute drag behavior
else
TrackUI->>TrackUI: No-op (guard blocks)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Multiple files with mixed complexity: context API additions with validation logic, consistent split-handler patterns across tracks with position-to-time computation, and refactored memo logic in ConfigSidebar. Heterogeneous changes demand separate reasoning per track implementation, though the underlying pattern repeats. Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/context.ts (1)
76-105: Add minimum length validation to splitClipSegment.Unlike
splitZoomSegment(line 140) andsplitSceneSegment(line 179),splitClipSegmentlacks validation to ensure both resulting segments meet a minimum length requirement. This could allow creating zero-length or very small segments if the split time is at or very close to a boundary.Consider adding similar validation:
splitClipSegment: (time: number) => { setProject( "timeline", "segments", produce((segments) => { let searchTime = time; let prevDuration = 0; const currentSegmentIndex = segments.findIndex((segment) => { const duration = segment.end - segment.start; if (searchTime > duration) { searchTime -= duration; prevDuration += duration; return false; } return true; }); if (currentSegmentIndex === -1) return; const segment = segments[currentSegmentIndex]; + const newLengths = [searchTime, segment.end - segment.start - searchTime]; + if (newLengths.some((l) => l < 1)) return; segments.splice(currentSegmentIndex + 1, 0, { ...segment, start: segment.start + searchTime, end: segment.end, }); segments[currentSegmentIndex].end = segment.start + searchTime; }), ); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/routes/editor/ConfigSidebar.tsx(1 hunks)apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx(0 hunks)apps/desktop/src/routes/editor/Timeline/SceneTrack.tsx(3 hunks)apps/desktop/src/routes/editor/Timeline/ZoomTrack.tsx(3 hunks)apps/desktop/src/routes/editor/context.ts(3 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Timeline/ZoomTrack.tsxapps/desktop/src/routes/editor/context.tsapps/desktop/src/routes/editor/Timeline/SceneTrack.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Timeline/ZoomTrack.tsxapps/desktop/src/routes/editor/context.tsapps/desktop/src/routes/editor/Timeline/SceneTrack.tsx
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/src/**/*.{ts,tsx}: Desktop icons are auto-imported (unplugin-icons); do not import icons manually
Desktop IPC: Call generated tauri_specta commands/events; listen to generated events and use typed interfaces
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Timeline/ZoomTrack.tsxapps/desktop/src/routes/editor/context.tsapps/desktop/src/routes/editor/Timeline/SceneTrack.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (11)
apps/desktop/src/routes/editor/Timeline/ZoomTrack.tsx (3)
44-44: LGTM: projectActions now available for split operations.The addition of
projectActionsto the destructured context enables the new split functionality.
295-296: LGTM: Guard correctly prevents dragging during split mode.The early return when
interactMode !== "seek"ensures drag operations are disabled in split mode, which is the intended behavior.
426-437: LGTM: Split handler correctly calculates relative split time.The implementation correctly:
- Calculates click position as a fraction within the segment
- Derives
splitTimerelative to the segment's duration- Calls
splitZoomSegmentwith the appropriate index and timeThe backend validation in
splitZoomSegment(line 140 in context.ts) ensures both resulting segments meet minimum length requirements.apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
916-919: LGTM: Cleaner segment resolution.The refactor from
segmentIndextoclipSegmentdirectly finds the segment containing the current time, eliminating the intermediate index lookup. The boundary checks.start <= st && s.end > stcorrectly identifies the containing segment.
921-935: LGTM: Consistent use of the new clipSegment memo.Both
relativeTimeand the video path construction correctly useclipSegment()with appropriate optional chaining and fallbacks. The TODO comment on line 930 refers to the hardcoded path structure (/content/segments/segment-X/display.mp4), not the now-dynamicrecordingSegmentindex resolution.apps/desktop/src/routes/editor/Timeline/SceneTrack.tsx (4)
34-41: LGTM: projectActions added to support split operations.Consistent with the changes in ZoomTrack,
projectActionsis now available to enable the split functionality for scene segments.
209-209: LGTM: Defensive null-coalescing added.The
?? []fallback ensuressceneSegments()always returns an array, preventing potential runtime errors whenproject.timeline?.sceneSegmentsis undefined.
216-217: LGTM: Guard prevents dragging during split mode.Consistent with ZoomTrack, this guard ensures drag operations are disabled when in split mode.
309-320: LGTM: Split handler mirrors ZoomTrack implementation.The split handler for scene segments follows the same pattern as zoom segments:
- Calculates click position as a fraction
- Derives relative
splitTime- Calls
splitSceneSegmentwith validation in place (line 179 in context.ts)apps/desktop/src/routes/editor/context.ts (2)
130-150: LGTM: splitZoomSegment correctly validates minimum segment lengths.The implementation properly:
- Validates that both resulting segments have length ≥ 1 second (line 140)
- Creates the new segment preserving all properties via spread
- Updates boundaries correctly
169-189: LGTM: splitSceneSegment mirrors splitZoomSegment with proper validation.The implementation follows the same validated pattern as
splitZoomSegment, ensuring both resulting segments meet the 1-second minimum requirement.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements