feat(editor): add Full Camera timeline effect#66
Conversation
|
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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds camera fullscreen timeline regions across editor state, timeline controls, preview playback, GIF/MP4 rendering, keyboard shortcuts, and localized strings. ChangesCamera Fullscreen Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant VideoEditor
participant TimelineEditor
participant VideoPlayback
participant ProgressUtility
participant GeometryUtility
VideoEditor->>TimelineEditor: cameraFullscreenRegions and editing handlers
TimelineEditor-->>VideoEditor: add, update, select, or delete region
VideoEditor->>VideoPlayback: cameraFullscreenRegions
VideoPlayback->>ProgressUtility: timeMs and regions
ProgressUtility-->>VideoPlayback: fullscreen progress
VideoPlayback->>GeometryUtility: canvas size and webcam aspect
GeometryUtility-->>VideoPlayback: fullscreen target rectangle
VideoPlayback->>VideoPlayback: interpolate PiP rectangle to target
sequenceDiagram
participant VideoEditor
participant VideoExporter
participant GifExporter
participant FrameRenderer
participant ProgressUtility
participant GeometryUtility
VideoEditor->>VideoExporter: cameraFullscreenRegions
VideoEditor->>GifExporter: cameraFullscreenRegions
VideoExporter->>FrameRenderer: cameraFullscreenRegions
GifExporter->>FrameRenderer: cameraFullscreenRegions
FrameRenderer->>ProgressUtility: frame time and regions
ProgressUtility-->>FrameRenderer: fullscreen progress
FrameRenderer->>GeometryUtility: canvas size and webcam rectangle
GeometryUtility-->>FrameRenderer: fullscreen target rectangle
FrameRenderer->>FrameRenderer: interpolate webcam compositing
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
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/VideoEditor.tsx (1)
862-884: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
doNewProjectmisses the camera-fullscreen resets.This block resets every other selection and ID counter but omits the two new camera-fullscreen refs/setters added by this PR.
selectedCameraFullscreenIddoes self-heal via the effect at Lines 1807-1814 afterresetState(), andnextCameraFullscreenIdRefstaying above1only affects ID numbering (IDs remain unique), but this is inconsistent with the sibling counters/selections reset here.Proposed fix
setSelectedZoomId(null); + setSelectedCameraFullscreenId(null); setSelectedTrimId(null); setSelectedSpeedId(null); setSelectedAnnotationId(null); setSelectedBlurId(null);nextZoomIdRef.current = 1; + nextCameraFullscreenIdRef.current = 1; nextTrimIdRef.current = 1; nextSpeedIdRef.current = 1;🤖 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/VideoEditor.tsx` around lines 862 - 884, The doNewProject reset block in VideoEditor.tsx is missing the new camera-fullscreen state cleanup. Update the same reset sequence that clears selection state and reinitializes counters to also clear selectedCameraFullscreenId and reset nextCameraFullscreenIdRef to its initial value, matching the pattern used for the other selected IDs and next*Id refs in doNewProject.
🧹 Nitpick comments (1)
src/i18n/locales/tr/timeline.json (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent capitalization in
addBlurstring.
"addBlur": "Bulanık ekle (B)"uses lowercase "ekle" while every other sibling button capitalizes it, e.g."addZoom": "Yakınlaştırma Ekle (Z)","addCameraFullscreen": "Tam Ekran Kamera Ekle (C)". Worth aligning for consistency.✏️ Suggested fix
- "addBlur": "Bulanık ekle (B)", + "addBlur": "Bulanık Ekle (B)",🤖 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/i18n/locales/tr/timeline.json` at line 8, The addBlur localization string in the timeline translations uses inconsistent capitalization compared with sibling actions; update the value in the addBlur entry to match the title-style capitalization used by addZoom and addCameraFullscreen. Keep the existing meaning and hotkey, but make the action phrase consistent with the other button labels in this locale file.
🤖 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.
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 862-884: The doNewProject reset block in VideoEditor.tsx is
missing the new camera-fullscreen state cleanup. Update the same reset sequence
that clears selection state and reinitializes counters to also clear
selectedCameraFullscreenId and reset nextCameraFullscreenIdRef to its initial
value, matching the pattern used for the other selected IDs and next*Id refs in
doNewProject.
---
Nitpick comments:
In `@src/i18n/locales/tr/timeline.json`:
- Line 8: The addBlur localization string in the timeline translations uses
inconsistent capitalization compared with sibling actions; update the value in
the addBlur entry to match the title-style capitalization used by addZoom and
addCameraFullscreen. Keep the existing meaning and hotkey, but make the action
phrase consistent with the other button labels in this locale file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64d5b82b-4b63-4bdf-8a26-810303d67efd
📒 Files selected for processing (42)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/ItemGlass.module.csssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/cameraFullscreenUtils.test.tssrc/components/video-editor/videoPlayback/cameraFullscreenUtils.tssrc/hooks/useEditorHistory.tssrc/i18n/locales/ar/shortcuts.jsonsrc/i18n/locales/ar/timeline.jsonsrc/i18n/locales/en/shortcuts.jsonsrc/i18n/locales/en/timeline.jsonsrc/i18n/locales/es/shortcuts.jsonsrc/i18n/locales/es/timeline.jsonsrc/i18n/locales/fr/shortcuts.jsonsrc/i18n/locales/fr/timeline.jsonsrc/i18n/locales/it/shortcuts.jsonsrc/i18n/locales/it/timeline.jsonsrc/i18n/locales/ja-JP/shortcuts.jsonsrc/i18n/locales/ja-JP/timeline.jsonsrc/i18n/locales/ko-KR/shortcuts.jsonsrc/i18n/locales/ko-KR/timeline.jsonsrc/i18n/locales/pt-BR/shortcuts.jsonsrc/i18n/locales/pt-BR/timeline.jsonsrc/i18n/locales/ru/shortcuts.jsonsrc/i18n/locales/ru/timeline.jsonsrc/i18n/locales/tr/shortcuts.jsonsrc/i18n/locales/tr/timeline.jsonsrc/i18n/locales/vi/shortcuts.jsonsrc/i18n/locales/vi/timeline.jsonsrc/i18n/locales/zh-CN/shortcuts.jsonsrc/i18n/locales/zh-CN/timeline.jsonsrc/i18n/locales/zh-TW/shortcuts.jsonsrc/i18n/locales/zh-TW/timeline.jsonsrc/lib/compositeLayout.test.tssrc/lib/compositeLayout.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.tssrc/lib/shortcuts.ts
|
Addressed the review: added the missing camera-fullscreen selection/id-counter resets in |
|
@rodrvc thanks for you PR, I think this is a good idea. |
EtienneLescot
left a comment
EtienneLescot
left a comment
There was a problem hiding this comment.
Thank you for the contribution! The implementation of the Full Camera effect is clean, structured, and consistent with other regions. I ran the typechecker and all unit tests locally, and they passed successfully.
I have left one inline comment regarding the transition easing calculation for the ease-out effect to avoid a visual pop/slam at the end of the transition.
|
Thanks for the review! Fixed the easing issue on the inline comment exactly as you suggested ( Re: platforms — I only tested this manually on macOS. Full Camera itself is renderer-only (canvas/PIXI math for the easing envelope and target-rect, consumed by both the DOM preview and the frame renderer for MP4/GIF export); it doesn't touch any of the native platform helpers (WGC/WASAPI on Windows, ScreenCaptureKit on macOS), which are only involved in live capture, not in this edit/export pipeline. |
|
@rodrvc Thank you. Thank you for your contribution! |
Adds a new timeline row for Full Camera segments: during a segment the webcam overlay grows to (almost) fill the canvas — keeping its aspect ratio and a small margin so rounded corners stay visible — then eases back to its normal layout, with a slower ease-out than ease-in. The transition is fully contained within the segment bounds. Follows the existing ZoomRegion pattern end to end: shared pure interpolation math for preview and export parity, persistence with normalization for older projects, undo/redo support, a configurable keyboard shortcut (C), and i18n keys for all 13 locales. While a Full Camera segment is active, the reactive webcam shrink is suppressed to avoid conflicting transforms; zoom behavior itself is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…project Address review feedback: doNewProject reset every other selection and region id counter but missed the two camera-fullscreen ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
EtienneLescot
force-pushed
the
feature/camera-animation
branch
from
c19c8a1 to
28a2a8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/compositeLayout.test.ts (1)
278-305: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect swapped dimension terms in test descriptions.
The terms "width" and "height" are reversed in some test descriptions and comments regarding which dimension acts as the limiting constraint. For example, fitting a portrait or square source into a landscape canvas hits the top/bottom edges first, making height the constrained dimension.
Consider fixing these to ensure the documentation accurately reflects the geometry logic.
📝 Proposed fixes
- // Square aspect source so width is the constrained dimension in a landscape canvas. + // Square aspect source so height is the constrained dimension in a landscape canvas. const rect = computeCameraFullscreenTargetRect(canvasSize, { width: 1080, height: 1080 });- it("fits a portrait source by constraining width, centering vertically and horizontally", () => { + it("fits a portrait source by constraining height, centering vertically and horizontally", () => { const canvasSize = { width: 1920, height: 1080 };- it("fits a landscape source by constraining height when it would overflow", () => { + it("fits a landscape source by constraining width when it would overflow", () => { const canvasSize = { width: 1080, height: 1920 }; // portrait canvas🤖 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/lib/compositeLayout.test.ts` around lines 278 - 305, Correct the dimension terminology in the descriptions and comments around computeCameraFullscreenTargetRect tests. For the square and portrait source cases, describe height as the constrained dimension because the source reaches the canvas’s top and bottom edges first; update the portrait test title accordingly. Review the adjacent landscape test wording and ensure its constraint description matches the actual geometry.
🤖 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/timeline/TimelineEditor.tsx`:
- Around line 1086-1089: Move the four ref assignments around zoomRegionsRef,
cameraFullscreenRegionsRef, trimRegionsRef, and speedRegionsRef into a useEffect
with the corresponding region values as dependencies. Define this
synchronization effect before the existing normalization useEffect so that
normalization reads the latest refs while keeping render pure.
In `@src/i18n/locales/fr/shortcuts.json`:
- Line 25: Update the addCameraFullscreen translation value in shortcuts.json to
use natural French grammar, including the article and “en plein écran” phrasing,
while leaving the translation key unchanged.
---
Nitpick comments:
In `@src/lib/compositeLayout.test.ts`:
- Around line 278-305: Correct the dimension terminology in the descriptions and
comments around computeCameraFullscreenTargetRect tests. For the square and
portrait source cases, describe height as the constrained dimension because the
source reaches the canvas’s top and bottom edges first; update the portrait test
title accordingly. Review the adjacent landscape test wording and ensure its
constraint description matches the actual geometry.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 54826504-6a30-4c87-810f-0f2cc36ea7f4
📒 Files selected for processing (42)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/ItemGlass.module.csssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/cameraFullscreenUtils.test.tssrc/components/video-editor/videoPlayback/cameraFullscreenUtils.tssrc/hooks/useEditorHistory.tssrc/i18n/locales/ar/shortcuts.jsonsrc/i18n/locales/ar/timeline.jsonsrc/i18n/locales/en/shortcuts.jsonsrc/i18n/locales/en/timeline.jsonsrc/i18n/locales/es/shortcuts.jsonsrc/i18n/locales/es/timeline.jsonsrc/i18n/locales/fr/shortcuts.jsonsrc/i18n/locales/fr/timeline.jsonsrc/i18n/locales/it/shortcuts.jsonsrc/i18n/locales/it/timeline.jsonsrc/i18n/locales/ja-JP/shortcuts.jsonsrc/i18n/locales/ja-JP/timeline.jsonsrc/i18n/locales/ko-KR/shortcuts.jsonsrc/i18n/locales/ko-KR/timeline.jsonsrc/i18n/locales/pt-BR/shortcuts.jsonsrc/i18n/locales/pt-BR/timeline.jsonsrc/i18n/locales/ru/shortcuts.jsonsrc/i18n/locales/ru/timeline.jsonsrc/i18n/locales/tr/shortcuts.jsonsrc/i18n/locales/tr/timeline.jsonsrc/i18n/locales/vi/shortcuts.jsonsrc/i18n/locales/vi/timeline.jsonsrc/i18n/locales/zh-CN/shortcuts.jsonsrc/i18n/locales/zh-CN/timeline.jsonsrc/i18n/locales/zh-TW/shortcuts.jsonsrc/i18n/locales/zh-TW/timeline.jsonsrc/lib/compositeLayout.test.tssrc/lib/compositeLayout.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.tssrc/lib/shortcuts.ts
🚧 Files skipped from review as they are similar to previous changes (35)
- src/i18n/locales/es/shortcuts.json
- src/i18n/locales/en/shortcuts.json
- src/i18n/locales/zh-TW/shortcuts.json
- src/i18n/locales/tr/timeline.json
- src/i18n/locales/vi/shortcuts.json
- src/i18n/locales/tr/shortcuts.json
- src/lib/compositeLayout.ts
- src/i18n/locales/vi/timeline.json
- src/components/video-editor/projectPersistence.ts
- src/lib/shortcuts.ts
- src/i18n/locales/es/timeline.json
- src/components/video-editor/videoPlayback/cameraFullscreenUtils.ts
- src/i18n/locales/ar/shortcuts.json
- src/i18n/locales/ja-JP/shortcuts.json
- src/lib/exporter/videoExporter.ts
- src/components/video-editor/timeline/Item.tsx
- src/components/video-editor/VideoPlayback.tsx
- src/i18n/locales/it/shortcuts.json
- src/i18n/locales/pt-BR/timeline.json
- src/i18n/locales/ko-KR/timeline.json
- src/i18n/locales/ko-KR/shortcuts.json
- src/lib/exporter/frameRenderer.ts
- src/i18n/locales/en/timeline.json
- src/i18n/locales/zh-CN/shortcuts.json
- src/i18n/locales/it/timeline.json
- src/components/video-editor/videoPlayback/cameraFullscreenUtils.test.ts
- src/i18n/locales/zh-CN/timeline.json
- src/i18n/locales/zh-TW/timeline.json
- src/hooks/useEditorHistory.ts
- src/components/video-editor/timeline/ItemGlass.module.css
- src/i18n/locales/ar/timeline.json
- src/i18n/locales/ja-JP/timeline.json
- src/i18n/locales/ru/timeline.json
- src/i18n/locales/fr/timeline.json
- src/components/video-editor/VideoEditor.tsx
- Sync region refs in an effect instead of during render (React purity / no-ref-current-in-render); ordered before the normalization effect so it still reads the freshest values. - Fix fr shortcut label grammar: "Ajouter une caméra en plein écran". - Correct swapped width/height wording in computeCameraFullscreenTargetRect test descriptions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
Adds a Full Camera timeline effect: a new timeline row where you can add segments (button or
Cshortcut) during which the webcam overlay smoothly grows to cover the canvas — a "cut to camera" moment — and then eases back to its normal position.Design decisions, all following the existing
ZoomRegionpattern 1:1 (no new abstractions):CameraFullscreenRegion { id, startMs, endMs }, edited on the timeline exactly like zoom segments (drag to move, drag edges to resize, same overlap guards).startMsandendMs; ease-out is intentionally slower (1.5x) than ease-in for a gentler return.cameraFullscreenUtils.ts,computeCameraFullscreenTargetRect) consumed by both the DOM preview and the frame renderer used for MP4/GIF export.C), and i18n keys for all 13 locales.Related issue
None — happy to open one first if you prefer tracking features via issues.
Type of change
Release impact
Desktop impact
Screenshots / video
The webcam grows from its PiP position to an aspect-ratio-preserving, near-fullscreen rect with a small margin, then eases back. (Can attach a short clip on request.)
Testing
npx tsc --noEmit,npm run lint,npm run i18n:checkclean (i18n failures pre-existing at HEAD are untouched).npm run test: 258 tests passing, including new unit tests for the easing envelope (containment, monotonicity, short-segment clamping, overlapping regions) and the target-rect math (aspect preservation, margin, portrait/landscape).Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests