Open
Conversation
📝 WalkthroughWalkthroughThis PR adds configurable webcam mask shape support ("rectangle", "circle", "square", "rounded") to the video editor. Changes include new UI controls in SettingsPanel, state management through VideoEditor and projectPersistence, rendering updates in VideoPlayback with CSS clip-path, canvas utilities, and layout computation adjustments. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsPanel
participant VideoEditor
participant ProjectPersistence
participant VideoPlayback
participant LayoutUtils
participant CompositeLayout
User->>SettingsPanel: Select webcam mask shape<br/>(circle, square, etc.)
SettingsPanel->>VideoEditor: onWebcamMaskShapeChange(shape)
VideoEditor->>VideoEditor: pushState({webcamMaskShape: shape})
VideoEditor->>ProjectPersistence: createProjectData(editor)
ProjectPersistence->>ProjectPersistence: Persist webcamMaskShape
VideoEditor->>VideoPlayback: Re-render with webcamMaskShape prop
VideoPlayback->>LayoutUtils: layoutVideoContentUtil(..., webcamMaskShape)
LayoutUtils->>CompositeLayout: computeCompositeLayout({..., webcamMaskShape})
CompositeLayout->>CompositeLayout: Calculate shape-specific<br/>dimensions & borderRadius
CompositeLayout-->>VideoPlayback: Return webcamRect with maskShape
VideoPlayback->>VideoPlayback: Apply CSS clip-path<br/>or borderRadius based on shape
VideoPlayback-->>User: Render webcam with mask shape
sequenceDiagram
participant VideoEditor
participant Exporter
participant FrameRenderer
participant Canvas
participant WebcamMaskShapes
VideoEditor->>Exporter: Export with webcamMaskShape config
Exporter->>FrameRenderer: new FrameRenderer({..., webcamMaskShape})
FrameRenderer->>Canvas: Render frame with webcam
Canvas->>FrameRenderer: requestAnimationFrame callback
FrameRenderer->>Canvas: computeCompositeLayout({..., webcamMaskShape})
FrameRenderer->>WebcamMaskShapes: drawCanvasClipPath(ctx, x, y, w, h,<br/>shape, borderRadius)
WebcamMaskShapes->>Canvas: ctx.beginPath()<br/>ctx.arc() or ctx.roundRect()<br/>ctx.closePath()
Canvas->>Canvas: ctx.clip()
Canvas->>Canvas: ctx.drawImage(webcam)
FrameRenderer-->>VideoEditor: Frame with masked webcam rendered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 9d0ccf3bde
ℹ️ 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".
Comment on lines
+1167
to
+1169
| <div | ||
| className="absolute" | ||
| style={{ |
There was a problem hiding this comment.
The new wrapper <div> around the webcam video defaults to pointer-events: auto, so in vertical-stack mode (where the inner <video> is explicitly pointer-events-none) this wrapper still becomes the hit target and swallows clicks/drags over the webcam area. That blocks underlying editor interactions in that region (e.g. timeline/annotation manipulations routed through lower layers) whenever a webcam track is present. Add pointer-events: none on the wrapper (and re-enable on the video for PiP drag) to preserve the prior pass-through behavior.
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: 1
🧹 Nitpick comments (3)
src/lib/exporter/videoExporter.ts (1)
35-35: Consider importingWebcamMaskShapeat the top with other types.Same as in
gifExporter.ts— the inline import type is inconsistent with how other types from@/components/video-editor/typesare imported (Lines 1-8).♻️ Suggested refactor
import type { AnnotationRegion, CropRegion, SpeedRegion, TrimRegion, WebcamLayoutPreset, + WebcamMaskShape, ZoomRegion, } from "@/components/video-editor/types";Then update Line 35:
- webcamMaskShape?: import("@/components/video-editor/types").WebcamMaskShape; + webcamMaskShape?: WebcamMaskShape;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/exporter/videoExporter.ts` at line 35, The inline type import for webcamMaskShape should be replaced by importing WebcamMaskShape alongside the other types at the top of videoExporter.ts; update the top imports to include WebcamMaskShape from "@/components/video-editor/types" and change the webcamMaskShape property declaration (webcamMaskShape?: import("@/components/video-editor/types").WebcamMaskShape;) to use the imported WebcamMaskShape type instead.src/lib/exporter/gifExporter.ts (1)
44-44: Consider importingWebcamMaskShapeat the top with other types.The inline import type syntax works but is inconsistent with how other types from
@/components/video-editor/typesare imported (Lines 2-8). Moving it to the top improves readability and consistency.♻️ Suggested refactor
import type { AnnotationRegion, CropRegion, SpeedRegion, TrimRegion, WebcamLayoutPreset, + WebcamMaskShape, ZoomRegion, } from "@/components/video-editor/types";Then update Line 44:
- webcamMaskShape?: import("@/components/video-editor/types").WebcamMaskShape; + webcamMaskShape?: WebcamMaskShape;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/exporter/gifExporter.ts` at line 44, The inline type import for WebcamMaskShape should be moved to the top alongside the other imports from "@/components/video-editor/types" for consistency and readability; add WebcamMaskShape to the existing import list (the same statement that currently imports other types from "@/components/video-editor/types") and then replace the inline reference "webcamMaskShape?: import(\"@/components/video-editor/types\").WebcamMaskShape;" with "webcamMaskShape?: WebcamMaskShape;" in the gifExporter.ts declaration.src/lib/webcamMaskShapes.ts (1)
40-45: VerifyroundRectbrowser support for your target environments.
CanvasRenderingContext2D.roundRect()is widely available: Chrome/Edge 99+, Firefox 112+, Safari 16.4+, and Opera 85+ (all supporting since April 2023). For older browser support, consider a polyfill or manual path drawing implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/webcamMaskShapes.ts` around lines 40 - 45, The code uses CanvasRenderingContext2D.roundRect (ctx.roundRect) in the switch cases for "rectangle"/"rounded"/"square"/default which may not exist in older browsers; add a fallback that detects if ctx.roundRect is undefined and then draws a rounded-rect path manually (using ctx.beginPath, moveTo/lineTo/arcTo or arc for corners, closePath, and ctx.fill()/ctx.stroke() as appropriate) or include a small polyfill that defines ctx.roundRect before use; update the switch branch in webcamMaskShapes (where ctx.roundRect is called) to call the fallback function (e.g., drawRoundedRect(ctx, x, y, w, h, borderRadius)) when ctx.roundRect is unavailable.
🤖 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 631-711: The shape option labels are hardcoded and the button
group doesn't expose pressed state; update the shape list used in the
webcamLayoutPreset === "picture-in-picture" block (the array mapped to render
buttons) to use localization keys via t(...) instead of hardcoded
"Rect"/"Circle"/"Square"/"Rounded", and add aria-pressed={webcamMaskShape ===
shape.value} to each button rendered by the map so assistive tech can detect the
selected state; ensure the onClick still calls
onWebcamMaskShapeChange(shape.value) and keep existing className logic using
webcamMaskShape for visual selection.
---
Nitpick comments:
In `@src/lib/exporter/gifExporter.ts`:
- Line 44: The inline type import for WebcamMaskShape should be moved to the top
alongside the other imports from "@/components/video-editor/types" for
consistency and readability; add WebcamMaskShape to the existing import list
(the same statement that currently imports other types from
"@/components/video-editor/types") and then replace the inline reference
"webcamMaskShape?: import(\"@/components/video-editor/types\").WebcamMaskShape;"
with "webcamMaskShape?: WebcamMaskShape;" in the gifExporter.ts declaration.
In `@src/lib/exporter/videoExporter.ts`:
- Line 35: The inline type import for webcamMaskShape should be replaced by
importing WebcamMaskShape alongside the other types at the top of
videoExporter.ts; update the top imports to include WebcamMaskShape from
"@/components/video-editor/types" and change the webcamMaskShape property
declaration (webcamMaskShape?:
import("@/components/video-editor/types").WebcamMaskShape;) to use the imported
WebcamMaskShape type instead.
In `@src/lib/webcamMaskShapes.ts`:
- Around line 40-45: The code uses CanvasRenderingContext2D.roundRect
(ctx.roundRect) in the switch cases for "rectangle"/"rounded"/"square"/default
which may not exist in older browsers; add a fallback that detects if
ctx.roundRect is undefined and then draws a rounded-rect path manually (using
ctx.beginPath, moveTo/lineTo/arcTo or arc for corners, closePath, and
ctx.fill()/ctx.stroke() as appropriate) or include a small polyfill that defines
ctx.roundRect before use; update the switch branch in webcamMaskShapes (where
ctx.roundRect is called) to call the fallback function (e.g.,
drawRoundedRect(ctx, x, y, w, h, borderRadius)) when ctx.roundRect is
unavailable.
🪄 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: 45695159-3486-4219-a24d-749c90b97808
📒 Files selected for processing (17)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.test.tssrc/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.tssrc/lib/webcamMaskShapes.ts
Comment on lines
+631
to
+711
| {webcamLayoutPreset === "picture-in-picture" && ( | ||
| <div className="mt-2 p-2 rounded-lg bg-white/5 border border-white/5"> | ||
| <div className="text-[10px] font-medium text-slate-300 mb-1.5"> | ||
| {t("layout.webcamShape")} | ||
| </div> | ||
| <div className="grid grid-cols-4 gap-1.5"> | ||
| {( | ||
| [ | ||
| { value: "rectangle", label: "Rect" }, | ||
| { value: "circle", label: "Circle" }, | ||
| { value: "square", label: "Square" }, | ||
| { value: "rounded", label: "Rounded" }, | ||
| ] as Array<{ value: WebcamMaskShape; label: string }> | ||
| ).map((shape) => ( | ||
| <button | ||
| key={shape.value} | ||
| type="button" | ||
| onClick={() => onWebcamMaskShapeChange?.(shape.value)} | ||
| className={cn( | ||
| "h-10 rounded-lg border flex flex-col items-center justify-center gap-0.5 transition-all", | ||
| webcamMaskShape === shape.value | ||
| ? "bg-[#34B27B] border-[#34B27B] text-white" | ||
| : "bg-white/5 border-white/10 hover:bg-white/10 hover:border-white/20 text-slate-400", | ||
| )} | ||
| > | ||
| <svg | ||
| width="16" | ||
| height="16" | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| {shape.value === "rectangle" && ( | ||
| <rect | ||
| x="1" | ||
| y="3" | ||
| width="14" | ||
| height="10" | ||
| rx="2" | ||
| stroke="currentColor" | ||
| strokeWidth="1.5" | ||
| /> | ||
| )} | ||
| {shape.value === "circle" && ( | ||
| <circle | ||
| cx="8" | ||
| cy="8" | ||
| r="6.5" | ||
| stroke="currentColor" | ||
| strokeWidth="1.5" | ||
| /> | ||
| )} | ||
| {shape.value === "square" && ( | ||
| <rect | ||
| x="2" | ||
| y="2" | ||
| width="12" | ||
| height="12" | ||
| rx="1" | ||
| stroke="currentColor" | ||
| strokeWidth="1.5" | ||
| /> | ||
| )} | ||
| {shape.value === "rounded" && ( | ||
| <rect | ||
| x="1" | ||
| y="3" | ||
| width="14" | ||
| height="10" | ||
| rx="5" | ||
| stroke="currentColor" | ||
| strokeWidth="1.5" | ||
| /> | ||
| )} | ||
| </svg> | ||
| <span className="text-[8px] leading-none">{shape.label}</span> | ||
| </button> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Localize shape labels and expose selected state to assistive tech.
The option labels are hardcoded in English, so they won’t translate in es / zh-CN. Also, this single-select button group should expose pressed state (aria-pressed) for better accessibility.
💡 Suggested patch
- {(
- [
- { value: "rectangle", label: "Rect" },
- { value: "circle", label: "Circle" },
- { value: "square", label: "Square" },
- { value: "rounded", label: "Rounded" },
- ] as Array<{ value: WebcamMaskShape; label: string }>
- ).map((shape) => (
+ {(
+ [
+ { value: "rectangle", label: t("layout.webcamShapeRectangle") },
+ { value: "circle", label: t("layout.webcamShapeCircle") },
+ { value: "square", label: t("layout.webcamShapeSquare") },
+ { value: "rounded", label: t("layout.webcamShapeRounded") },
+ ] as Array<{ value: WebcamMaskShape; label: string }>
+ ).map((shape) => (
<button
key={shape.value}
type="button"
onClick={() => onWebcamMaskShapeChange?.(shape.value)}
+ aria-pressed={webcamMaskShape === shape.value}
className={cn(📝 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
| {webcamLayoutPreset === "picture-in-picture" && ( | |
| <div className="mt-2 p-2 rounded-lg bg-white/5 border border-white/5"> | |
| <div className="text-[10px] font-medium text-slate-300 mb-1.5"> | |
| {t("layout.webcamShape")} | |
| </div> | |
| <div className="grid grid-cols-4 gap-1.5"> | |
| {( | |
| [ | |
| { value: "rectangle", label: "Rect" }, | |
| { value: "circle", label: "Circle" }, | |
| { value: "square", label: "Square" }, | |
| { value: "rounded", label: "Rounded" }, | |
| ] as Array<{ value: WebcamMaskShape; label: string }> | |
| ).map((shape) => ( | |
| <button | |
| key={shape.value} | |
| type="button" | |
| onClick={() => onWebcamMaskShapeChange?.(shape.value)} | |
| className={cn( | |
| "h-10 rounded-lg border flex flex-col items-center justify-center gap-0.5 transition-all", | |
| webcamMaskShape === shape.value | |
| ? "bg-[#34B27B] border-[#34B27B] text-white" | |
| : "bg-white/5 border-white/10 hover:bg-white/10 hover:border-white/20 text-slate-400", | |
| )} | |
| > | |
| <svg | |
| width="16" | |
| height="16" | |
| viewBox="0 0 16 16" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| > | |
| {shape.value === "rectangle" && ( | |
| <rect | |
| x="1" | |
| y="3" | |
| width="14" | |
| height="10" | |
| rx="2" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| {shape.value === "circle" && ( | |
| <circle | |
| cx="8" | |
| cy="8" | |
| r="6.5" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| {shape.value === "square" && ( | |
| <rect | |
| x="2" | |
| y="2" | |
| width="12" | |
| height="12" | |
| rx="1" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| {shape.value === "rounded" && ( | |
| <rect | |
| x="1" | |
| y="3" | |
| width="14" | |
| height="10" | |
| rx="5" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| </svg> | |
| <span className="text-[8px] leading-none">{shape.label}</span> | |
| </button> | |
| ))} | |
| </div> | |
| </div> | |
| )} | |
| {webcamLayoutPreset === "picture-in-picture" && ( | |
| <div className="mt-2 p-2 rounded-lg bg-white/5 border border-white/5"> | |
| <div className="text-[10px] font-medium text-slate-300 mb-1.5"> | |
| {t("layout.webcamShape")} | |
| </div> | |
| <div className="grid grid-cols-4 gap-1.5"> | |
| {( | |
| [ | |
| { value: "rectangle", label: t("layout.webcamShapeRectangle") }, | |
| { value: "circle", label: t("layout.webcamShapeCircle") }, | |
| { value: "square", label: t("layout.webcamShapeSquare") }, | |
| { value: "rounded", label: t("layout.webcamShapeRounded") }, | |
| ] as Array<{ value: WebcamMaskShape; label: string }> | |
| ).map((shape) => ( | |
| <button | |
| key={shape.value} | |
| type="button" | |
| onClick={() => onWebcamMaskShapeChange?.(shape.value)} | |
| aria-pressed={webcamMaskShape === shape.value} | |
| className={cn( | |
| "h-10 rounded-lg border flex flex-col items-center justify-center gap-0.5 transition-all", | |
| webcamMaskShape === shape.value | |
| ? "bg-[`#34B27B`] border-[`#34B27B`] text-white" | |
| : "bg-white/5 border-white/10 hover:bg-white/10 hover:border-white/20 text-slate-400", | |
| )} | |
| > | |
| <svg | |
| width="16" | |
| height="16" | |
| viewBox="0 0 16 16" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| > | |
| {shape.value === "rectangle" && ( | |
| <rect | |
| x="1" | |
| y="3" | |
| width="14" | |
| height="10" | |
| rx="2" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| {shape.value === "circle" && ( | |
| <circle | |
| cx="8" | |
| cy="8" | |
| r="6.5" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| {shape.value === "square" && ( | |
| <rect | |
| x="2" | |
| y="2" | |
| width="12" | |
| height="12" | |
| rx="1" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| {shape.value === "rounded" && ( | |
| <rect | |
| x="1" | |
| y="3" | |
| width="14" | |
| height="10" | |
| rx="5" | |
| stroke="currentColor" | |
| strokeWidth="1.5" | |
| /> | |
| )} | |
| </svg> | |
| <span className="text-[8px] leading-none">{shape.label}</span> | |
| </button> | |
| ))} | |
| </div> | |
| </div> | |
| )} |
🤖 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 631 - 711, The
shape option labels are hardcoded and the button group doesn't expose pressed
state; update the shape list used in the webcamLayoutPreset ===
"picture-in-picture" block (the array mapped to render buttons) to use
localization keys via t(...) instead of hardcoded
"Rect"/"Circle"/"Square"/"Rounded", and add aria-pressed={webcamMaskShape ===
shape.value} to each button rendered by the map so assistive tech can detect the
selected state; ensure the onClick still calls
onWebcamMaskShapeChange(shape.value) and keep existing className logic using
webcamMaskShape for visual selection.
Owner
|
awesome! while you are at it, can we also have this #285 fixed? Shape and size go hand in hand. Would really appreciate it |
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.
Summary
webcamMaskShapes.tswith shape definitions and rendering logicDemo:
export-1775164482142.mp4
Test plan
compositeLayout.test.ts,projectPersistence.test.ts)Summary by CodeRabbit
Release Notes