feat: Add webcam size presets (small/medium/large)#289
feat: Add webcam size presets (small/medium/large)#289Jah-yee wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
- Add WebcamSizePreset type with small | medium | large - Add DEFAULT_WEBCAM_SIZE_PRESET in types - Update compositeLayout to accept webcamSizePreset parameter - Update computeCompositeLayout to use size preset fractions - Add WEBCAM_SIZE_FRACTIONS constant for sizing control
📝 WalkthroughWalkthroughThis change implements configurable webcam sizing during video recording. New controls allow users to select from three presets (small, medium, large) that dynamically adjust the webcam's stage fraction in composite layouts, addressing the issue where 9:16 aspect ratio recordings display excessively small webcam images. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
![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: 3f3ed64a25
ℹ️ 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".
| // Webcam size fractions for different presets | ||
| const WEBCAM_SIZE_FRACTIONS = { | ||
| small: 0.10, | ||
| medium: 0.18, | ||
| large: 0.30, | ||
| } as const; |
There was a problem hiding this comment.
Replacing the top-level MAX_STAGE_FRACTION constant with WEBCAM_SIZE_FRACTIONS leaves WEBCAM_LAYOUT_PRESET_MAP still referencing MAX_STAGE_FRACTION (maxStageFraction: MAX_STAGE_FRACTION), which is now undefined at module scope. This causes src/lib/compositeLayout.ts to fail type-checking and can throw at runtime when the module is loaded, so preview/export layout computation breaks before any preset logic runs.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/compositeLayout.ts (1)
134-144:⚠️ Potential issue | 🟠 Major
webcamSizePresetis optional here, but current call sites never pass it.The provided call sites in
src/lib/exporter/frameRenderer.tsandsrc/components/video-editor/videoPlayback/layoutUtils.tsomit this new arg, so preview/export stay on default"medium"regardless of user selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/compositeLayout.ts` around lines 134 - 144, The new optional parameter webcamSizePreset added to the composite layout factory in src/lib/compositeLayout.ts is never forwarded from its callers, so callers in src/lib/exporter/frameRenderer.ts and src/components/video-editor/videoPlayback/layoutUtils.ts keep using the default "medium"; update those call sites to accept/derive the user-selected webcamSizePreset (from UI state/props or layout settings) and pass it into the composite layout factory call (the function that returns WebcamCompositeLayout | null), importing/using the WebcamSizePreset type or string value as needed; keep the function's default behavior if callers still omit the argument.
🧹 Nitpick comments (1)
src/components/video-editor/types.ts (1)
5-7: ConsolidateWebcamSizePresetinto a single source of truth.
WebcamSizePresetis declared here and again insrc/lib/compositeLayout.ts(Line 18). Keeping both unions in sync manually is brittle.Proposed refactor
import type { WebcamLayoutPreset } from "@/lib/compositeLayout"; +import type { WebcamSizePreset } from "@/lib/compositeLayout"; export type ZoomDepth = 1 | 2 | 3 | 4 | 5 | 6; export type { WebcamLayoutPreset }; -export type WebcamSizePreset = "small" | "medium" | "large"; +export type { WebcamSizePreset }; export const DEFAULT_WEBCAM_SIZE_PRESET: WebcamSizePreset = "medium";🤖 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 5 - 7, The project defines the same union type and default constant twice (WebcamSizePreset and DEFAULT_WEBCAM_SIZE_PRESET); remove the duplicate union declaration in the composite layout module and have that module import the single source of truth from the existing exported definitions (WebcamSizePreset and DEFAULT_WEBCAM_SIZE_PRESET) in types.ts, update any references to use the imported symbols, and run the type-check/build to ensure no remaining local declarations or incorrect imports remain.
🤖 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 147-148: The new props webcamSizePreset and
onWebcamSizePresetChange on SettingsPanel are never used so users cannot change
webcam size; update SettingsPanel (component) to render a visible control (e.g.,
a select or radio group) that reflects webcamSizePreset and calls
onWebcamSizePresetChange when changed, and ensure the VideoEditor call site is
updated to pass through the chosen preset and handler; specifically locate the
SettingsPanel prop declarations and the VideoEditor invocation, add the UI
control bound to webcamSizePreset, wire change events to call
onWebcamSizePresetChange(preset), and propagate those props from VideoEditor so
preset changes are reachable from the UI.
In `@src/hooks/useEditorHistory.ts`:
- Around line 36-37: History now includes webcamSizePreset but project
persistence omits it; update the persistence interface and normalize/restore
logic in projectPersistence.ts to include webcamSizePreset (type
WebcamSizePreset) alongside webcamPosition so saved projects round-trip this
value. Modify the serialize/save schema to write webcamSizePreset, update the
normalize function to read it, and when loading handle missing/old projects by
defaulting to a sane value (e.g., the existing default WebcamSizePreset constant
or enum entry) so older files don’t become undefined; reference the
webcamSizePreset field, the normalize function, and the persistence
interface/type in projectPersistence.ts when making changes.
In `@src/lib/compositeLayout.ts`:
- Around line 60-65: The code references an undefined MAX_STAGE_FRACTION and
never applies the chosen preset fraction from WEBCAM_SIZE_FRACTIONS to overlay
sizing, so add a module-level constant MAX_STAGE_FRACTION (or alias it to the
appropriate WEBCAM_SIZE_FRACTIONS entry) and wire the selected preset into the
sizing math: when a preset is chosen look up WEBCAM_SIZE_FRACTIONS[preset] and
assign that value to whatever sizing variable is used (replace usages of
transform.maxStageFraction with the resolved preset/max constant), remove the
unused local preset value at the previous location, and ensure all calculations
that previously used transform.maxStageFraction now use the defined
MAX_STAGE_FRACTION / resolved preset value (affecting the spots mentioned around
transform.maxStageFraction and the local value).
---
Outside diff comments:
In `@src/lib/compositeLayout.ts`:
- Around line 134-144: The new optional parameter webcamSizePreset added to the
composite layout factory in src/lib/compositeLayout.ts is never forwarded from
its callers, so callers in src/lib/exporter/frameRenderer.ts and
src/components/video-editor/videoPlayback/layoutUtils.ts keep using the default
"medium"; update those call sites to accept/derive the user-selected
webcamSizePreset (from UI state/props or layout settings) and pass it into the
composite layout factory call (the function that returns WebcamCompositeLayout |
null), importing/using the WebcamSizePreset type or string value as needed; keep
the function's default behavior if callers still omit the argument.
---
Nitpick comments:
In `@src/components/video-editor/types.ts`:
- Around line 5-7: The project defines the same union type and default constant
twice (WebcamSizePreset and DEFAULT_WEBCAM_SIZE_PRESET); remove the duplicate
union declaration in the composite layout module and have that module import the
single source of truth from the existing exported definitions (WebcamSizePreset
and DEFAULT_WEBCAM_SIZE_PRESET) in types.ts, update any references to use the
imported symbols, and run the type-check/build to ensure no remaining local
declarations or incorrect imports remain.
🪄 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: 9b731949-0b54-4da5-829e-68e3546ec2df
📒 Files selected for processing (4)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/types.tssrc/hooks/useEditorHistory.tssrc/lib/compositeLayout.ts
| webcamSizePreset?: WebcamSizePreset; | ||
| onWebcamSizePresetChange?: (preset: WebcamSizePreset) => void; |
There was a problem hiding this comment.
New webcam size props are currently inert (no user-facing control path).
These props are added and defaulted, but this component doesn’t use them to render a selector, and the provided VideoEditor call site also does not pass them. As-is, preset changes are not reachable from the UI flow shown.
Also applies to: 217-218
🤖 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 147 - 148, The
new props webcamSizePreset and onWebcamSizePresetChange on SettingsPanel are
never used so users cannot change webcam size; update SettingsPanel (component)
to render a visible control (e.g., a select or radio group) that reflects
webcamSizePreset and calls onWebcamSizePresetChange when changed, and ensure the
VideoEditor call site is updated to pass through the chosen preset and handler;
specifically locate the SettingsPanel prop declarations and the VideoEditor
invocation, add the UI control bound to webcamSizePreset, wire change events to
call onWebcamSizePresetChange(preset), and propagate those props from
VideoEditor so preset changes are reachable from the UI.
| webcamSizePreset: WebcamSizePreset; | ||
| webcamPosition: WebcamPosition | null; |
There was a problem hiding this comment.
webcamSizePreset is added to runtime state but not persisted/restored.
After this change, history tracks webcamSizePreset, but project save/load paths still omit it (src/components/video-editor/projectPersistence.ts interface and normalize function in provided snippets). Reloaded projects can silently fall back or become undefined for this field.
Also applies to: 54-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useEditorHistory.ts` around lines 36 - 37, History now includes
webcamSizePreset but project persistence omits it; update the persistence
interface and normalize/restore logic in projectPersistence.ts to include
webcamSizePreset (type WebcamSizePreset) alongside webcamPosition so saved
projects round-trip this value. Modify the serialize/save schema to write
webcamSizePreset, update the normalize function to read it, and when loading
handle missing/old projects by defaulting to a sane value (e.g., the existing
default WebcamSizePreset constant or enum entry) so older files don’t become
undefined; reference the webcamSizePreset field, the normalize function, and the
persistence interface/type in projectPersistence.ts when making changes.
| // Webcam size fractions for different presets | ||
| const WEBCAM_SIZE_FRACTIONS = { | ||
| small: 0.10, | ||
| medium: 0.18, | ||
| large: 0.30, | ||
| } as const; |
There was a problem hiding this comment.
Fix undefined MAX_STAGE_FRACTION and apply the selected preset fraction in sizing math.
Line 74 references MAX_STAGE_FRACTION that is not defined in module scope, and Line 153’s local value is unused. Overlay sizing still uses transform.maxStageFraction, so preset selection has no effect.
Proposed fix
const WEBCAM_SIZE_FRACTIONS = {
small: 0.10,
medium: 0.18,
large: 0.30,
} as const;
@@
"picture-in-picture": {
label: "Picture in Picture",
transform: {
type: "overlay",
- maxStageFraction: MAX_STAGE_FRACTION,
+ maxStageFraction: WEBCAM_SIZE_FRACTIONS.medium,
marginFraction: MARGIN_FRACTION,
minMargin: 0,
minSize: 0,
},
@@
- // Get the max stage fraction based on size preset
- const MAX_STAGE_FRACTION = WEBCAM_SIZE_FRACTIONS[webcamSizePreset];
+ // Get the max stage fraction based on size preset
+ const maxStageFraction = WEBCAM_SIZE_FRACTIONS[webcamSizePreset];
@@
- const maxWidth = Math.max(transform.minSize, canvasWidth * transform.maxStageFraction);
- const maxHeight = Math.max(transform.minSize, canvasHeight * transform.maxStageFraction);
+ const maxWidth = Math.max(transform.minSize, canvasWidth * maxStageFraction);
+ const maxHeight = Math.max(transform.minSize, canvasHeight * maxStageFraction);Also applies to: 74-75, 152-154, 210-211
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/compositeLayout.ts` around lines 60 - 65, The code references an
undefined MAX_STAGE_FRACTION and never applies the chosen preset fraction from
WEBCAM_SIZE_FRACTIONS to overlay sizing, so add a module-level constant
MAX_STAGE_FRACTION (or alias it to the appropriate WEBCAM_SIZE_FRACTIONS entry)
and wire the selected preset into the sizing math: when a preset is chosen look
up WEBCAM_SIZE_FRACTIONS[preset] and assign that value to whatever sizing
variable is used (replace usages of transform.maxStageFraction with the resolved
preset/max constant), remove the unused local preset value at the previous
location, and ensure all calculations that previously used
transform.maxStageFraction now use the defined MAX_STAGE_FRACTION / resolved
preset value (affecting the spots mentioned around transform.maxStageFraction
and the local value).
1 participant
Summary
Adds webcam size presets to address issue #285 - making webcam recordings resizable in different layouts.
Changes
Motivation
Issue #285 requests webcam resizability, especially for 9:16 vertical formats where the webcam becomes very small.
Testing
Related
Summary by CodeRabbit
New Features