feat: background color compositing for transparent windows + improved AI zoom suggestions for mobile#556
Open
mograph wants to merge 4 commits into
Conversation
… AI zoom suggestions for mobile - Add canvas compositing pipeline in useScreenRecorder to fill transparent window areas with a user-chosen color instead of encoding as black (H.264/VP8 have no alpha channel) - Add recording background color picker to HUD bar: preset swatches, transparent option, and custom color wheel via @uiw/react-color-colorful - Fix smoothedAutoFocusRef not writing back clamped position, causing auto-zoom trail to start outside the phone content area - Add detectClickCandidates() in zoomSuggestionUtils to generate zoom regions from click timestamps (picks up every mobile tap via uiohook-napi) - Lower MIN_DWELL_DURATION_MS from 450ms to 120ms so brief cursor pauses at click points are detected even without Accessibility permission - Use shorter fixed duration (1000ms) and pre-tap anchor (-100ms) for click-based zoom candidates so rapid taps don't produce overlapping regions - Remove inter-suggestion blocking in handleSuggestZooms so every tap gets a zoom region on the timeline regardless of adjacency - Pass cursorClickTimestamps from VideoEditor to TimelineEditor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds off-screen canvas compositing to record a configurable solid background behind screen captures, introduces per-zoom-region custom scale and aspect-ratio, adds click-derived zoom suggestion candidates merged with dwell detection, and wires these through playback, export, settings UI, device-frame overlays, locales, tests, and CI. ChangesRecording Background Compositing
Zoom Scale & Aspect Ratio Customization with Click-Based Suggestions
Configuration, Localization & Test Updates
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/timeline/TimelineEditor.tsx (1)
1063-1095:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMerge click and dwell candidates before adding regions.
right now a tap with a short pause can produce two zooms: one from
detectClickCandidates()and one fromdetectZoomDwellCandidates(). sincereservedSpansonly protects pre-existing manual regions, both get added and the timeline gets kinda cursed fast. a small proximity merge/dedupe pass here would keep it to one suggestion per interaction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/timeline/TimelineEditor.tsx` around lines 1063 - 1095, Merge nearby/duplicate suggestion candidates before emitting regions: after building allCandidates from detectZoomDwellCandidates and detectClickCandidates, run a small proximity/dedupe pass that groups candidates whose centerTimeMs are within a short threshold (e.g., few hundred ms) or whose proposed spans overlap, then collapse each group into a single candidate (pick the candidate with highest strength, use a sensible duration/effectiveDuration resolution, and carry its focus) and only push those merged results into sortedCandidates/forEach; ensure this uses the same symbols (allCandidates, detectZoomDwellCandidates, detectClickCandidates, sortedCandidates, reservedSpans, onZoomSuggested) and preserves the existing overlap check against reservedSpans before calling onZoomSuggested.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 583-605: The transparent-swatch button currently calls
setCaptureBackgroundColor(null) and behaves like a normal preset even though
null exports as black; change the UI to prevent users from selecting a broken
transparent export path by either disabling the button OR showing an inline
warning in the popover when captureBackgroundColor === null. Locate the
transparent swatch JSX (the button that references captureBackgroundColor and
calls setCaptureBackgroundColor(null), setShowCustomWheel(false),
setIsBgPickerOpen(false)) and implement one of two fixes: 1) disable the button
and add a tooltip/title clarifying “Transparent export not supported yet” (keep
the click handlers removed) or 2) keep the button enabled but onClick open the
popover with an inline warning message and do not call
setCaptureBackgroundColor(null) until export supports transparency; ensure the
visual ring/opacity logic still reflects disabled state when chosen.
In `@src/components/video-editor/timeline/zoomSuggestionUtils.ts`:
- Around line 88-105: The current MIN_CLICK_GAP_MS gating in
detectClickCandidates is suppressing rapid legitimate taps; change the dedupe
logic to only drop true duplicate timestamps rather than any nearby clicks. In
detectClickCandidates, stop using the MIN_CLICK_GAP_MS comparison against lastMs
to skip candidates; instead deduplicate by exact-equality (or a very small
epsilon, e.g., 0-1ms) of timeMs against the previous candidate timestamp (or
build a Set of clickTimestamps before sorting). Leave MIN_CLICK_GAP_MS available
if you need a separate throttle elsewhere but remove/replace its use in
detectClickCandidates (references: detectClickCandidates, MIN_CLICK_GAP_MS,
lastMs).
In `@src/hooks/useScreenRecorder.ts`:
- Around line 698-728: The canvas sizing/metadata is being set before the video
track constraints are applied, so if videoTrack.applyConstraints/upgrades the
track the canvas (bgCanvas) and videoEl dimensions stay stale; update the logic
in the captureBackgroundColor branch (symbols: captureBackgroundColor, videoEl,
bgCanvas, bgCtx, compositeAnimFrameRef, canvasStream) to await or run
videoTrack.applyConstraints() and let the videoEl metadata update before
computing bgCanvas.width/bgCanvas.height and starting the draw loop (i.e., apply
constraints, await onloadedmetadata/played, then set canvas size, then start
requestAnimationFrame drawFrame and captureStream); make the same change in the
other similar block referenced (the block around lines 750-761).
---
Outside diff comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1063-1095: Merge nearby/duplicate suggestion candidates before
emitting regions: after building allCandidates from detectZoomDwellCandidates
and detectClickCandidates, run a small proximity/dedupe pass that groups
candidates whose centerTimeMs are within a short threshold (e.g., few hundred
ms) or whose proposed spans overlap, then collapse each group into a single
candidate (pick the candidate with highest strength, use a sensible
duration/effectiveDuration resolution, and carry its focus) and only push those
merged results into sortedCandidates/forEach; ensure this uses the same symbols
(allCandidates, detectZoomDwellCandidates, detectClickCandidates,
sortedCandidates, reservedSpans, onZoomSuggested) and preserves the existing
overlap check against reservedSpans before calling onZoomSuggested.
🪄 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: CHILL
Plan: Pro
Run ID: 402f770b-1412-4713-a92b-82082d85287f
📒 Files selected for processing (7)
PR_DESCRIPTION.mdsrc/components/launch/LaunchWindow.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/timeline/zoomSuggestionUtils.tssrc/hooks/useScreenRecorder.ts
Comment on lines
+88
to
+105
| export const MIN_CLICK_GAP_MS = 200; | ||
| export const CLICK_CANDIDATE_STRENGTH = 300; | ||
| export const CLICK_REGION_DURATION_MS = 1000; | ||
| // Anchor slightly before the click so the zoom captures the result of the tap, not what came before. | ||
| export const CLICK_REGION_START_OFFSET_MS = -100; | ||
|
|
||
| export function detectClickCandidates( | ||
| telemetry: CursorTelemetryPoint[], | ||
| clickTimestamps: number[], | ||
| ): ZoomDwellCandidate[] { | ||
| if (telemetry.length === 0 || clickTimestamps.length === 0) return []; | ||
|
|
||
| const sorted = [...clickTimestamps].sort((a, b) => a - b); | ||
| const candidates: ZoomDwellCandidate[] = []; | ||
|
|
||
| for (const timeMs of sorted) { | ||
| const lastMs = candidates.length > 0 ? candidates[candidates.length - 1].centerTimeMs : -Infinity; | ||
| if (timeMs - lastMs < MIN_CLICK_GAP_MS) continue; |
Contributor
There was a problem hiding this comment.
MIN_CLICK_GAP_MS still drops rapid taps.
this fixed 200ms gate means quick double-taps or fast nav taps still only produce one candidate. if the goal is really “a zoom for every tap/click”, this should only dedupe true duplicates, not suppress nearby clicks wholesale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/timeline/zoomSuggestionUtils.ts` around lines 88
- 105, The current MIN_CLICK_GAP_MS gating in detectClickCandidates is
suppressing rapid legitimate taps; change the dedupe logic to only drop true
duplicate timestamps rather than any nearby clicks. In detectClickCandidates,
stop using the MIN_CLICK_GAP_MS comparison against lastMs to skip candidates;
instead deduplicate by exact-equality (or a very small epsilon, e.g., 0-1ms) of
timeMs against the previous candidate timestamp (or build a Set of
clickTimestamps before sorting). Leave MIN_CLICK_GAP_MS available if you need a
separate throttle elsewhere but remove/replace its use in detectClickCandidates
(references: detectClickCandidates, MIN_CLICK_GAP_MS, lastMs).
![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: 3e7729fcca
ℹ️ 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
+1079
to
+1083
| const effectiveDuration = candidate.durationMs ?? defaultDuration; | ||
| const startOffset = candidate.startOffsetMs ?? -effectiveDuration / 2; | ||
| const rawStart = Math.round(candidate.centerTimeMs + startOffset); | ||
| const candidateStart = Math.max(0, Math.min(rawStart, totalMs - effectiveDuration)); | ||
| const candidateEnd = candidateStart + effectiveDuration; |
There was a problem hiding this comment.
When a click candidate uses durationMs (currently fixed at 1000ms), effectiveDuration is no longer bounded by totalMs. For short recordings (for example, clips under 1s), candidateStart clamps to 0 but candidateEnd becomes greater than totalMs, so onZoomSuggested receives an out-of-range span. That can create zoom regions that extend past the timeline/video end and behave inconsistently in editing/playback.
Useful? React with 👍 / 👎.
Comment on lines
2152
to
2154
| unsavedExport={unsavedExport} | ||
| onSaveUnsavedExport={handleSaveUnsavedExport} | ||
| onSaveDiagnostic={handleSaveDiagnostic} | ||
| /> |
There was a problem hiding this comment.
The SettingsPanel no longer receives onSaveDiagnostic, which makes its diagnostics action disappear (SettingsPanel only renders that button when the callback is present). After this change, users lose the in-app way to export diagnostic bundles during export failures, which is a regression in troubleshooting/support workflow.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 603-626: Several UI labels in SettingsPanel are hardcoded English;
replace them with translation lookups using the existing t(...) i18n function
(e.g., change "Zoom size" → t('videoEditor.zoomSize'), the percent label
formatting to use t('videoEditor.zoomPercent', { percent:
Math.round(selectedZoomScale * 100) }) or similar, "Less"/"More" →
t('controls.less')/t('controls.more'), "Zoom shape", "Canvas Size", and dropdown
option "None (transparent)" → t('videoEditor.noneTransparent')). Update the JSX
elements around the Slider and Select in SettingsPanel (where selectedZoomScale,
onZoomScaleChange, zoomEnabled, and the Select for zoom shape/canvas size are
defined) to call t(...) instead of literal strings and ensure the translation
hook/import used elsewhere in this file is present (or import useTranslation/t
as needed) and add matching i18n keys for those strings.
In `@src/components/video-editor/types.ts`:
- Around line 364-366: The getZoomScale function should validate
region.customScale before returning it: inside getZoomScale (and referencing
ZoomRegion and ZOOM_DEPTH_SCALES) check that const cs = region.customScale is a
finite, positive number (e.g. Number.isFinite(cs) && cs > 0); if that guard
passes return cs, otherwise fall back to ZOOM_DEPTH_SCALES[region.depth]; this
prevents NaN, 0, or negative persisted values from breaking zoom math.
In `@src/components/video-editor/videoPlayback/overlayUtils.ts`:
- Around line 41-43: The zoom aspect ratio calculation incorrectly treats
region.zoomAspectRatio === "native" by calling getAspectRatioValue which returns
a 16:9 fallback; update the logic in the zoomAR computation (the use of
region.zoomAspectRatio, getAspectRatioValue, stageWidth, stageHeight) to
special-case "native" and use the recording aspect ratio (e.g.,
region.recordingAspectRatio) if available, otherwise fall back to
stageWidth/stageHeight, and only call getAspectRatioValue for non-"native" named
ratios.
🪄 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: CHILL
Plan: Pro
Run ID: da211675-3309-434d-b4de-b21ddb6d5668
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.github/workflows/ci.ymldocs/tests/writing-tests.mdelectron/electron-env.d.tselectron/ipc/handlers.tselectron/preload.tssrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/overlayUtils.tssrc/components/video-editor/videoPlayback/zoomRegionUtils.tssrc/hooks/useEditorHistory.tssrc/i18n/__tests__/tutorialHelpTranslations.test.tssrc/lib/blurEffects.test.tssrc/lib/exporter/frameRenderer.tssrc/lib/wallpaper.tssrc/utils/aspectRatioUtils.tsvitest.config.ts
💤 Files with no reviewable changes (7)
- src/i18n/tests/tutorialHelpTranslations.test.ts
- vitest.config.ts
- electron/electron-env.d.ts
- electron/preload.ts
- .github/workflows/ci.yml
- electron/ipc/handlers.ts
- docs/tests/writing-tests.md
✅ Files skipped from review due to trivial changes (1)
- src/hooks/useEditorHistory.ts
- Guard getZoomScale against NaN/zero customScale with finite+positive check - Fix "native" zoom aspect ratio using actual video ratio instead of 16:9 fallback - Lower MIN_CLICK_GAP_MS 200→50ms so rapid taps each get a zoom suggestion - Clamp click-based zoom candidateEnd to totalMs to prevent out-of-range spans - Apply applyConstraints before compositing canvas setup so canvas is sized to constrained dimensions - Add visible amber warning in bg picker when transparent (unsupported) is selected - Restore save-diagnostic IPC handler, preload bridge, type, prop, button and callback - Wire new UI strings into t() and add keys to all 9 locale settings.json files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/i18n/locales/ar/settings.json (1)
2-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winArabic locale is missing
zoom.threeDkeys and new entries are English.this is lowkey risky for UX in AR: 3D zoom labels likely fall back to raw keys, and the new labels (
zoom.size/less/more/shape,background.none,canvas.title) are still English. Please add the missingzoom.threeD.*subtree and localize these new strings.Also applies to: 69-70, 199-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/ar/settings.json` around lines 2 - 16, The Arabic locale file is missing the zoom.threeD subtree and several new keys are still in English; add a localized zoom.threeD object (e.g., zoom.threeD.title, zoom.threeD.depth, zoom.threeD.mode etc.) and translate the new keys zoom.size, zoom.less, zoom.more, zoom.shape plus background.none and canvas.title into Arabic, ensuring all strings under the "zoom" object and the referenced keys at lines around 69-70 and 199-200 are replaced with Arabic translations and matching key names (zoom.threeD, zoom.size, background.none, canvas.title) to avoid fallback to raw keys.
♻️ Duplicate comments (1)
src/components/video-editor/timeline/zoomSuggestionUtils.ts (1)
103-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStill throttling legit rapid taps in click candidate detection.
lowkey risky: the gap gate still drops fast taps, so this can miss “one zoom per tap/click” behavior. dedupe true duplicate timestamps instead of filtering by a 50ms window.
nit: cleaner dedupe approach
export function detectClickCandidates( telemetry: CursorTelemetryPoint[], clickTimestamps: number[], ): ZoomDwellCandidate[] { if (telemetry.length === 0 || clickTimestamps.length === 0) return []; const sorted = [...clickTimestamps].sort((a, b) => a - b); const candidates: ZoomDwellCandidate[] = []; + const DUPLICATE_EPSILON_MS = 1; + let lastSeenTimestamp = -Infinity; for (const timeMs of sorted) { - const lastMs = candidates.length > 0 ? candidates[candidates.length - 1].centerTimeMs : -Infinity; - if (timeMs - lastMs < MIN_CLICK_GAP_MS) continue; + if (Math.abs(timeMs - lastSeenTimestamp) <= DUPLICATE_EPSILON_MS) continue; + lastSeenTimestamp = timeMs; const focus = interpolateCursorAt(telemetry, timeMs); if (!focus) continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/timeline/zoomSuggestionUtils.ts` around lines 103 - 106, The current loop in zoomSuggestionUtils.ts is aggressively dropping candidate taps by using MIN_CLICK_GAP_MS to gate additions; change the logic to only dedupe true duplicate timestamps so rapid legitimate taps are preserved: when iterating over sorted, compare timeMs to the last candidate's centerTimeMs for equality (or maintain a small Set of seen timestamp values) and only skip when it's an exact duplicate, removing the `timeMs - lastMs < MIN_CLICK_GAP_MS` check; keep the rest of the candidate creation code (references: sorted, candidates array, centerTimeMs, MIN_CLICK_GAP_MS) intact and ensure tests for rapid taps still pass.
🧹 Nitpick comments (1)
src/components/video-editor/VideoEditor.tsx (1)
2021-2030: ⚡ Quick winExtract shared aspect-ratio layout coercion helper (dup logic in two callbacks).
This same conditional mapping for
webcamLayoutPresetappears twice; lowkey risky for future drift. nit: cleaner to centralize once and call it from both handlers.♻️ Proposed refactor
+ const coerceWebcamLayoutForAspectRatio = useCallback( + (ar: AspectRatio, preset: typeof webcamLayoutPreset) => + (isPortraitAspectRatio(ar) && preset === "dual-frame") || + (!isPortraitAspectRatio(ar) && preset === "vertical-stack") + ? "picture-in-picture" + : preset, + [], + ); ... - onAspectRatioChange={(ar) => - pushState({ - aspectRatio: ar, - webcamLayoutPreset: - (isPortraitAspectRatio(ar) && webcamLayoutPreset === "dual-frame") || - (!isPortraitAspectRatio(ar) && webcamLayoutPreset === "vertical-stack") - ? "picture-in-picture" - : webcamLayoutPreset, - }) - } + onAspectRatioChange={(ar) => + pushState({ + aspectRatio: ar, + webcamLayoutPreset: coerceWebcamLayoutForAspectRatio(ar, webcamLayoutPreset), + }) + } ... - onAspectRatioChange={(ar) => - pushState({ - aspectRatio: ar, - webcamLayoutPreset: - (isPortraitAspectRatio(ar) && webcamLayoutPreset === "dual-frame") || - (!isPortraitAspectRatio(ar) && webcamLayoutPreset === "vertical-stack") - ? "picture-in-picture" - : webcamLayoutPreset, - }) - } + onAspectRatioChange={(ar) => + pushState({ + aspectRatio: ar, + webcamLayoutPreset: coerceWebcamLayoutForAspectRatio(ar, webcamLayoutPreset), + }) + }Also applies to: 2096-2105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/VideoEditor.tsx` around lines 2021 - 2030, The aspect-ratio -> webcamLayoutPreset coercion logic is duplicated in the onAspectRatioChange callback (and again around lines 2096-2105); extract a single helper function (e.g., coerceWebcamLayoutForAspect or computeWebcamLayoutForAspect) that accepts the new aspect ratio and the current webcamLayoutPreset and returns the corrected preset using the existing conditions with isPortraitAspectRatio; replace the inline conditional inside pushState in onAspectRatioChange and the other callback to call this helper so both use the same centralized logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/i18n/locales/es/settings.json`:
- Around line 20-23: Several Spanish locale strings are still in English; update
the keys zoom.size, zoom.less, zoom.more, zoom.shape, background.none and
canvas.title in the Spanish locale to their proper Spanish translations so the
ES UI doesn't mix languages. Locate the entries for
"zoom.size"/"less"/"more"/"shape" and replace the English values with Spanish
equivalents (e.g., "Tamaño de zoom", "Menos", "Más", "Forma de zoom"), and
similarly translate the values for "background.none" and "canvas.title" to
Spanish; keep the existing key names unchanged and ensure JSON syntax/quoting
remains valid.
In `@src/i18n/locales/fr/settings.json`:
- Around line 27-30: The French locale contains untranslated English strings —
update the keys "zoom.size", "zoom.less", "zoom.more", "zoom.shape" (from the
shown diff) plus the other affected keys "background.none" and "canvas.title" to
proper French translations in src/i18n/locales/fr/settings.json; locate these
keys in the file (symbols: zoom.size, zoom.less, zoom.more, zoom.shape,
background.none, canvas.title) and replace the English values with correct
French phrases, ensuring pluralization/grammar matches existing locale style.
In `@src/i18n/locales/ko-KR/settings.json`:
- Around line 20-23: The KR locale contains English strings for several settings
keys; update the translations in src/i18n/locales/ko-KR/settings.json for the
keys "zoom.size", "zoom.less", "zoom.more", "zoom.shape", "background.none", and
"canvas.title" (and the other occurrences noted around lines 63 and 194)
replacing the English values with proper Korean text so the settings UI is fully
localized; ensure you only change the string values for those keys and keep the
JSON structure intact.
In `@src/i18n/locales/tr/settings.json`:
- Around line 20-23: The Turkish locale contains untranslated strings: update
the JSON keys "zoom.size", "zoom.less", "zoom.more", "zoom.shape" and also
"background.none" and "canvas.title" in src/i18n/locales/tr/settings.json to
their Turkish translations (replace the English values with correct Turkish
phrases), ensuring you keep the same key names and valid JSON formatting; search
for any duplicate occurrences (noted elsewhere in the file) and make the same
translations so all instances are consistent.
In `@src/i18n/locales/zh-CN/settings.json`:
- Around line 20-23: The JSON contains untranslated English labels (keys:
"zoom.size", "zoom.less", "zoom.more", "zoom.shape", "background.none", and
"canvas.title"); replace those English values with their Chinese translations so
the zh-CN locale is consistent (e.g., "Zoom size" -> "缩放大小", "Less" -> "较小",
"More" -> "较大", "Zoom shape" -> "缩放形状", "None" -> "无", "Canvas" or canvas title
-> "画布标题" or appropriate wording), updating each corresponding key in
src/i18n/locales/zh-CN/settings.json (also apply the same translation fix for
the other occurrences mentioned).
In `@src/i18n/locales/zh-TW/settings.json`:
- Around line 27-30: The zh-TW locale file has untranslated English strings for
new UI controls; update the values for the keys "zoom.size", "zoom.less",
"zoom.more", "zoom.shape", "background.none", and "canvas.title" in
src/i18n/locales/zh-TW/settings.json to proper Traditional Chinese translations
(ensure you replace the English text with localized text for those exact keys),
and verify the same keys mentioned around the other occurrences (lines noted in
the review) are also localized so the UI is not mixed-language.
---
Outside diff comments:
In `@src/i18n/locales/ar/settings.json`:
- Around line 2-16: The Arabic locale file is missing the zoom.threeD subtree
and several new keys are still in English; add a localized zoom.threeD object
(e.g., zoom.threeD.title, zoom.threeD.depth, zoom.threeD.mode etc.) and
translate the new keys zoom.size, zoom.less, zoom.more, zoom.shape plus
background.none and canvas.title into Arabic, ensuring all strings under the
"zoom" object and the referenced keys at lines around 69-70 and 199-200 are
replaced with Arabic translations and matching key names (zoom.threeD,
zoom.size, background.none, canvas.title) to avoid fallback to raw keys.
---
Duplicate comments:
In `@src/components/video-editor/timeline/zoomSuggestionUtils.ts`:
- Around line 103-106: The current loop in zoomSuggestionUtils.ts is
aggressively dropping candidate taps by using MIN_CLICK_GAP_MS to gate
additions; change the logic to only dedupe true duplicate timestamps so rapid
legitimate taps are preserved: when iterating over sorted, compare timeMs to the
last candidate's centerTimeMs for equality (or maintain a small Set of seen
timestamp values) and only skip when it's an exact duplicate, removing the
`timeMs - lastMs < MIN_CLICK_GAP_MS` check; keep the rest of the candidate
creation code (references: sorted, candidates array, centerTimeMs,
MIN_CLICK_GAP_MS) intact and ensure tests for rapid taps still pass.
---
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2021-2030: The aspect-ratio -> webcamLayoutPreset coercion logic
is duplicated in the onAspectRatioChange callback (and again around lines
2096-2105); extract a single helper function (e.g., coerceWebcamLayoutForAspect
or computeWebcamLayoutForAspect) that accepts the new aspect ratio and the
current webcamLayoutPreset and returns the corrected preset using the existing
conditions with isPortraitAspectRatio; replace the inline conditional inside
pushState in onAspectRatioChange and the other callback to call this helper so
both use the same centralized logic.
🪄 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: CHILL
Plan: Pro
Run ID: 870bc467-b5f3-498d-aba1-b57c20d85195
📒 Files selected for processing (17)
src/components/launch/LaunchWindow.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/timeline/zoomSuggestionUtils.tssrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/overlayUtils.tssrc/hooks/useScreenRecorder.tssrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.json
✅ Files skipped from review due to trivial changes (1)
- src/i18n/locales/ja-JP/settings.json
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/video-editor/types.ts
- src/components/launch/LaunchWindow.tsx
- src/components/video-editor/videoPlayback/overlayUtils.ts
- src/hooks/useScreenRecorder.ts
- src/components/video-editor/timeline/TimelineEditor.tsx
Comment on lines
+20
to
+23
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
Spanish locale has new labels left in English.
lowkey this will show mixed-language UI in the ES build (zoom.size/less/more/shape, background.none, canvas.title). Please localize these entries to Spanish to keep the panel consistent.
Also applies to: 63-63, 194-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/es/settings.json` around lines 20 - 23, Several Spanish
locale strings are still in English; update the keys zoom.size, zoom.less,
zoom.more, zoom.shape, background.none and canvas.title in the Spanish locale to
their proper Spanish translations so the ES UI doesn't mix languages. Locate the
entries for "zoom.size"/"less"/"more"/"shape" and replace the English values
with Spanish equivalents (e.g., "Tamaño de zoom", "Menos", "Más", "Forma de
zoom"), and similarly translate the values for "background.none" and
"canvas.title" to Spanish; keep the existing key names unchanged and ensure JSON
syntax/quoting remains valid.
Comment on lines
+27
to
+30
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
French locale includes untranslated English UI strings.
kinda jarring in-app: these new FR entries are still English (zoom.size/less/more/shape, background.none, canvas.title). Worth localizing before ship.
Also applies to: 70-70, 201-201
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/fr/settings.json` around lines 27 - 30, The French locale
contains untranslated English strings — update the keys "zoom.size",
"zoom.less", "zoom.more", "zoom.shape" (from the shown diff) plus the other
affected keys "background.none" and "canvas.title" to proper French translations
in src/i18n/locales/fr/settings.json; locate these keys in the file (symbols:
zoom.size, zoom.less, zoom.more, zoom.shape, background.none, canvas.title) and
replace the English values with correct French phrases, ensuring
pluralization/grammar matches existing locale style.
Comment on lines
+20
to
+23
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
Korean locale regression: new labels are English.
these keys are visible in settings and currently not localized (zoom.size/less/more/shape, background.none, canvas.title), so KR users will get mixed UI language.
Also applies to: 63-63, 194-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/ko-KR/settings.json` around lines 20 - 23, The KR locale
contains English strings for several settings keys; update the translations in
src/i18n/locales/ko-KR/settings.json for the keys "zoom.size", "zoom.less",
"zoom.more", "zoom.shape", "background.none", and "canvas.title" (and the other
occurrences noted around lines 63 and 194) replacing the English values with
proper Korean text so the settings UI is fully localized; ensure you only change
the string values for those keys and keep the JSON structure intact.
Comment on lines
+20
to
+23
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
Turkish locale has untranslated new settings labels.
nit but high-impact for i18n polish: zoom.size/less/more/shape, background.none, and canvas.title are still English in tr.
Also applies to: 63-63, 205-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/tr/settings.json` around lines 20 - 23, The Turkish locale
contains untranslated strings: update the JSON keys "zoom.size", "zoom.less",
"zoom.more", "zoom.shape" and also "background.none" and "canvas.title" in
src/i18n/locales/tr/settings.json to their Turkish translations (replace the
English values with correct Turkish phrases), ensuring you keep the same key
names and valid JSON formatting; search for any duplicate occurrences (noted
elsewhere in the file) and make the same translations so all instances are
consistent.
Comment on lines
+20
to
+23
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
zh-CN locale: newly added labels are not localized.
zoom.size/less/more/shape, background.none, and canvas.title are English right now, so the settings panel will feel inconsistent for Chinese users.
Also applies to: 63-63, 194-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/zh-CN/settings.json` around lines 20 - 23, The JSON contains
untranslated English labels (keys: "zoom.size", "zoom.less", "zoom.more",
"zoom.shape", "background.none", and "canvas.title"); replace those English
values with their Chinese translations so the zh-CN locale is consistent (e.g.,
"Zoom size" -> "缩放大小", "Less" -> "较小", "More" -> "较大", "Zoom shape" -> "缩放形状",
"None" -> "无", "Canvas" or canvas title -> "画布标题" or appropriate wording),
updating each corresponding key in src/i18n/locales/zh-CN/settings.json (also
apply the same translation fix for the other occurrences mentioned).
Comment on lines
+27
to
+30
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
zh-TW locale includes untranslated English strings for new controls.
these keys (zoom.size/less/more/shape, background.none, canvas.title) should be localized to avoid mixed-language UI.
Also applies to: 70-70, 201-201
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/zh-TW/settings.json` around lines 27 - 30, The zh-TW locale
file has untranslated English strings for new UI controls; update the values for
the keys "zoom.size", "zoom.less", "zoom.more", "zoom.shape", "background.none",
and "canvas.title" in src/i18n/locales/zh-TW/settings.json to proper Traditional
Chinese translations (ensure you replace the English text with localized text
for those exact keys), and verify the same keys mentioned around the other
occurrences (lines noted in the review) are also localized so the UI is not
mixed-language.
- New DeviceFrameType: "none" | "browser" | "iphone" | "android" - src/lib/deviceFrames.ts: canvas drawing for all three frames using offscreen canvas + destination-out compositing for screen cutout - src/components/video-editor/DeviceFrameOverlay.tsx: SVG React component for live preview using fill-rule evenodd for screen holes - Frame renders outside the 3D rotation container so it stays flat - Export path: drawDeviceFrameToCanvas() called after all compositing in FrameRenderer, passes through GifExporter and VideoExporter configs - SettingsPanel: new "Device Frame" accordion section with 2×2 button grid - i18n: frame keys added to all 9 locale settings.json files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/deviceFrames.ts (1)
60-63: ⚡ Quick winAvoid per-frame canvas allocation in hot render path
kinda cursed for perf to create a fresh offscreen canvas on every draw. Reusing a cached canvas/context (per
w/h) would reduce GC churn during long previews/exports.Also applies to: 71-71, 130-130, 191-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/deviceFrames.ts` around lines 60 - 63, The code currently allocates a new canvas per frame in makeFrameCanvas (and the other per-frame allocations noted) which causes GC churn; change makeFrameCanvas to return a cached canvas/context keyed by width/height (or a single reusable canvas if sizes are constant), updating c.width/c.height only when size changes, clear the canvas before use, and use OffscreenCanvas when available; update all call sites that previously invoked makeFrameCanvas-per-draw to reuse the cached instance instead of creating a fresh element each frame (search for makeFrameCanvas and the other per-frame creation sites to replace with the cache-aware version).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 111-112: The editor's deviceFrame field is used for UI/export but
isn't persisted; add deviceFrame to the project serialization and
deserialization paths so it's preserved across save/load. Specifically: include
editorState.deviceFrame in the payload built by saveProject, add deviceFrame to
currentProjectSnapshot so snapshots capture it, and restore
editorState.deviceFrame inside applyLoadedProject when rehydrating state; update
any places that construct or apply editorState (e.g., functions/methods that
merge loaded project into editorState) to read/write the deviceFrame value so
exports and UI remain consistent after load.
In `@src/i18n/locales/ar/settings.json`:
- Around line 12-15: The Arabic locale file contains untranslated English
strings; update the keys zoom.size, zoom.less, zoom.more, zoom.shape,
background.none, all keys under frame (frame.*), and canvas.title in
src/i18n/locales/ar/settings.json to their proper Arabic translations to avoid
mixed-language UI; locate these exact keys in the JSON and replace the English
values with their Arabic equivalents, preserving JSON structure and escaping,
and run a quick lint/translation check to ensure no other untranslated strings
remain (also apply the same fixes at the other occurrences mentioned around
lines 69 and 200-207).
---
Nitpick comments:
In `@src/lib/deviceFrames.ts`:
- Around line 60-63: The code currently allocates a new canvas per frame in
makeFrameCanvas (and the other per-frame allocations noted) which causes GC
churn; change makeFrameCanvas to return a cached canvas/context keyed by
width/height (or a single reusable canvas if sizes are constant), updating
c.width/c.height only when size changes, clear the canvas before use, and use
OffscreenCanvas when available; update all call sites that previously invoked
makeFrameCanvas-per-draw to reuse the cached instance instead of creating a
fresh element each frame (search for makeFrameCanvas and the other per-frame
creation sites to replace with the cache-aware version).
🪄 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: CHILL
Plan: Pro
Run ID: 8634289c-f675-48a9-8b4a-20e555a59057
📒 Files selected for processing (18)
src/components/video-editor/DeviceFrameOverlay.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/hooks/useEditorHistory.tssrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.jsonsrc/lib/deviceFrames.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.ts
✅ Files skipped from review due to trivial changes (5)
- src/i18n/locales/zh-TW/settings.json
- src/components/video-editor/DeviceFrameOverlay.tsx
- src/i18n/locales/tr/settings.json
- src/i18n/locales/en/settings.json
- src/i18n/locales/fr/settings.json
🚧 Files skipped from review as they are similar to previous changes (6)
- src/hooks/useEditorHistory.ts
- src/lib/exporter/frameRenderer.ts
- src/i18n/locales/ja-JP/settings.json
- src/i18n/locales/ko-KR/settings.json
- src/components/video-editor/VideoPlayback.tsx
- src/components/video-editor/SettingsPanel.tsx
Comment on lines
+111
to
112
| deviceFrame, | ||
| } = editorState; |
Contributor
There was a problem hiding this comment.
deviceFrame is wired to UI/export but not project persistence
lowkey risky: users can pick a frame, export works, then save/load project and lose that setting. Please include deviceFrame in the project round-trip paths (saveProject editor payload, currentProjectSnapshot, and applyLoadedProject state restore).
Also applies to: 166-171, 2182-2183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/VideoEditor.tsx` around lines 111 - 112, The
editor's deviceFrame field is used for UI/export but isn't persisted; add
deviceFrame to the project serialization and deserialization paths so it's
preserved across save/load. Specifically: include editorState.deviceFrame in the
payload built by saveProject, add deviceFrame to currentProjectSnapshot so
snapshots capture it, and restore editorState.deviceFrame inside
applyLoadedProject when rehydrating state; update any places that construct or
apply editorState (e.g., functions/methods that merge loaded project into
editorState) to read/write the deviceFrame value so exports and UI remain
consistent after load.
Comment on lines
+12
to
+15
| "size": "Zoom size", | ||
| "less": "Less", | ||
| "more": "More", | ||
| "shape": "Zoom shape" |
Contributor
There was a problem hiding this comment.
Arabic locale has untranslated English labels
lowkey this will ship a mixed-language settings panel in ar. Please localize these new values (zoom.size/less/more/shape, background.none, frame.*, canvas.title) to Arabic for consistency.
Also applies to: 69-69, 200-207
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/ar/settings.json` around lines 12 - 15, The Arabic locale
file contains untranslated English strings; update the keys zoom.size,
zoom.less, zoom.more, zoom.shape, background.none, all keys under frame
(frame.*), and canvas.title in src/i18n/locales/ar/settings.json to their proper
Arabic translations to avoid mixed-language UI; locate these exact keys in the
JSON and replace the English values with their Arabic equivalents, preserving
JSON structure and escaping, and run a quick lint/translation check to ensure no
other untranslated strings remain (also apply the same fixes at the other
occurrences mentioned around lines 69 and 200-207).
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
This PR adds two major features:
Recording background color compositing — when recording a transparent window (e.g. iPhone Mirroring, iOS Simulator), the transparent areas are now filled with a user-chosen solid color via an off-screen canvas pipeline, instead of encoding as black.
Improved AI zoom suggestions for mobile recordings — the "Suggest Zooms" wand now generates a zoom region for every tap/click during recording, not just long cursor dwells. Click timestamps and short cursor pauses (120ms+) are both used as candidates, and suggestions no longer block each other so all interactions appear on the timeline.
Additional changes bundled in
Motivation
Background compositing: When recording a transparent Electron window (e.g. a mirrored iPhone in a frameless window), H.264/VP8 video codecs have no alpha channel — transparent areas encode as solid black. Users had no way to control or replace that background color.
AI zoom suggestions: The previous dwell-detection algorithm required 450ms+ of cursor stillness, missing rapid navigation taps on mobile recordings. Suggestions were also silently dropped when they overlapped each other, causing most mobile interactions to be skipped entirely.
Type of Change
Review Feedback Addressed
getZoomScaledidn't guard against NaN/zerocustomScaleNumber.isFinite && > 0guard with depth-scale fallback"native"zoom AR used 16:9 fallback in overlay pathvideoSize.width/heightratioMIN_CLICK_GAP_MS = 200mssuppressed rapid tapscandidateEndclamped tototalMsapplyConstraintsrant()with keys added to all 9 localesettings.jsonfilesTesting
Background color picker:
AI zoom suggestions (mobile):
Known Bug
Transparent background does not work — selecting the checkerboard/transparent swatch sets
captureBackgroundColortonull, which bypasses canvas compositing and passes the raw video track toMediaRecorder. Because H.264 (and VP8/VP9) have no alpha channel, any transparent pixels in the source window are encoded as black. The transparent option is present in the UI but produces the same black result as having no compositing. A fix would require a codec that supports alpha (e.g. HEVC with alpha, or ProRes 4444), which is not currently supported byMediaRecorderin Chromium/Electron.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests