feat: cursor follow crop with text cursor focus mode#640
Open
walters954 wants to merge 3 commits into
Open
Conversation
Contributor
|
|
Contributor
📝 WalkthroughWalkthroughAdds a cursor-follow crop mode for video editors: type definitions and defaults, per-frame crop computation from cursor telemetry with text-zoom engagement layer, CropControl UI controls and updated drag behavior, VideoPlayback ticker integration, VideoEditor state threading and export wiring, project persistence and normalization, export renderer application in both standard and modern pipelines, exporter config updates, comprehensive tests, and related waveform decode and media content-type improvements. ChangesCursor-follow crop feature
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
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 |
Contributor
|
|
Adds a "Track cursor" crop mode that pans the viewport to keep the mouse cursor inside a configurable safe zone during playback and export. A new "Text cursor focus" toggle locks the viewport over the typing area when the I-beam cursor is active and the mouse is stationary, then smoothly switches back to mouse tracking once movement is detected. Mouse always wins; a 700ms debounce prevents jarring transitions between modes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
walters954
force-pushed
the
feature/cursor-follow-crop
branch
from
2bfe0d4 to
75ee15c
Compare
Contributor
|
|
1 similar comment
Contributor
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/video-editor/VideoEditor.tsx (1)
1078-1104:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
cursorFollowCropto the thumbnail callback deps.
captureProjectThumbnail()now closes overcursorFollowCrophere, but theuseCallbackdependency list below doesn't include it. Saving right after changing Track cursor can therefore bake the previous cursor-follow config into the project thumbnail.💡 Minimal fix
}, [ annotationRegions, autoCaptionSettings, autoCaptions, backgroundBlur, borderRadius, connectZooms, connectedZoomDurationMs, connectedZoomEasing, connectedZoomGapMs, cropRegion, + cursorFollowCrop, currentTime, cursorClickBounce,🤖 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 1078 - 1104, The captureProjectThumbnail callback in VideoEditor closes over cursorFollowCrop but its useCallback dependency list is missing that variable; update the useCallback that defines captureProjectThumbnail to include cursorFollowCrop (alongside the other deps such as wallpaper, zoomRegions, targetWidth/Height, etc.) so the thumbnail logic uses the current cursor-follow setting when saved.src/lib/exporter/gifExporter.ts (1)
143-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForward
cursorFollowCropinto the GIF frame renderer config.
GifExporterConfigaccepts the new setting, butbuildGifFrameRendererConfig()drops it. As written, GIF exports won't apply Track cursor even though preview and MP4 exports do.Suggested fix
borderRadius: config.borderRadius, padding: config.padding, cropRegion: config.cropRegion, + cursorFollowCrop: config.cursorFollowCrop, webcam: config.webcam,🤖 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/exporter/gifExporter.ts` around lines 143 - 201, The buildGifFrameRendererConfig() return object is missing the cursorFollowCrop option from GifExporterConfig, so GIFs ignore Track cursor; update the object returned by buildGifFrameRendererConfig to include cursorFollowCrop: config.cursorFollowCrop (ensuring the key matches the renderer's expected name) so the GIF frame renderer receives and applies the setting.
🧹 Nitpick comments (1)
src/components/video-editor/VideoPlayback.tsx (1)
1551-1554: 💤 Low valueConsider splitting ref update and state reset into separate effects.
The current effect updates
cursorFollowCropRef.currentand resets state, but the dependency array only includes specific properties. IfcursorFollowCropobject reference changes but these properties remain the same, the ref won't be updated. While this works functionally (since the properties are identical), it could be clearer to separate concerns:+useEffect(() => { + cursorFollowCropRef.current = cursorFollowCrop; +}, [cursorFollowCrop]); + useEffect(() => { - cursorFollowCropRef.current = cursorFollowCrop; resetCursorFollowCropState(cursorFollowCropStateRef.current); }, [cursorFollowCrop?.enabled, cursorFollowCrop?.safeZoneRatio, cursorFollowCrop?.smoothness, cursorFollowCrop?.trackTextCursor]);This makes the intent clearer: always keep the ref synced, but only reset state when computation-affecting properties change.
🤖 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/VideoPlayback.tsx` around lines 1551 - 1554, Split the current useEffect into two: one effect that always syncs the ref by assigning cursorFollowCropRef.current = cursorFollowCrop and depends on the cursorFollowCrop object reference (so it runs whenever the object changes), and a separate effect that calls resetCursorFollowCropState(cursorFollowCropStateRef.current) with dependencies only on the computation-affecting properties (cursorFollowCrop?.enabled, cursorFollowCrop?.safeZoneRatio, cursorFollowCrop?.smoothness, cursorFollowCrop?.trackTextCursor); keep references to resetCursorFollowCropState, cursorFollowCropRef, and cursorFollowCropStateRef as-is to locate the code.
🤖 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/CropControl.tsx`:
- Around line 115-118: The preview is being stretched because
canvas.width/canvas.height are always set to sourceWidth/sourceHeight; when in
output mode use the cropped region's dimensions instead of the full source to
preserve aspect ratio—compute the targetWidth/targetHeight from the current crop
rectangle (e.g., cropRect.width/cropRect.height or cropWidth/cropHeight) or
scale them to the configured output resolution, and set canvas.width and
canvas.height to those values before drawing; update the same logic where
sourceWidth/sourceHeight are used (the blocks around canvas.width/canvas.height
at the given ranges) so the canvas matches the crop/output aspect rather than
the full video.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 6357-6358: The crop modal is being passed raw cursorTelemetry
which can differ from the normalized/loop-aware path used by VideoPlayback and
export; change the prop passed to the crop editor from cursorTelemetry to the
already-computed effectiveCursorTelemetry (the same value used by
VideoPlayback/export) so the crop preview uses the identical telemetry variant;
locate the prop usage in VideoEditor (prop name cursorTelemetry/currentTimeMs)
and replace it to supply effectiveCursorTelemetry (or call the helper that
computes it) so preview/export paths match.
- Around line 6355-6356: The cancel path only restores cropRegion but not the
mutable cursor-follow state, so when the crop modal is opened you should
snapshot the current cursorFollowCrop (e.g., prevCursorFollowCrop) and any
related fields (safe zone, smoothness) and then in handleCancelCropEditor()
restore cursorFollowCrop via setCursorFollowCrop(prevCursorFollowCrop) (and
clear the snapshot after). Add the snapshot creation where the editor opens and
use the snapshot in handleCancelCropEditor to roll back cursorFollowCrop
changes.
In `@src/lib/exporter/modernFrameRenderer.ts`:
- Line 2984: The call to applyCursorFollowCrop(timeMs, layoutCache) uses
source/output timeMs when sampling cursor-follow crop but cursor sampling must
use the cursor timeline; update applyCursorFollowCrop (and any callers at the
other site) to accept and pass the timeline-aligned cursor time (cursorTimeMs)
or convert timeMs to cursorTimeMs via the same remapping used by
cursorOverlay.update before calling computeCursorFollowCrop so the crop sampling
uses cursorTimeMs (ensure references to applyCursorFollowCrop,
computeCursorFollowCrop, and cursorOverlay.update are updated accordingly).
---
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1078-1104: The captureProjectThumbnail callback in VideoEditor
closes over cursorFollowCrop but its useCallback dependency list is missing that
variable; update the useCallback that defines captureProjectThumbnail to include
cursorFollowCrop (alongside the other deps such as wallpaper, zoomRegions,
targetWidth/Height, etc.) so the thumbnail logic uses the current cursor-follow
setting when saved.
In `@src/lib/exporter/gifExporter.ts`:
- Around line 143-201: The buildGifFrameRendererConfig() return object is
missing the cursorFollowCrop option from GifExporterConfig, so GIFs ignore Track
cursor; update the object returned by buildGifFrameRendererConfig to include
cursorFollowCrop: config.cursorFollowCrop (ensuring the key matches the
renderer's expected name) so the GIF frame renderer receives and applies the
setting.
---
Nitpick comments:
In `@src/components/video-editor/VideoPlayback.tsx`:
- Around line 1551-1554: Split the current useEffect into two: one effect that
always syncs the ref by assigning cursorFollowCropRef.current = cursorFollowCrop
and depends on the cursorFollowCrop object reference (so it runs whenever the
object changes), and a separate effect that calls
resetCursorFollowCropState(cursorFollowCropStateRef.current) with dependencies
only on the computation-affecting properties (cursorFollowCrop?.enabled,
cursorFollowCrop?.safeZoneRatio, cursorFollowCrop?.smoothness,
cursorFollowCrop?.trackTextCursor); keep references to
resetCursorFollowCropState, cursorFollowCropRef, and cursorFollowCropStateRef
as-is to locate the code.
🪄 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: 735219a8-1b54-4121-be56-645a82bfc981
📒 Files selected for processing (12)
src/components/video-editor/CropControl.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/cursorFollowCrop.test.tssrc/components/video-editor/videoPlayback/cursorFollowCrop.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/modernFrameRenderer.tssrc/lib/exporter/modernVideoExporter.tssrc/lib/exporter/videoExporter.ts
Comment on lines
+115
to
+118
| const sourceWidth = videoElement.videoWidth || 1920; | ||
| const sourceHeight = videoElement.videoHeight || 1080; | ||
| canvas.width = sourceWidth; | ||
| canvas.height = sourceHeight; |
Contributor
There was a problem hiding this comment.
Don't stretch the output preview to the source aspect ratio.
In output mode the cropped region is still rendered into a full-source canvas and wrapped in a container sized from the full video. Any crop whose aspect differs from the source gets geometrically distorted, so the new Output preview can show the wrong framing.
Also applies to: 130-137, 271-276
🤖 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/CropControl.tsx` around lines 115 - 118, The
preview is being stretched because canvas.width/canvas.height are always set to
sourceWidth/sourceHeight; when in output mode use the cropped region's
dimensions instead of the full source to preserve aspect ratio—compute the
targetWidth/targetHeight from the current crop rectangle (e.g.,
cropRect.width/cropRect.height or cropWidth/cropHeight) or scale them to the
configured output resolution, and set canvas.width and canvas.height to those
values before drawing; update the same logic where sourceWidth/sourceHeight are
used (the blocks around canvas.width/canvas.height at the given ranges) so the
canvas matches the crop/output aspect rather than the full video.
Comment on lines
+6355
to
+6356
| cursorFollow={cursorFollowCrop} | ||
| onCursorFollowChange={setCursorFollowCrop} |
Contributor
There was a problem hiding this comment.
Cancel should roll back cursor-follow edits too.
These props let the crop modal mutate cursorFollowCrop immediately, but handleCancelCropEditor() only restores cropRegion. If the user toggles Track cursor or changes safe zone/smoothness and then cancels, those edits still stick.
💡 Minimal fix
+ const cursorFollowSnapshotRef = useRef<CursorFollowCropSettings | null>(null);
+
const handleOpenCropEditor = useCallback(() => {
cropSnapshotRef.current = { ...cropRegion };
+ cursorFollowSnapshotRef.current = { ...cursorFollowCrop };
setShowCropModal(true);
- }, [cropRegion]);
+ }, [cropRegion, cursorFollowCrop]);
const handleCancelCropEditor = useCallback(() => {
if (cropSnapshotRef.current) {
setCropRegion(cropSnapshotRef.current);
}
+ if (cursorFollowSnapshotRef.current) {
+ setCursorFollowCrop(cursorFollowSnapshotRef.current);
+ }
setShowCropModal(false);
- }, []);
+ }, []);🤖 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 6355 - 6356, The
cancel path only restores cropRegion but not the mutable cursor-follow state, so
when the crop modal is opened you should snapshot the current cursorFollowCrop
(e.g., prevCursorFollowCrop) and any related fields (safe zone, smoothness) and
then in handleCancelCropEditor() restore cursorFollowCrop via
setCursorFollowCrop(prevCursorFollowCrop) (and clear the snapshot after). Add
the snapshot creation where the editor opens and use the snapshot in
handleCancelCropEditor to roll back cursorFollowCrop changes.
Comment on lines
+6357
to
+6358
| cursorTelemetry={cursorTelemetry} | ||
| currentTimeMs={currentTime * 1000} |
Contributor
There was a problem hiding this comment.
Use the same telemetry variant as preview/export.
The crop modal gets raw cursorTelemetry, while VideoPlayback and both export paths use effectiveCursorTelemetry after normalization/loop handling. With looped cursor playback or timeline-adjusted telemetry, the crop editor can preview a different camera path than the actual render.
💡 Minimal fix
- cursorTelemetry={cursorTelemetry}
+ cursorTelemetry={effectiveCursorTelemetry}
currentTimeMs={currentTime * 1000}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cursorTelemetry={cursorTelemetry} | |
| currentTimeMs={currentTime * 1000} | |
| cursorTelemetry={effectiveCursorTelemetry} | |
| currentTimeMs={currentTime * 1000} |
🤖 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 6357 - 6358, The
crop modal is being passed raw cursorTelemetry which can differ from the
normalized/loop-aware path used by VideoPlayback and export; change the prop
passed to the crop editor from cursorTelemetry to the already-computed
effectiveCursorTelemetry (the same value used by VideoPlayback/export) so the
crop preview uses the identical telemetry variant; locate the prop usage in
VideoEditor (prop name cursorTelemetry/currentTimeMs) and replace it to supply
effectiveCursorTelemetry (or call the helper that computes it) so preview/export
paths match.
| const timeMs = this.currentVideoTime * 1000; | ||
| const cursorTimeMs = cursorTimestamp / 1000; | ||
|
|
||
| this.applyCursorFollowCrop(timeMs, layoutCache); |
Contributor
There was a problem hiding this comment.
Use the cursor timeline when sampling cursor-follow crop.
These call sites pass source-media time into computeCursorFollowCrop, but the crop logic is driven by cursor telemetry. When trims or speed regions remap output time, the crop will follow the wrong cursor sample while cursorOverlay.update(...) still uses cursorTimeMs, so exported framing diverges from the visible cursor.
Suggested fix
- this.applyCursorFollowCrop(timeMs, layoutCache);
+ this.applyCursorFollowCrop(cursorTimeMs, layoutCache);Also applies to: 3235-3235
🤖 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/exporter/modernFrameRenderer.ts` at line 2984, The call to
applyCursorFollowCrop(timeMs, layoutCache) uses source/output timeMs when
sampling cursor-follow crop but cursor sampling must use the cursor timeline;
update applyCursorFollowCrop (and any callers at the other site) to accept and
pass the timeline-aligned cursor time (cursorTimeMs) or convert timeMs to
cursorTimeMs via the same remapping used by cursorOverlay.update before calling
computeCursorFollowCrop so the crop sampling uses cursorTimeMs (ensure
references to applyCursorFollowCrop, computeCursorFollowCrop, and
cursorOverlay.update are updated accordingly).
Contributor
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/VideoEditor.tsx (1)
1175-1222:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
cursorFollowCropin dependency array.The
captureProjectThumbnailcallback usescursorFollowCropon line 1073 but does not include it in its dependency array. This can cause thumbnails to be generated with stale cursor-follow-crop settings if the setting changes after the callback is created.🔧 Proposed fix
Add
cursorFollowCropto the dependency array:}, [ annotationRegions, autoCaptionSettings, autoCaptions, backgroundBlur, borderRadius, connectZooms, connectedZoomDurationMs, connectedZoomEasing, connectedZoomGapMs, cropRegion, + cursorFollowCrop, currentTime, cursorClickBounce,🤖 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 1175 - 1222, The captureProjectThumbnail callback uses cursorFollowCrop but the effect's dependency array (the useCallback/useEffect that defines captureProjectThumbnail) is missing cursorFollowCrop, causing stale values; update the dependency array that currently lists annotationRegions, autoCaptionSettings, ..., zoomClassicMode to include cursorFollowCrop so captureProjectThumbnail re-creates when cursorFollowCrop changes (look for the captureProjectThumbnail function reference and the large dependency array near it).
🧹 Nitpick comments (1)
src/components/video-editor/CropControl.tsx (1)
318-318: 💤 Low valueRedundant conditional expression.
(!followEnabled ? true : true)always evaluates totrue, so this line simplifies to!showOutputMode.♻️ Suggested simplification
- const showHandles = !showOutputMode && (!followEnabled ? true : true); + const showHandles = !showOutputMode;🤖 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/CropControl.tsx` at line 318, The assignment to showHandles in CropControl.tsx uses a redundant ternary "(!followEnabled ? true : true)" which always evaluates to true; replace the entire expression with a simplified value by setting showHandles to "!showOutputMode" (remove the unnecessary ternary and redundant checks around followEnabled) so the variable reflects only the showOutputMode condition.
🤖 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/VideoPlayback.tsx`:
- Around line 2135-2137: The code currently only assigns to
baseMaskRef.current.sourceCrop when it already exists, so when cursor-follow
starts enabled that field may be undefined and downstream rendering misses the
effective crop; update the logic around baseMaskRef (referencing
baseMaskRef.current, sourceCrop, and effectiveCrop) to always initialize
sourceCrop to the effectiveCrop (e.g., set baseMaskRef.current.sourceCrop = {
...effectiveCrop } unconditionally or ensure initialization before cursor-follow
usage), ensuring the property exists whenever cursor-follow or rendering logic
reads it; keep the assignment in the same function where effectiveCrop is
computed so you don't change call sites.
---
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1175-1222: The captureProjectThumbnail callback uses
cursorFollowCrop but the effect's dependency array (the useCallback/useEffect
that defines captureProjectThumbnail) is missing cursorFollowCrop, causing stale
values; update the dependency array that currently lists annotationRegions,
autoCaptionSettings, ..., zoomClassicMode to include cursorFollowCrop so
captureProjectThumbnail re-creates when cursorFollowCrop changes (look for the
captureProjectThumbnail function reference and the large dependency array near
it).
---
Nitpick comments:
In `@src/components/video-editor/CropControl.tsx`:
- Line 318: The assignment to showHandles in CropControl.tsx uses a redundant
ternary "(!followEnabled ? true : true)" which always evaluates to true; replace
the entire expression with a simplified value by setting showHandles to
"!showOutputMode" (remove the unnecessary ternary and redundant checks around
followEnabled) so the variable reflects only the showOutputMode condition.
🪄 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: 083ab8d0-6c60-4020-bfcf-827871a42fad
📒 Files selected for processing (12)
src/components/video-editor/CropControl.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/cursorFollowCrop.test.tssrc/components/video-editor/videoPlayback/cursorFollowCrop.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/modernFrameRenderer.tssrc/lib/exporter/modernVideoExporter.tssrc/lib/exporter/videoExporter.ts
Comment on lines
+2135
to
+2137
| if (baseMaskRef.current.sourceCrop) { | ||
| baseMaskRef.current.sourceCrop = { ...effectiveCrop }; | ||
| } |
Contributor
There was a problem hiding this comment.
sourceCrop may never be initialized when cursor-follow is enabled.
The code only mutates baseMaskRef.current.sourceCrop if it already exists, but nothing initializes it. If cursor-follow is enabled from the start, downstream rendering logic expecting sourceCrop won't receive the effective crop coordinates.
Consider initializing sourceCrop unconditionally:
🐛 Proposed fix
- if (baseMaskRef.current.sourceCrop) {
- baseMaskRef.current.sourceCrop = { ...effectiveCrop };
- }
+ baseMaskRef.current.sourceCrop = { ...effectiveCrop };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (baseMaskRef.current.sourceCrop) { | |
| baseMaskRef.current.sourceCrop = { ...effectiveCrop }; | |
| } | |
| baseMaskRef.current.sourceCrop = { ...effectiveCrop }; |
🤖 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/VideoPlayback.tsx` around lines 2135 - 2137, The
code currently only assigns to baseMaskRef.current.sourceCrop when it already
exists, so when cursor-follow starts enabled that field may be undefined and
downstream rendering misses the effective crop; update the logic around
baseMaskRef (referencing baseMaskRef.current, sourceCrop, and effectiveCrop) to
always initialize sourceCrop to the effectiveCrop (e.g., set
baseMaskRef.current.sourceCrop = { ...effectiveCrop } unconditionally or ensure
initialization before cursor-follow usage), ensuring the property exists
whenever cursor-follow or rendering logic reads it; keep the assignment in the
same function where effectiveCrop is computed so you don't change call sites.
Ari4ka
approved these changes
Jun 1, 2026
…nd file size limit Added support for decoding audio waveforms at a lower sample rate to optimize memory usage, preventing out-of-memory errors for large files. Introduced a maximum file size limit for in-memory audio decoding to ensure stability during processing. Additionally, registered a new audio content type for .m4a files.
Text-cursor focus (cursorFollowCrop) now actively eases the viewport to center the I-beam (the typing spot) while typing instead of merely freezing it, snapping back to safe-zone mouse tracking the instant the mouse moves. Adds an independent, toggleable "Text zoom" layer that punches the camera in on the typing area when sustained typing is detected and eases back out on mouse movement. Explicit zoom regions always win over it. The layer seeds its still-detection from the telemetry window so it engages when paused/scrubbing, not just during playback, and is applied in both export paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
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/lib/exporter/modernFrameRenderer.ts (1)
3002-3002:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSample text zoom on the cursor timeline.
computeCursorTextZoomis driven bycursorTelemetry, but Line 3831 usestimeMswhile the export path already hascursorTimeMs. With trims or speed regions, text zoom can focus/activate on the wrong telemetry sample even when the rendered cursor is correct.Proposed fix
- this.updateAnimationState(timeMs); + this.updateAnimationState(timeMs, cursorTimeMs);- this.updateAnimationState(timeMs); + this.updateAnimationState(timeMs, cursorTimeMs);- private updateAnimationState(timeMs: number): number { + private updateAnimationState(timeMs: number, cursorTimeMs = timeMs): number {const textZoom = computeCursorTextZoom( this.cursorTextZoomState, this.config.cursorTelemetry, - timeMs, + cursorTimeMs, this.config.cursorFollowCrop, );Also applies to: 3253-3253, 3736-3736, 3831-3831
🤖 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/exporter/modernFrameRenderer.ts` at line 3002, The `computeCursorTextZoom` function uses cursor telemetry which must be sampled using `cursorTimeMs` rather than `timeMs` to correctly handle trims and speed regions in the export path. In src/lib/exporter/modernFrameRenderer.ts, replace all instances where cursor-related operations (particularly calls to `computeCursorTextZoom`) are parameterized with `timeMs` and replace them with `cursorTimeMs` instead. This affects the following locations: line 3253, line 3736, and line 3831. At each site, identify the call to `computeCursorTextZoom` or similar cursor telemetry sampling operation and change the time parameter passed from `timeMs` to `cursorTimeMs` to ensure the correct telemetry sample is selected regardless of trims or speed regions.
🤖 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/videoPlayback/cursorTextZoom.ts`:
- Around line 38-43: The clampDepth function incorrectly rejects a depth value
of 1 by treating it as invalid, but persistence accepts 1 as a valid value
(valid range is 1..4). In the clampDepth function, change the condition from
`depth <= 1` to `depth < 1` so that a persisted value of 1 is honored and
returned instead of being overridden with DEFAULT_TEXT_ZOOM_DEPTH_SCALE. This
allows valid saved values to be properly reproduced.
---
Outside diff comments:
In `@src/lib/exporter/modernFrameRenderer.ts`:
- Line 3002: The `computeCursorTextZoom` function uses cursor telemetry which
must be sampled using `cursorTimeMs` rather than `timeMs` to correctly handle
trims and speed regions in the export path. In
src/lib/exporter/modernFrameRenderer.ts, replace all instances where
cursor-related operations (particularly calls to `computeCursorTextZoom`) are
parameterized with `timeMs` and replace them with `cursorTimeMs` instead. This
affects the following locations: line 3253, line 3736, and line 3831. At each
site, identify the call to `computeCursorTextZoom` or similar cursor telemetry
sampling operation and change the time parameter passed from `timeMs` to
`cursorTimeMs` to ensure the correct telemetry sample is selected regardless of
trims or speed regions.
🪄 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: 64ec1d76-567d-4ee8-8873-f391a1a6613f
📒 Files selected for processing (10)
src/components/video-editor/CropControl.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/cursorFollowCrop.test.tssrc/components/video-editor/videoPlayback/cursorFollowCrop.tssrc/components/video-editor/videoPlayback/cursorTextZoom.test.tssrc/components/video-editor/videoPlayback/cursorTextZoom.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/modernFrameRenderer.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/video-editor/types.ts
- src/components/video-editor/videoPlayback/cursorFollowCrop.test.ts
- src/components/video-editor/videoPlayback/cursorFollowCrop.ts
- src/components/video-editor/CropControl.tsx
Comment on lines
+38
to
+43
| function clampDepth(depth: number | undefined): number { | ||
| if (!Number.isFinite(depth) || depth === undefined || depth <= 1) { | ||
| return DEFAULT_TEXT_ZOOM_DEPTH_SCALE; | ||
| } | ||
| return Math.min(4, depth); | ||
| } |
Contributor
There was a problem hiding this comment.
Honor persisted textZoomDepth = 1 instead of overriding it.
At Line 39, depth <= 1 falls back to DEFAULT_TEXT_ZOOM_DEPTH_SCALE, but persistence accepts/clamps depth to 1..4. This makes a valid saved value (1) impossible to reproduce.
Suggested fix
function clampDepth(depth: number | undefined): number {
- if (!Number.isFinite(depth) || depth === undefined || depth <= 1) {
+ if (!Number.isFinite(depth) || depth === undefined || depth < 1) {
return DEFAULT_TEXT_ZOOM_DEPTH_SCALE;
}
return Math.min(4, depth);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function clampDepth(depth: number | undefined): number { | |
| if (!Number.isFinite(depth) || depth === undefined || depth <= 1) { | |
| return DEFAULT_TEXT_ZOOM_DEPTH_SCALE; | |
| } | |
| return Math.min(4, depth); | |
| } | |
| function clampDepth(depth: number | undefined): number { | |
| if (!Number.isFinite(depth) || depth === undefined || depth < 1) { | |
| return DEFAULT_TEXT_ZOOM_DEPTH_SCALE; | |
| } | |
| return Math.min(4, depth); | |
| } |
🤖 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/videoPlayback/cursorTextZoom.ts` around lines 38
- 43, The clampDepth function incorrectly rejects a depth value of 1 by treating
it as invalid, but persistence accepts 1 as a valid value (valid range is 1..4).
In the clampDepth function, change the condition from `depth <= 1` to `depth <
1` so that a persisted value of 1 is honored and returned instead of being
overridden with DEFAULT_TEXT_ZOOM_DEPTH_SCALE. This allows valid saved values to
be properly reproduced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When recording screen captures with a crop applied (e.g. 1080p output from a 1440p source), the viewport tracks the mouse cursor — which works great when you're moving the mouse, but causes the frame to drift away from the typing area whenever you stop to type. This PR adds cursor-follow crop: a per-frame viewport pan that keeps the cursor inside a configurable safe zone, plus an opt-in text cursor focus mode that locks the viewport to the typing area when the I-beam is active and the mouse is stationary.
What's in the PR
Core algorithm (
videoPlayback/cursorFollowCrop.ts) — stateful per-frame computation that reads cursor telemetry, interpolates position, and eases the viewport top-left so the cursor stays inside the safe-zone inset. No dependencies on anything outside the existing telemetry pipeline.Text cursor focus — optional mode that detects mouse velocity over a 400ms look-back window and the
cursorTypefield already present in cursor.json telemetry. When the mouse has been still for 700ms and the cursor type is"text", the viewport locks to the typing area with a high smoothness floor (0.92). Mouse movement immediately snaps back to tracking mode. Debounced to avoid jarring transitions.UI — a "Track cursor" toggle in the crop panel with safe zone and smoothness sliders, plus the text cursor focus checkbox. All settings persist with the project.
Export — wired into
FrameRenderer/modernFrameRendererso the same crop behavior applies during export.Tests — 9 unit tests covering: edge clamping, safe zone hold, scrub reinit, telemetry fallback, mouse/text mode switching, and mode reset on scrub backwards.
Files changed
types.ts—CursorFollowCropSettings,DEFAULT_CURSOR_FOLLOW_CROPvideoPlayback/cursorFollowCrop.ts— new file, core algorithmvideoPlayback/cursorFollowCrop.test.ts— new file, testsCropControl.tsx— crop panel UIVideoPlayback.tsx— preview integration + dep array updateprojectPersistence.ts— load/save normalizationframeRenderer.ts,modernFrameRenderer.ts— export pipelinevideoExporter.ts,modernVideoExporter.ts,gifExporter.ts— config threadingTesting
Enable Track cursor on a 1440p recording cropped to 1080p. The viewport should pan to keep the cursor in frame during playback and export. With Text cursor focus on, scrubbing to a typing section should show the viewport staying stable on the text field; moving the mouse should immediately resume tracking.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
New Features
Chores / Improvements