feat: add device frame mockups for video export#294
feat: add device frame mockups for video export#294mvanhorn wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
Adds MacBook and Browser window frame overlays that wrap recordings in device mockups. Frames are SVG-based and render as the final compositing layer in both MP4 and GIF exports. Changes: - New deviceFrame field in EditorState (default: "none") - Frame definitions in src/lib/deviceFrames.ts - SVG frame assets in public/frames/ - Frame selector UI in SettingsPanel (None/MacBook/Browser) - Frame rendering in FrameRenderer export pipeline - Project persistence support for device frame selection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughA device frame feature is being added to the video editor, allowing users to wrap recordings with decorative mockups (MacBook, Browser) through a new settings UI control. The feature integrates state management, persistence, and export rendering across multiple layers of the application. Changes
Sequence DiagramsequenceDiagram
participant User
participant SettingsPanel
participant VideoEditor
participant EditorHistory
participant Exporter
participant FrameRenderer
User->>SettingsPanel: Select device frame (macbook)
SettingsPanel->>VideoEditor: onDeviceFrameChange("macbook")
VideoEditor->>EditorHistory: Push {deviceFrame: "macbook"}
EditorHistory->>EditorHistory: Update state & history
Note over User,EditorHistory: ...later, during export...
User->>VideoEditor: Trigger export
VideoEditor->>Exporter: Create with deviceFrame config
Exporter->>FrameRenderer: Initialize with deviceFrame
FrameRenderer->>FrameRenderer: loadDeviceFrame() via getDeviceFrame()
FrameRenderer->>FrameRenderer: Load image from /frames/macbook.svg
FrameRenderer->>FrameRenderer: renderFrame() loop
FrameRenderer->>FrameRenderer: Composite video + annotations
FrameRenderer->>FrameRenderer: drawDeviceFrame() overlay
FrameRenderer-->>Exporter: Composite canvas with frame
Exporter-->>User: Export complete (video/gif with frame)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. ✨ 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: 3203879e56
ℹ️ 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".
| return JSON.stringify( | ||
| createProjectData(currentProjectMedia, { | ||
| wallpaper, | ||
| deviceFrame, |
There was a problem hiding this comment.
currentProjectSnapshot now serializes deviceFrame, but applyLoadedProject() never writes normalizedEditor.deviceFrame into pushState. When a user opens a project saved with a non-default frame, the editor keeps the stale in-memory frame value, so subsequent exports can use the wrong mockup and the project is immediately marked dirty because currentProjectSnapshot no longer matches lastSavedSnapshot.
Useful? React with 👍 / 👎.
| const w = this.config.width; | ||
| const h = this.config.height; | ||
|
|
||
| ctx.drawImage(this.deviceFrameImage, 0, 0, w, h); |
There was a problem hiding this comment.
drawDeviceFrame() stretches the SVG over the full export canvas and never uses the screen geometry defined in DEVICE_FRAMES, so the compositor still lays out video/webcam/annotations against the full canvas instead of the frame cutout. In practice this misaligns content for the MacBook/Browser templates (content is cropped or padded inside the transparent window), especially when padding or webcam layouts are changed.
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/components/video-editor/VideoEditor.tsx (1)
185-200:⚠️ Potential issue | 🟠 MajorMissing
deviceFramewhen restoring project state.The
applyLoadedProjectfunction pushes normalized editor values to state but omitsdeviceFrame. This means when a project is loaded, the saved device frame selection is lost and defaults back to"none".🐛 Proposed fix
pushState({ wallpaper: normalizedEditor.wallpaper, + deviceFrame: normalizedEditor.deviceFrame, shadowIntensity: normalizedEditor.shadowIntensity, showBlur: normalizedEditor.showBlur, motionBlurAmount: normalizedEditor.motionBlurAmount,🤖 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 185 - 200, The applyLoadedProject routine is restoring normalizedEditor values via pushState but omits deviceFrame, causing loaded projects to lose saved device frame selection; update the pushState call inside applyLoadedProject (the block that sets wallpaper, shadowIntensity, showBlur, etc.) to also include deviceFrame: normalizedEditor.deviceFrame so the saved deviceFrame is restored when the project is loaded.
🤖 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 733-757: The "Device Frame" block in SettingsPanel.tsx currently
uses hardcoded labels; replace the static strings ("Device Frame", "None",
"MacBook", "Browser") with i18n keys (for example settings.deviceFrame,
settings.deviceFrame.none, settings.deviceFrame.macbook,
settings.deviceFrame.browser) and call the translation function t() where those
strings are used (e.g., for the header and for each frame.label in the mapping),
ensuring the component imports/uses the same t() instance as the rest of the
panel and that corresponding keys are added to the project's i18n translation
files for each supported locale; keep the onClick handler (onDeviceFrameChange)
and the deviceFrame equality checks unchanged.
In `@src/lib/exporter/frameRenderer.ts`:
- Around line 330-352: loadDeviceFrame() currently fetches the
DeviceFrameDefinition via getDeviceFrame(frameId) but only uses
frameDef.imagePath; capture and store the screen positioning data (x, y, width,
height) from the returned DeviceFrameDefinition into a new/existing property
(e.g. this.deviceFrameScreen) so downstream layout code can position video
inside the frame; specifically, after const frameDef = getDeviceFrame(frameId)
and before loading the image, assign this.deviceFrameScreen = { x: frameDef.x,
y: frameDef.y, width: frameDef.width, height: frameDef.height } (normalizing
types if needed) and keep this.deviceFrameImage assignment as-is so later video
layout computation can reference this.deviceFrameScreen.
- Around line 354-362: The device frame's screen rectangle
(DeviceFrameDefinition.screen) is computed but never applied, so drawDeviceFrame
currently paints the full frame over the canvas and hides video; fix by
converting the screen percentages to pixel coordinates using
this.config.width/height (compute screenX, screenY, screenW, screenH from
deviceFrame.screen), then ensure video rendering is constrained to that rect —
either (preferred) draw video using ctx.drawImage(source, sx, sy, sw, sh,
screenX, screenY, screenW, screenH) or apply a clipping mask (ctx.save();
ctx.beginPath(); ctx.rect(screenX, screenY, screenW, screenH); ctx.clip(); draw
video; ctx.restore()) before drawing the bezel image with
drawImage(this.deviceFrameImage, 0,0,w,h); update drawDeviceFrame and the
video-rendering location/functions to use the computed screen rect so video
appears only in the frame opening.
---
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 185-200: The applyLoadedProject routine is restoring
normalizedEditor values via pushState but omits deviceFrame, causing loaded
projects to lose saved device frame selection; update the pushState call inside
applyLoadedProject (the block that sets wallpaper, shadowIntensity, showBlur,
etc.) to also include deviceFrame: normalizedEditor.deviceFrame so the saved
deviceFrame is restored when the project is loaded.
🪄 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: 662a8f2c-9658-4a93-938e-23cb1be2d6f3
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/frames/browser.svgis excluded by!**/*.svgpublic/frames/macbook.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/projectPersistence.tssrc/hooks/useEditorHistory.tssrc/lib/deviceFrames.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.ts
| <div className="p-2 rounded-lg bg-white/5 border border-white/5"> | ||
| <div className="flex items-center justify-between mb-1.5"> | ||
| <div className="text-[10px] font-medium text-slate-300">Device Frame</div> | ||
| </div> | ||
| <div className="flex gap-1.5"> | ||
| {[ | ||
| { id: "none", label: "None" }, | ||
| { id: "macbook", label: "MacBook" }, | ||
| { id: "browser", label: "Browser" }, | ||
| ].map((frame) => ( | ||
| <button | ||
| key={frame.id} | ||
| type="button" | ||
| onClick={() => onDeviceFrameChange?.(frame.id)} | ||
| className={`flex-1 text-[9px] font-medium px-2 py-1.5 rounded-md transition-all ${ | ||
| deviceFrame === frame.id | ||
| ? "bg-[#34B27B] text-white" | ||
| : "bg-white/5 text-slate-400 hover:bg-white/10 hover:text-slate-300" | ||
| }`} | ||
| > | ||
| {frame.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Hardcoded UI strings should use i18n translations.
The "Device Frame", "None", "MacBook", and "Browser" labels are hardcoded, while the rest of the settings panel consistently uses t() for translations. This creates an inconsistent experience for non-English users.
💬 Suggested fix
Add translation keys to your i18n files and replace hardcoded strings:
-<div className="text-[10px] font-medium text-slate-300">Device Frame</div>
+<div className="text-[10px] font-medium text-slate-300">{t("effects.deviceFrame")}</div>
{[
- { id: "none", label: "None" },
- { id: "macbook", label: "MacBook" },
- { id: "browser", label: "Browser" },
+ { id: "none", label: t("effects.deviceFrameNone") },
+ { id: "macbook", label: t("effects.deviceFrameMacbook") },
+ { id: "browser", label: t("effects.deviceFrameBrowser") },
].map((frame) => (📝 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.
| <div className="p-2 rounded-lg bg-white/5 border border-white/5"> | |
| <div className="flex items-center justify-between mb-1.5"> | |
| <div className="text-[10px] font-medium text-slate-300">Device Frame</div> | |
| </div> | |
| <div className="flex gap-1.5"> | |
| {[ | |
| { id: "none", label: "None" }, | |
| { id: "macbook", label: "MacBook" }, | |
| { id: "browser", label: "Browser" }, | |
| ].map((frame) => ( | |
| <button | |
| key={frame.id} | |
| type="button" | |
| onClick={() => onDeviceFrameChange?.(frame.id)} | |
| className={`flex-1 text-[9px] font-medium px-2 py-1.5 rounded-md transition-all ${ | |
| deviceFrame === frame.id | |
| ? "bg-[#34B27B] text-white" | |
| : "bg-white/5 text-slate-400 hover:bg-white/10 hover:text-slate-300" | |
| }`} | |
| > | |
| {frame.label} | |
| </button> | |
| ))} | |
| </div> | |
| </div> | |
| <div className="p-2 rounded-lg bg-white/5 border border-white/5"> | |
| <div className="flex items-center justify-between mb-1.5"> | |
| <div className="text-[10px] font-medium text-slate-300">{t("effects.deviceFrame")}</div> | |
| </div> | |
| <div className="flex gap-1.5"> | |
| {[ | |
| { id: "none", label: t("effects.deviceFrameNone") }, | |
| { id: "macbook", label: t("effects.deviceFrameMacbook") }, | |
| { id: "browser", label: t("effects.deviceFrameBrowser") }, | |
| ].map((frame) => ( | |
| <button | |
| key={frame.id} | |
| type="button" | |
| onClick={() => onDeviceFrameChange?.(frame.id)} | |
| className={`flex-1 text-[9px] font-medium px-2 py-1.5 rounded-md transition-all ${ | |
| deviceFrame === frame.id | |
| ? "bg-[`#34B27B`] text-white" | |
| : "bg-white/5 text-slate-400 hover:bg-white/10 hover:text-slate-300" | |
| }`} | |
| > | |
| {frame.label} | |
| </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 733 - 757, The
"Device Frame" block in SettingsPanel.tsx currently uses hardcoded labels;
replace the static strings ("Device Frame", "None", "MacBook", "Browser") with
i18n keys (for example settings.deviceFrame, settings.deviceFrame.none,
settings.deviceFrame.macbook, settings.deviceFrame.browser) and call the
translation function t() where those strings are used (e.g., for the header and
for each frame.label in the mapping), ensuring the component imports/uses the
same t() instance as the rest of the panel and that corresponding keys are added
to the project's i18n translation files for each supported locale; keep the
onClick handler (onDeviceFrameChange) and the deviceFrame equality checks
unchanged.
| private async loadDeviceFrame(): Promise<void> { | ||
| const frameId = this.config.deviceFrame; | ||
| if (!frameId || frameId === "none") return; | ||
|
|
||
| const frameDef = getDeviceFrame(frameId); | ||
| if (!frameDef) return; | ||
|
|
||
| const img = new Image(); | ||
| const imageUrl = window.location.origin + frameDef.imagePath; | ||
|
|
||
| await new Promise<void>((resolve) => { | ||
| img.onload = () => resolve(); | ||
| img.onerror = (err) => { | ||
| console.warn("[FrameRenderer] Failed to load device frame:", imageUrl, err); | ||
| resolve(); // Don't fail export if frame can't load | ||
| }; | ||
| img.src = imageUrl; | ||
| }); | ||
|
|
||
| if (img.complete && img.naturalWidth > 0) { | ||
| this.deviceFrameImage = img; | ||
| } | ||
| } |
There was a problem hiding this comment.
loadDeviceFrame() retrieves the frame definition but discards the screen coordinates.
The function calls getDeviceFrame(frameId) which returns a DeviceFrameDefinition containing screen positioning data (x, y, width, height as percentages), but only frameDef.imagePath is used. The screen coordinates should be stored and later used to properly position video content within the device frame's screen region.
🔧 Suggested approach to store screen coordinates
export class FrameRenderer {
private deviceFrameImage: HTMLImageElement | null = null;
+ private deviceFrameScreen: { x: number; y: number; width: number; height: number } | null = null;
// ... other fields ...
private async loadDeviceFrame(): Promise<void> {
const frameId = this.config.deviceFrame;
if (!frameId || frameId === "none") return;
const frameDef = getDeviceFrame(frameId);
if (!frameDef) return;
+ // Store screen coordinates for video positioning
+ this.deviceFrameScreen = frameDef.screen;
const img = new Image();
// ... rest of loading logic ...
}Then use this.deviceFrameScreen when computing video layout to ensure content fits within the frame's screen region.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/exporter/frameRenderer.ts` around lines 330 - 352, loadDeviceFrame()
currently fetches the DeviceFrameDefinition via getDeviceFrame(frameId) but only
uses frameDef.imagePath; capture and store the screen positioning data (x, y,
width, height) from the returned DeviceFrameDefinition into a new/existing
property (e.g. this.deviceFrameScreen) so downstream layout code can position
video inside the frame; specifically, after const frameDef =
getDeviceFrame(frameId) and before loading the image, assign
this.deviceFrameScreen = { x: frameDef.x, y: frameDef.y, width: frameDef.width,
height: frameDef.height } (normalizing types if needed) and keep
this.deviceFrameImage assignment as-is so later video layout computation can
reference this.deviceFrameScreen.
| private drawDeviceFrame(): void { | ||
| if (!this.deviceFrameImage || !this.compositeCtx) return; | ||
|
|
||
| const ctx = this.compositeCtx; | ||
| const w = this.config.width; | ||
| const h = this.config.height; | ||
|
|
||
| ctx.drawImage(this.deviceFrameImage, 0, 0, w, h); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the screen property is defined but never accessed in frameRenderer.ts
echo "=== Checking if 'screen' property from DeviceFrameDefinition is ever used ==="
rg -n "screen" src/lib/exporter/frameRenderer.ts
echo ""
echo "=== Checking how frameDef is used after getDeviceFrame call ==="
rg -nA 10 "getDeviceFrame" src/lib/exporter/frameRenderer.ts
echo ""
echo "=== DeviceFrameDefinition screen coordinates that should be used ==="
cat src/lib/deviceFrames.tsRepository: siddharthvaddem/openscreen
Length of output: 3284
🏁 Script executed:
#!/bin/bash
# Find compositeLayout creation and initialization
echo "=== Searching for compositeLayout variable declaration ==="
rg -n "compositeLayout\s*=" src/lib/exporter/frameRenderer.ts | head -20
echo ""
echo "=== Searching for where compositeLayout is created/assigned ==="
rg -nB 5 "compositeLayout" src/lib/exporter/frameRenderer.ts | head -40
echo ""
echo "=== Look for any layout calculation or compositing logic ==="
rg -n "layout|composite" src/lib/exporter/frameRenderer.ts | head -30Repository: siddharthvaddem/openscreen
Length of output: 3489
🏁 Script executed:
#!/bin/bash
# Check if frameDef.screen is EVER used anywhere in the codebase
echo "=== Searching entire codebase for frameDef.screen usage ==="
rg "frameDef\.screen|\.screen\." src/ --type ts --type tsx | grep -i frame
echo ""
echo "=== Searching for DeviceFrameDefinition.screen usage ==="
rg "screen\s*:" src/lib/deviceFrames.ts
echo ""
echo "=== Check rendering order - when is drawDeviceFrame called? ==="
rg -n "drawDeviceFrame" src/lib/exporter/frameRenderer.ts
echo ""
echo "=== Look at full renderFrame to understand order ==="
sed -n '445,465p' src/lib/exporter/frameRenderer.tsRepository: siddharthvaddem/openscreen
Length of output: 1141
🏁 Script executed:
#!/bin/bash
# Let's verify the exact implementation of drawDeviceFrame to confirm it draws at full canvas
echo "=== Full drawDeviceFrame implementation ==="
sed -n '354,362p' src/lib/exporter/frameRenderer.ts
echo ""
echo "=== Check what ctx.drawImage parameters mean ==="
echo "ctx.drawImage(image, x, y, width, height) draws image at position (x,y) with size (width, height)"
echo "In this case: ctx.drawImage(frame, 0, 0, canvas_width, canvas_height)"
echo "This draws the ENTIRE frame image at FULL CANVAS SIZE, ignoring the screen region"
echo ""
echo "=== Check if there's any masking or clipping for the screen region ==="
rg -n "clip|mask" src/lib/exporter/frameRenderer.ts | grep -v "maskGraphics\|maskRect"Repository: siddharthvaddem/openscreen
Length of output: 772
Device frame overlay is drawn at full canvas size without positioning video content within the screen region.
The DeviceFrameDefinition.screen property (defining where video should appear within the frame as percentages) is retrieved but never used. The drawDeviceFrame() method draws the entire frame image at full canvas dimensions via ctx.drawImage(this.deviceFrameImage, 0, 0, w, h), and is called after all video rendering (line 453). This means:
- Video renders at full canvas size
- Frame overlay draws at full canvas size on top
- Video content is not positioned or masked to appear only through the frame's transparent screen region
- The video will be hidden behind the frame's bezel
The screen region coordinates must either be applied to scale/position the video content within the frame, or a masking mechanism must be implemented so the video only renders within the frame's transparent screen area.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/exporter/frameRenderer.ts` around lines 354 - 362, The device frame's
screen rectangle (DeviceFrameDefinition.screen) is computed but never applied,
so drawDeviceFrame currently paints the full frame over the canvas and hides
video; fix by converting the screen percentages to pixel coordinates using
this.config.width/height (compute screenX, screenY, screenW, screenH from
deviceFrame.screen), then ensure video rendering is constrained to that rect —
either (preferred) draw video using ctx.drawImage(source, sx, sy, sw, sh,
screenX, screenY, screenW, screenH) or apply a clipping mask (ctx.save();
ctx.beginPath(); ctx.rect(screenX, screenY, screenW, screenH); ctx.clip(); draw
video; ctx.restore()) before drawing the bezel image with
drawImage(this.deviceFrameImage, 0,0,w,h); update drawDeviceFrame and the
video-rendering location/functions to use the computed screen rect so video
appears only in the frame opening.
1 participant
Summary
Adds MacBook and Browser window frame overlays that wrap recordings in device mockups. The frame renders as the final compositing layer on top of all content (video, webcam, annotations) in both MP4 and GIF exports.
Why this matters
OpenScreen positions itself as a free Screen Studio alternative. Device frame mockups are one of Screen Studio's most recognizable features. They make demos look polished without post-production.
Changes
src/lib/deviceFrames.ts(new): Frame definitions with screen area coordinates for each device typepublic/frames/macbook.svg(new): MacBook Pro frame SVGpublic/frames/browser.svg(new): Browser window frame SVGsrc/hooks/useEditorHistory.ts: AddeddeviceFrametoEditorState(default:"none")src/lib/exporter/frameRenderer.ts: Loads device frame image during init, draws it as the topmost compositing layer inrenderFrame()src/lib/exporter/videoExporter.ts: PassesdeviceFrameconfig to FrameRenderersrc/lib/exporter/gifExporter.ts: Same for GIF export pathsrc/components/video-editor/SettingsPanel.tsx: Frame selector UI (None/MacBook/Browser buttons)src/components/video-editor/VideoEditor.tsx: WiresdeviceFramestate through save/export pathssrc/components/video-editor/projectPersistence.ts: Persists frame selection in project filesHow it works
FrameRenderer.loadDeviceFrame()loads the SVG as an HTMLImageElementdrawDeviceFrame()draws the frame overlay on topThe frame overlay approach is intentionally simple: the SVG image is drawn on top of the composite canvas at full size. The video content underneath already fills the frame's screen area because the frame SVGs have transparent screen regions.
Testing
npx tsc --noEmit)This contribution was developed with AI assistance (Claude Code).
Summary by CodeRabbit
Release Notes