feat: Add webcam size with slider#345
Merged
siddharthvaddem merged 5 commits intosiddharthvaddem:mainfrom Apr 8, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds a numeric webcamSizePreset (default 25) and slider UI for picture-in-picture; threads the preset through editor state/history, project persistence, SettingsPanel, VideoEditor, VideoPlayback, layout utils, compositeLayout, exporters, and frame rendering; updates i18n and tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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. 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 |
This was referenced Apr 5, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/compositeLayout.test.ts (1)
131-150: Rename this test or assert the centering contract again.The title still says the stack is centered, but the assertions only verify full-canvas placement and never use
maxContentSize. As written, this would stay green even if centering was removed accidentally.🧪 Suggested adjustment
-it("centers the combined screen and webcam stack in vertical stack mode", () => { +it("fills the canvas in vertical stack mode", () => {If centering is still the intended contract, keep the current name and restore assertions that actually use
maxContentSize.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/compositeLayout.test.ts` around lines 131 - 150, The test "centers the combined screen and webcam stack in vertical stack mode" currently verifies full-canvas placement but never exercises maxContentSize, so either rename the test to reflect full-canvas behavior or restore assertions to verify centering using maxContentSize; update the test around computeCompositeLayout to assert that the combined content width/height respects maxContentSize and is centered within canvas (e.g., check that (canvas.width - maxContentSize.width)/2 equals the left x offset of the stacked group and similarly for vertical offsets), or change the test name to something like "fills canvas in vertical-stack mode when maxContentSize >= canvas" if you intend to keep the existing assertions.src/components/video-editor/types.ts (1)
6-9: Export the slider bounds from the same source as the default.
10and50are now duplicated insrc/lib/compositeLayout.tsandsrc/components/video-editor/projectPersistence.ts, while only25is shared here. If the range changes later, preview, export, persistence, and tests can drift silently.♻️ Suggested cleanup
/** Webcam size as a percentage of the canvas reference dimension (10–50). */ export type WebcamSizePreset = number; +export const MIN_WEBCAM_SIZE_PRESET: WebcamSizePreset = 10; +export const MAX_WEBCAM_SIZE_PRESET: WebcamSizePreset = 50; export const DEFAULT_WEBCAM_SIZE_PRESET: WebcamSizePreset = 25;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/types.ts` around lines 6 - 9, Declare and export explicit min/max constants alongside DEFAULT_WEBCAM_SIZE_PRESET in the same module (e.g., MIN_WEBCAM_SIZE_PRESET and MAX_WEBCAM_SIZE_PRESET) and change usages in compositeLayout and projectPersistence to import these constants instead of hardcoded 10 and 50; update any related types or comments for WebcamSizePreset if needed so the slider bounds and default come from a single source of truth.
🤖 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/lib/compositeLayout.ts`:
- Around line 61-65: webcamSizeToFraction currently lets NaN propagate through
Math.min/Math.max; update the function (webcamSizeToFraction) to guard the
incoming percent by first checking Number.isFinite(percent) (or
!Number.isNaN(percent)) and substituting a sensible default (e.g., 10) when it’s
invalid, then perform the existing clamping (clamped) and return clamped/100;
ensure the guard covers non-numeric and infinite values so scale/width/height
won't become NaN.
---
Nitpick comments:
In `@src/components/video-editor/types.ts`:
- Around line 6-9: Declare and export explicit min/max constants alongside
DEFAULT_WEBCAM_SIZE_PRESET in the same module (e.g., MIN_WEBCAM_SIZE_PRESET and
MAX_WEBCAM_SIZE_PRESET) and change usages in compositeLayout and
projectPersistence to import these constants instead of hardcoded 10 and 50;
update any related types or comments for WebcamSizePreset if needed so the
slider bounds and default come from a single source of truth.
In `@src/lib/compositeLayout.test.ts`:
- Around line 131-150: The test "centers the combined screen and webcam stack in
vertical stack mode" currently verifies full-canvas placement but never
exercises maxContentSize, so either rename the test to reflect full-canvas
behavior or restore assertions to verify centering using maxContentSize; update
the test around computeCompositeLayout to assert that the combined content
width/height respects maxContentSize and is centered within canvas (e.g., check
that (canvas.width - maxContentSize.width)/2 equals the left x offset of the
stacked group and similarly for vertical offsets), or change the test name to
something like "fills canvas in vertical-stack mode when maxContentSize >=
canvas" if you intend to keep the existing assertions.
🪄 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
Run ID: 62b66e5b-2a2c-4608-9d85-de9d7876be55
📒 Files selected for processing (15)
src/components/video-editor/SettingsPanel.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/layoutUtils.tssrc/hooks/useEditorHistory.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/lib/compositeLayout.test.tssrc/lib/compositeLayout.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/compositeLayout.ts (1)
154-155: Consider lowercase naming for the computed variable.
MAX_STAGE_FRACTIONuses constant-style naming (SCREAMING_CASE) but is now a computed value derived from input. A name likestageFractionwould better signal that it varies per call. Very minor style nit.♻️ Optional rename
- const MAX_STAGE_FRACTION = webcamSizeToFraction(webcamSizePreset); + const stageFraction = webcamSizeToFraction(webcamSizePreset);Then update line 214-215:
- const maxWidth = Math.max(transform.minSize, referenceDim * MAX_STAGE_FRACTION); - const maxHeight = Math.max(transform.minSize, referenceDim * MAX_STAGE_FRACTION); + const maxWidth = Math.max(transform.minSize, referenceDim * stageFraction); + const maxHeight = Math.max(transform.minSize, referenceDim * stageFraction);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/compositeLayout.ts` around lines 154 - 155, Rename the computed constant MAX_STAGE_FRACTION to a lowercased, non-constant-style name (e.g., stageFraction) because it’s derived from input via webcamSizeToFraction(webcamSizePreset); update all references to MAX_STAGE_FRACTION (including usages near the previous lines referenced around where MAX_STAGE_FRACTION was used) to the new name so the code consistently reflects that this value varies per call.
🤖 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/lib/compositeLayout.ts`:
- Around line 154-155: Rename the computed constant MAX_STAGE_FRACTION to a
lowercased, non-constant-style name (e.g., stageFraction) because it’s derived
from input via webcamSizeToFraction(webcamSizePreset); update all references to
MAX_STAGE_FRACTION (including usages near the previous lines referenced around
where MAX_STAGE_FRACTION was used) to the new name so the code consistently
reflects that this value varies per call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aaa7c4a-60c9-4bca-9521-50bd05d5665f
📒 Files selected for processing (1)
src/lib/compositeLayout.ts
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)
272-293:⚠️ Potential issue | 🔴 CriticalCritical:
webcamSizePresetis missing from the unsaved-change snapshotLine 272-293 builds
currentProjectSnapshot, but it never includeswebcamSizePreset. So if the user only changes webcam size, unsaved-change detection won’t flip, which can silently drop edits on close. kinda cursed edge case, but real.🐛 Minimal fix
return createProjectSnapshot(currentProjectMedia, { wallpaper, shadowIntensity, showBlur, motionBlurAmount, borderRadius, padding, cropRegion, zoomRegions, trimRegions, speedRegions, annotationRegions, aspectRatio, webcamLayoutPreset, webcamMaskShape, + webcamSizePreset, webcamPosition, exportQuality, exportFormat, gifFrameRate, gifLoop, gifSizePreset, });🤖 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 272 - 293, The snapshot created by createProjectSnapshot(currentProjectMedia, {...}) omits webcamSizePreset so changes to only webcam size won't mark the project as dirty; update the call site in VideoEditor.tsx to include webcamSizePreset in the object passed to createProjectSnapshot (alongside webcamLayoutPreset, webcamMaskShape, webcamPosition, etc.) so snapshots reflect webcam size changes and unsaved-change detection works correctly.
🤖 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/types.ts`:
- Around line 6-9: Add exported constants for the webcam size bounds and use
them as the single source of truth: define and export MIN_WEBCAM_SIZE_PRESET
(10) and MAX_WEBCAM_SIZE_PRESET (50) alongside WebcamSizePreset and
DEFAULT_WEBCAM_SIZE_PRESET in types.ts, update any normalization, validation, or
UI/layout code that currently uses the magic numbers 10/50 to reference these
new constants (e.g., any functions that clamp or validate the preset value), and
ensure any docs/comments reference the constants instead of hard-coded values.
---
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 272-293: The snapshot created by
createProjectSnapshot(currentProjectMedia, {...}) omits webcamSizePreset so
changes to only webcam size won't mark the project as dirty; update the call
site in VideoEditor.tsx to include webcamSizePreset in the object passed to
createProjectSnapshot (alongside webcamLayoutPreset, webcamMaskShape,
webcamPosition, etc.) so snapshots reflect webcam size changes and
unsaved-change detection works correctly.
🪄 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: be420b9c-a564-4232-be69-cbb4af6ab5df
📒 Files selected for processing (7)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.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
siddharthvaddem
approved these changes
Apr 8, 2026
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.
Pull Request Template
Description
Continues and improves the webcam resize feature from PR #289. Merges the original work with main, resolves all conflicts (including new
webcamMaskShapefeature), fixes broken type-checking and missing wiring, and replaces the small/medium/large preset dropdown with a continuous percentage slider (10%–50%) for finer control over webcam overlay size.Motivation
PR #289 introduced webcam size presets but had several issues:
MAX_STAGE_FRACTIONwas referenced at module scope before being defined, causing TypeScript compilation errorswebcamSizePresetwas never wired through the rendering pipeline (canvas preview, export, project persistence), so changing the preset had no visible effectcanvasWidth/canvasHeightindependently instead of a unified reference dimensionType of Change
Related Issue(s)
Screenshots / Video
Screenshot:
Video:
Screen.Recording.2026-04-05.at.20.05.27.mov
Testing
webcamSizePreset) — verify it defaults to 25%npx vitest run --environment node src/lib/compositeLayout.test.ts— all 9 tests should passChecklist
Thank you for contributing!
Summary by CodeRabbit
New Features
Tests
Localization