Open
Conversation
…m, trim, and speed region controls.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 112f02fe03
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…le smooth transitions between connected regions
📝 WalkthroughWalkthroughAdds per-region zoom-in/out duration state and controls: UI selector, timeline drag handles to edit durations, propagation of duration changes through SettingsPanel → VideoEditor → TimelineEditor → Item, and playback strength computation updated to use per-region durations and proportional scaling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Author
|
@siddharthvaddem hey, I have implemented zoom can u check it, I am new to open source contributions. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 169-175: The TODO comment near ZOOM_SPEED_OPTIONS indicates
configurability is planned but untracked; either remove the TODO or replace it
with a reference to a tracking issue and a brief comment explaining intent.
Update the SettingsPanel.tsx comment above the ZOOM_SPEED_OPTIONS constant to
either (a) remove the TODO if configurability won't be implemented soon, or (b)
replace it with a one-line comment like "Make zoom speed configurable — see
ISSUE-123" (or your issue ID) so the work is tracked; ensure the constant name
ZOOM_SPEED_OPTIONS remains unchanged.
- Line 927: The className on the TabsList in SettingsPanel.tsx contains a double
space between "grid-cols-3" and "rounded-lg"; update the TabsList component's
className value (the string passed to TabsList) to remove the extra space so it
reads "... grid grid-cols-3 rounded-lg" to keep classes consistent.
- Around line 572-574: selectedZoomInDuration/selectedZoomOutDuration in
SettingsPanel.tsx are floats coming from VideoEditor.tsx while
ZOOM_SPEED_OPTIONS defines integer zoomIn/zoomOut values, so the strict equality
check in the isActive computation never matches; fix by normalizing before
compare — either round (Math.round) or floor the selectedZoom* values or compare
after casting ZOOM_SPEED_OPTIONS entries to numbers with the same rounding
strategy, or alternatively change
ZOOM_IN_TRANSITION_WINDOW_MS/TRANSITION_WINDOW_MS to integer constants; update
the isActive logic that references selectedZoomInDuration,
selectedZoomOutDuration and ZOOM_SPEED_OPTIONS accordingly so default/smooth
options correctly become active.
In `@src/components/video-editor/timeline/Item.tsx`:
- Around line 117-143: Replace the repeated magic numbers with shared constants
and use the canonical duration logic: export DEFAULT_ZOOM_IN_MS and
DEFAULT_ZOOM_OUT_MS from zoomRegionUtils (or a central constants file), import
them into Item.tsx, and replace all literal occurrences of 1522.575 and 1015.05
with DEFAULT_ZOOM_IN_MS and DEFAULT_ZOOM_OUT_MS respectively (references:
zoomInDurationMs, zoomOutDurationMs, onZoomDurationChange). Additionally, when
computing handle positions/widths use the values returned from getDurations(...)
(or call getDurations with span and the defaults) so the UI uses the rescaled
durations getDurations may produce, preventing handle position divergence from
actual playback timing.
- Around line 127-154: The onPointerDown handler captures e.clientX inside the
onPointerMove closure which is fragile; capture the startX (e.clientX) and any
other needed initial values (e.g., zoomInDurationMs, zoomOutDurationMs) at the
top of the onPointerDown before creating onPointerMove so the closure uses those
captured primitives instead of reading e later; update the onPointerMove logic
to compute deltaPx = moveEvent.clientX - startX and keep the rest
(pixelsToValue, newDuration calc, onZoomDurationChange(id,...)) unchanged, and
ensure pointer capture/release and event listener cleanup still use the original
pointerId and target logic from onPointerDown.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1772-1786: The fallback durations for selectedZoomInDuration and
selectedZoomOutDuration use floating constants ZOOM_IN_TRANSITION_WINDOW_MS and
TRANSITION_WINDOW_MS which don't match the integer presets in
ZOOM_SPEED_OPTIONS, causing mismatches downstream; update the logic in the
selectedZoomInDuration/selectedZoomOutDuration expressions (which reference
selectedZoomId and zoomRegions) to round (or otherwise coerce to integer) the
fallback values so they match the integer presets (e.g., use
Math.round/Math.floor on ZOOM_IN_TRANSITION_WINDOW_MS and TRANSITION_WINDOW_MS)
before returning them to ensure comparisons with ZOOM_SPEED_OPTIONS succeed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9f0ff17d-d830-44ce-b4bc-7ee6510c0731
📒 Files selected for processing (9)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/zoomRegionUtils.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/zh-CN/settings.json
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/video-editor/VideoEditor.tsx (1)
1773-1783:⚠️ Potential issue | 🟡 MinorFallback duration values should be coerced to preset-compatible values.
Lowkey risky UX here: Line 1775 and Line 1781 can return float defaults (
1522.575/1015.05). If preset matching is exact in the settings selector, the default option may not appear selected until user interaction.nit: cleaner fallback coercion
selectedZoomInDuration={ selectedZoomId ? (zoomRegions.find((z) => z.id === selectedZoomId)?.zoomInDurationMs ?? - ZOOM_IN_TRANSITION_WINDOW_MS) + Math.round(ZOOM_IN_TRANSITION_WINDOW_MS)) : undefined } selectedZoomOutDuration={ selectedZoomId ? (zoomRegions.find((z) => z.id === selectedZoomId)?.zoomOutDurationMs ?? - TRANSITION_WINDOW_MS) + Math.round(TRANSITION_WINDOW_MS)) : undefined }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 1773 - 1783, The selectedZoomInDuration and selectedZoomOutDuration fallbacks may be floats (from ZOOM_IN_TRANSITION_WINDOW_MS / TRANSITION_WINDOW_MS) which won't match exact preset options; when computing these props in VideoEditor (use of selectedZoomId and zoomRegions), coerce the fallback values to preset-compatible integers or mapped preset values (e.g., round or snap to the nearest preset) before returning them so the settings selector can match correctly; implement this by applying a normalization helper (e.g., normalizeToPresetDuration) or Math.round/nearest-preset logic to the fallback expressions that reference ZOOM_IN_TRANSITION_WINDOW_MS and TRANSITION_WINDOW_MS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1773-1783: The selectedZoomInDuration and selectedZoomOutDuration
fallbacks may be floats (from ZOOM_IN_TRANSITION_WINDOW_MS /
TRANSITION_WINDOW_MS) which won't match exact preset options; when computing
these props in VideoEditor (use of selectedZoomId and zoomRegions), coerce the
fallback values to preset-compatible integers or mapped preset values (e.g.,
round or snap to the nearest preset) before returning them so the settings
selector can match correctly; implement this by applying a normalization helper
(e.g., normalizeToPresetDuration) or Math.round/nearest-preset logic to the
fallback expressions that reference ZOOM_IN_TRANSITION_WINDOW_MS and
TRANSITION_WINDOW_MS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08020be9-f425-492a-ba31-52ae7b8afd6b
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/components/video-editor/timeline/Item.tsx (1)
148-159:⚠️ Potential issue | 🔴 CriticalUse the normalized durations in the drag handlers.
Lines 149-157 and Lines 193-201 still seed drag math from raw props/defaults instead of the normalized
zoomIn/zoomOutvalues fromgetDurations(...). For any region shorter than the combined in/out duration, the available span goes negative and the first drag snaps a handle to0ms, even though render/playback are showing scaled values.♻️ Small fix
- const initialZoomIn = zoomInDurationMs ?? DEFAULT_ZOOM_IN_MS; - const initialZoomOut = zoomOutDurationMs ?? DEFAULT_ZOOM_OUT_MS; + const initialZoomIn = zoomIn; + const initialZoomOut = zoomOut;Apply the same swap in both pointer handlers so drag, render, and playback all use the same normalized durations.
Also applies to: 192-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/timeline/Item.tsx` around lines 148 - 159, The drag handlers are using raw props/defaults (initialZoomIn/initialZoomOut) instead of the normalized durations from getDurations, causing incorrect snap behavior; update both pointer handlers (the onPointerMove and its counterpart later) to use the normalized zoomIn and zoomOut values returned by getDurations (replace initialZoomIn/initialZoomOut with zoomIn/zoomOut in the delta/duration math and the onZoomDurationChange call) so drag, render, and playback use the same normalized durations.src/components/video-editor/SettingsPanel.tsx (1)
169-174:⚠️ Potential issue | 🟠 Major
Smoothis still out of sync with the real default timing.The canonical default zoom-in rounds to
1523ms, but this preset hardcodes1522ms. Default zoom regions will still show no active speed button, which is confusing UX. Please derive this pair from the shared transition constants so it can’t drift again.🔧 Keep the preset synced to the shared constants
+import { TRANSITION_WINDOW_MS, ZOOM_IN_TRANSITION_WINDOW_MS } from "./videoPlayback/constants"; + const ZOOM_SPEED_OPTIONS = [ { label: "Instant", zoomIn: 0, zoomOut: 0 }, { label: "Fast", zoomIn: 500, zoomOut: 350 }, - { label: "Smooth", zoomIn: 1522, zoomOut: 1015 }, + { + label: "Smooth", + zoomIn: Math.round(ZOOM_IN_TRANSITION_WINDOW_MS), + zoomOut: Math.round(TRANSITION_WINDOW_MS), + }, { label: "Lazy", zoomIn: 3000, zoomOut: 2000 }, ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/SettingsPanel.tsx` around lines 169 - 174, The "Smooth" preset in ZOOM_SPEED_OPTIONS hardcodes zoom timings (1522/1015) that drift from the canonical defaults; update the "Smooth" entry in ZOOM_SPEED_OPTIONS to derive its zoomIn/zoomOut values from the shared transition constants instead of literals — import the appropriate constants (e.g., DEFAULT_ZOOM_IN_MS and DEFAULT_ZOOM_OUT_MS or the names exported from the shared transitions module) and use those constants for the "Smooth" object's zoomIn and zoomOut so the preset always matches the canonical defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 564-590: The current SettingsPanel rendering uses opt.label from
ZOOM_SPEED_OPTIONS which bypasses i18n; update ZOOM_SPEED_OPTIONS to carry
translation keys (e.g., labelKey) instead of hardcoded English, and change the
render in SettingsPanel.tsx to call t(opt.labelKey) (or t(opt.labelKey, {
defaultValue: opt.label }) if you want a fallback) inside the Button span so
labels are localized; ensure any tests or consumers that construct
ZOOM_SPEED_OPTIONS are updated to use the new labelKey property.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 852-863: handleZoomDurationChange currently calls pushState on
every pointermove (Item.tsx -> onZoomDurationChange), spamming undo/history;
change it to use the move-safe pattern: during pointermove call updateState
instead of pushState so intermediate drags do not create history entries, and
ensure a corresponding commitState is called on pointerup/end to record the
final change; locate handleZoomDurationChange and replace the pushState call
with updateState, and verify Item.tsx or the parent drag handlers call
commitState when the drag finishes.
---
Duplicate comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 169-174: The "Smooth" preset in ZOOM_SPEED_OPTIONS hardcodes zoom
timings (1522/1015) that drift from the canonical defaults; update the "Smooth"
entry in ZOOM_SPEED_OPTIONS to derive its zoomIn/zoomOut values from the shared
transition constants instead of literals — import the appropriate constants
(e.g., DEFAULT_ZOOM_IN_MS and DEFAULT_ZOOM_OUT_MS or the names exported from the
shared transitions module) and use those constants for the "Smooth" object's
zoomIn and zoomOut so the preset always matches the canonical defaults.
In `@src/components/video-editor/timeline/Item.tsx`:
- Around line 148-159: The drag handlers are using raw props/defaults
(initialZoomIn/initialZoomOut) instead of the normalized durations from
getDurations, causing incorrect snap behavior; update both pointer handlers (the
onPointerMove and its counterpart later) to use the normalized zoomIn and
zoomOut values returned by getDurations (replace initialZoomIn/initialZoomOut
with zoomIn/zoomOut in the delta/duration math and the onZoomDurationChange
call) so drag, render, and playback use the same normalized durations.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 41b07cc1-3e6b-40ba-8cd1-42b9122b1d98
📒 Files selected for processing (4)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/Item.tsxsrc/components/video-editor/videoPlayback/zoomRegionUtils.ts
Moncef-Mhz
changed the title
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
1 participant
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.
Description
Adds interactive region controls for zoom, trim, and speed segments in the video editor timeline. Users can now create, adjust, and delete these regions directly from the settings panel and timeline UI.
Motivation
Previously there was no way to fine-tune zoom, trim, or speed regions after placing them. This change gives users precise control over each region's parameters, improving the overall editing experience.
Type of Change
Related Issue(s)
#265
Screenshots / Video
Video (if applicable):
2026-04-07.00-51-10.mp4
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit
New Features
Localization