feat: extend speed options with higher presets and custom speed input#291
Open
1shanpanta wants to merge 1 commit intosiddharthvaddem:mainfrom
Open
feat: extend speed options with higher presets and custom speed input#2911shanpanta wants to merge 1 commit intosiddharthvaddem:mainfrom
1shanpanta wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
add 3x, 4x, 5x speed presets and a custom playback speed input field that accepts any integer value up to 16x. change PlaybackSpeed type from a fixed union to number with min/max constants and clamp utility. update project persistence to validate any speed in range instead of exact value matching. add i18n keys for en, es, zh-CN. closes siddharthvaddem#252
📝 WalkthroughWalkthroughThe PR extends the video editor's playback speed feature from fixed preset options (max 2×) to support custom speeds within a range of 0.1× to 16×, including a new custom input component, type generalization, validation logic, and localized UI text. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 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)
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 |
1 task
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/video-editor/SettingsPanel.tsx (1)
58-119: The render-time state synchronization pattern could cause issues.Lines 71-75 update state (
setDraft) during render when the value changes. While this works, it's an anti-pattern that React may warn about and can cause unexpected re-renders.♻️ Consider using useEffect for synchronization
function CustomSpeedInput({ value, onChange, onError, }: { value: number; onChange: (val: number) => void; onError: () => void; }) { const isPreset = SPEED_OPTIONS.some((o) => o.speed === value); const [draft, setDraft] = useState(isPreset ? "" : String(Math.round(value))); const [isFocused, setIsFocused] = useState(false); - const prevValue = useRef(value); - if (!isFocused && prevValue.current !== value) { - prevValue.current = value; - setDraft(isPreset ? "" : String(Math.round(value))); - } + useEffect(() => { + if (!isFocused) { + const newIsPreset = SPEED_OPTIONS.some((o) => o.speed === value); + setDraft(newIsPreset ? "" : String(Math.round(value))); + } + }, [value, isFocused]);🤖 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 58 - 119, The component CustomSpeedInput currently updates state during render via prevValue/ref logic (prevValue, setDraft, isFocused, isPreset), which is an anti-pattern; move that synchronization into a useEffect that watches [value, isFocused, isPreset] (and optionally SPEED_OPTIONS-derived isPreset) and inside the effect update prevValue.current and call setDraft(isPreset ? "" : String(Math.round(value))) only when not focused and prevValue.current !== value, removing the inline render-time setDraft branch so all state updates occur inside the effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 58-119: The component CustomSpeedInput currently updates state
during render via prevValue/ref logic (prevValue, setDraft, isFocused,
isPreset), which is an anti-pattern; move that synchronization into a useEffect
that watches [value, isFocused, isPreset] (and optionally SPEED_OPTIONS-derived
isPreset) and inside the effect update prevValue.current and call
setDraft(isPreset ? "" : String(Math.round(value))) only when not focused and
prevValue.current !== value, removing the inline render-time setDraft branch so
all state updates occur inside the effect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b506ae2-bcfb-44f2-a4d5-e7c04c4ca6d9
📒 Files selected for processing (6)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/types.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/zh-CN/settings.json
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
Extends playback speed options beyond the original 2x cap. Adds 3x, 4x, 5x speed presets and a custom playback speed input field that accepts any integer value up to 16x.
Motivation
Issue #252 requests higher speed options because trimming isn't always applicable -- users need to fast-forward through slow sections of code reviews and demos while still showing what happened. A commenter also requested a custom input field for speeds beyond the presets, similar to how iMovie supports custom speed values.
Type of Change
Related Issue(s)
Closes #252
Screenshots / Video
Screenshot (if applicable):
Will add screenshot of the new speed panel UI
Video (if applicable):
N/A
Testing
npx tsc --noEmit-- 0 errorsnpm run lint-- cleannpx vite build-- builds successfullynpx vitest --run-- 14 pass, 2 pre-existing failures on main (compositeLayout, unrelated)Checklist
Summary by CodeRabbit
New Features
Localization