feat(launch): refine recording HUD and language switching UX#362
Merged
siddharthvaddem merged 11 commits intoApr 16, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPR adds system language detection with user confirmation prompts, replacing hardcoded locale lists with dynamic availability checking. Introduces a language-picker sidebar modal in the launch window and system-locale-suggestion flow with translations for English, Spanish, and Simplified Chinese. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Multi-layered changes spanning context state management, dynamic locale validation, new UI patterns with portals/positioning, and a type-signature shift from union to string that could have downstream implications. Moderate heterogeneity across i18n plumbing, component extensions, and UI logic. Kinda a lot to reason through in isolation, but the patterns are fairly consistent once you grok the system-locale-detection flow. Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines the Launch/HUD user experience by adding a one-time system-language suggestion prompt and redesigning the in-HUD language switcher and recording controls to reduce distraction and clipping issues.
Changes:
- Add first-launch system language detection and a one-time suggestion prompt via
I18nContext. - Replace the previous LaunchWindow language selector with a compact HUD language menu and adjust recording bar controls/layout.
- Update
launchtranslations foren,es, andzh-CNto support the new prompt strings.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/i18n/locales/en/launch.json | Adds systemLanguagePrompt strings for the new onboarding prompt. |
| src/i18n/locales/es/launch.json | Adds Spanish systemLanguagePrompt strings. |
| src/i18n/locales/zh-CN/launch.json | Adds Chinese systemLanguagePrompt strings. |
| src/contexts/I18nContext.tsx | Implements supported system-locale detection and prompt state/actions with persistence. |
| src/components/ui/select.tsx | Extends SelectContent API and adjusts viewport sizing/scroll button behavior. |
| src/components/launch/LaunchWindow.tsx | Introduces the system-language prompt UI and a new HUD language menu + recording HUD behavior updates. |
![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: 08b5580ca2
ℹ️ 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".
![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
🤖 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/launch/LaunchWindow.tsx`:
- Around line 197-220: The handler name onPointerDown in the useEffect is
inconsistent with the event it's attached to
(document.addEventListener("mousedown", ...)); rename the function to
onMouseDown or, alternatively, change the event to "pointerdown" so naming and
behavior match; update both the function declaration (currently referenced as
onPointerDown) and the addEventListener/removeEventListener calls involving
document and keep references to isLanguageMenuOpen, languageMenuRef, and
setIsLanguageMenuOpen intact.
- Around line 573-594: The language menu is not keyboard accessible; replace the
custom conditional JSX using
isLanguageMenuOpen/SUPPORTED_LOCALES/setLocale/setIsLanguageMenuOpen/getLocaleName
in LaunchWindow with the existing Radix-based wrapper from
src/components/ui/dropdown-menu.tsx so you get built-in focus management,
arrow-key navigation and ARIA roles; render SUPPORTED_LOCALES as
DropdownMenu.Trigger and DropdownMenu.Content (or DropdownMenu.Item) entries,
keep the checkmark for the selected locale, call setLocale(...) and close the
menu (setIsLanguageMenuOpen(false) or use Radix's onSelect/close) on selection,
and remove the manual Escape handling — alternatively, if you keep the custom
implementation add role="menu"/role="menuitem", manage focus on open, and
implement onKeyDown to handle ArrowUp/ArrowDown/Home/End/Enter/Escape for proper
keyboard navigation and selection.
In `@src/contexts/I18nContext.tsx`:
- Around line 119-139: The effect currently lists systemLocaleSuggestion in its
dependency array but the effect is intended to run only once (it early-returns
when suggestion exists), so replace that dep with a run-once ref: create a ref
(e.g., hasRunRef) at top of the component, check hasRunRef.current at the start
of the useEffect and set it to true when you proceed, then change the dependency
array to only include markPromptAsHandled; keep the existing guards
(hasStoredLocale, hasHandledSystemPrompt, getSupportedSystemLocale,
setSystemLocaleSuggestion, markPromptAsHandled) unchanged so behavior is
identical but the effect no longer depends on systemLocaleSuggestion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca1cc556-e354-47ae-8f02-a315a4eea1d5
📒 Files selected for processing (6)
src/components/launch/LaunchWindow.tsxsrc/components/ui/select.tsxsrc/contexts/I18nContext.tsxsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/zh-CN/launch.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/launch/LaunchWindow.tsx`:
- Around line 538-574: The onSelect handler inside the DropdownMenuItem always
calls resolveSystemLocaleSuggestion() even when the user re-selects the current
locale; change the handler so it only updates when loc !== locale by checking
the current locale first, and only then call setLocale(loc) and
resolveSystemLocaleSuggestion(); locate the onSelect on the DropdownMenuItem
within the SUPPORTED_LOCALES mapping and use the existing locale, setLocale, and
resolveSystemLocaleSuggestion identifiers to implement the guard.
- Around line 467-490: Replace the non-standard Tailwind opacity utilities in
the LaunchWindow button className (the ternary branch that uses recording and
paused) with standard scale values: change "bg-red-500/12" to "bg-red-500/10"
and "bg-red-500/16" to "bg-red-500/15" inside the className string used for the
recording toggle button in LaunchWindow.tsx (the button that calls
toggleRecording and renders getIcon based on
recording/paused/hasSelectedSource).
In `@src/contexts/I18nContext.tsx`:
- Around line 154-162: The two callbacks dismissSystemLocaleSuggestion and
resolveSystemLocaleSuggestion are identical; replace the duplicate by creating a
single useCallback (calling setSystemLocaleSuggestion(null) and
markPromptAsHandled()) and assign the other name as an alias to it (keep the
original useCallback named dismissSystemLocaleSuggestion and set
resolveSystemLocaleSuggestion = dismissSystemLocaleSuggestion) so callers keep
semantic names while removing duplicated code; reference the existing symbols
dismissSystemLocaleSuggestion, resolveSystemLocaleSuggestion,
setSystemLocaleSuggestion, and markPromptAsHandled when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bad7f768-ca01-4454-90c6-15336e6dfcb1
📒 Files selected for processing (2)
src/components/launch/LaunchWindow.tsxsrc/contexts/I18nContext.tsx
1 task
imAaryash
commented
Apr 6, 2026
Collaborator
Author
|
@siddharthvaddem @getSono is everything fine? or need any changes? |
Collaborator
|
Hey @imAaryash , |
…anslations fix(i18n): add missing tutorial dialog translation keys
siddharthvaddem
requested changes
Apr 11, 2026
…orial-translations" This reverts commit 5494acb.
…nscreen into detect-system-lang
- derive available locales from locale folders with required namespace validation - exclude incomplete locales and report missing namespace files - align system-language suggestion and selectors with discovered locales - improve launch HUD language menu interaction, scrolling, and viewport clipping - make i18n-check discover locale folders automatically
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/video-editor/VideoEditor.tsx (2)
282-331:⚠️ Potential issue | 🟠 Major
webcamSizePresetnever makes it into the dirty snapshot.
saveProject()persists this field, butcurrentProjectSnapshotomits it. So changing only the webcam size can either fail to mark the project dirty or leave the editor permanently dirty after a save, depending on the saved value. Add it to the snapshot payload too.minimal fix
return createProjectSnapshot(currentProjectMedia, { wallpaper, shadowIntensity, showBlur, motionBlurAmount, borderRadius, padding, cropRegion, zoomRegions, trimRegions, speedRegions, annotationRegions, aspectRatio, webcamLayoutPreset, webcamMaskShape, + webcamSizePreset, webcamPosition, exportQuality, exportFormat, gifFrameRate, gifLoop,🤖 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 282 - 331, The snapshot created by currentProjectSnapshot omits webcamSizePreset, causing dirty-state mismatches; update the object passed into createProjectSnapshot within the currentProjectSnapshot useMemo to include webcamSizePreset (same identifier), ensuring the snapshot payload contains webcamSizePreset so changes to it mark the project dirty and align with saveProject persistence.
641-676:⚠️ Potential issue | 🟡 MinorSelection reset is still asymmetric with speed regions.
These paths clear blur selection now, but they still leave
selectedSpeedIdaround. lowkey risky: the speed panel/delete state can stay active while a zoom, trim, or annotation is newly selected. Blur already clears speed, so the other selection/add flows should do the same for consistency.Also applies to: 678-729, 905-955
🤖 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 641 - 676, The selection-reset behavior is asymmetric: handleSelectZoom, handleSelectTrim, and handleSelectAnnotation do not clear selectedSpeedId, which allows the speed UI/state to remain active incorrectly; update these handlers (handleSelectZoom, handleSelectTrim, handleSelectAnnotation) to also call setSelectedSpeedId(null) when an id is selected, and apply the same fix to the equivalent selection/add handlers in the other ranges mentioned (the selection handlers around 678-729 and the selection/delete flows around 905-955) so every selection path consistently clears selectedZoomId, selectedTrimId, selectedAnnotationId, selectedBlurId, and selectedSpeedId as appropriate.src/components/video-editor/timeline/TimelineEditor.tsx (1)
1374-1392:⚠️ Potential issue | 🟡 MinorBlur items still render like annotations.
This now emits
variant: "blur", butsrc/components/video-editor/timeline/Item.tsxnever branches on that variant yet, so blur clips still get the annotation icon/color treatment. makes the new blur lane pretty hard to distinguish at a glance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/timeline/TimelineEditor.tsx` around lines 1374 - 1392, The blur items are given variant: "blur" in TimelineEditor (the TimelineRenderItem objects) but Item.tsx's rendering logic doesn't handle "blur", so blur clips still look like annotations; update the Item component (where it reads item.variant / the switch/branch that handles "annotation" and "speed") to add a case for "blur" that supplies the appropriate icon, color/style and any tooltip/label behavior for blur items (use the same prop names as other variants like speedValue/label) so blur lane items render distinctively; ensure TimelineRenderItem type handling in Item.tsx recognizes "blur" as a valid variant.
♻️ Duplicate comments (1)
src/components/launch/LaunchWindow.tsx (1)
186-211:⚠️ Potential issue | 🟠 MajorThe custom language menu still isn't keyboard-complete.
This opens and closes fine, but focus stays on the trigger and the menu never implements ArrowUp/ArrowDown/Home/End navigation. With
role="menu"/menuitemradiosemantics, that's a real mismatch for keyboard users, and in practice Tab is going to wander into the window controls instead of the locale list. Either move focus into the menu and add roving-key handling, or swap this back to a menu primitive that gives you that behavior for free.Also applies to: 628-665
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/LaunchWindow.tsx` around lines 186 - 211, The language menu opens/closes but doesn't move focus into the menu or implement roving-keyboard navigation, so implement keyboard-complete behavior: when isLanguageMenuOpen becomes true (use the existing isLanguageMenuOpen setter), programmatically move focus from languageTriggerRef into the menu (languageMenuPanelRef) to the selected or first menuitem, attach a keydown handler on the menu panel to implement ArrowDown/ArrowUp/Home/End to move focus between menuitem elements (roving focus), keep Escape closing via setIsLanguageMenuOpen(false), and ensure Tab/Shift+Tab behavior is handled (prevent tabbing out when the menu should trap focus or restore focus to languageTriggerRef on close); update/remove the temporary key listeners in the current useEffect and add the menu keydown handler and focus management to the components that use languageTriggerRef and languageMenuPanelRef (also apply same fix to the other menu instance noted).
🧹 Nitpick comments (8)
scripts/build_macos.sh (1)
70-71: nit: use fixed-string grep for signing identity
grep -q "$SIGN_IDENTITY"treats value as regex. Usinggrep -Fq -- "$SIGN_IDENTITY"avoids odd matches when identity contains regex-special chars.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build_macos.sh` around lines 70 - 71, The grep call that checks for the signing identity currently treats SIGN_IDENTITY as a regex which can mis-match when the identity contains regex-special characters; update the command that uses grep -q "$SIGN_IDENTITY" (the check after security find-identity) to use fixed-string matching and safe argument handling by calling grep -Fq -- "$SIGN_IDENTITY" so the identity is treated literally and won’t be interpreted as a regex.electron/i18n.ts (1)
26-29: nit: centralize supported locales to avoid driftRight now locale values are duplicated in the union + whitelist condition. Lowkey easy to forget one later. A
const SUPPORTED_MAIN_LOCALES = [...] as constplusincludes()type guard would keep this cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/i18n.ts` around lines 26 - 29, The locale whitelist is duplicated in setMainLocale, which risks divergence; add a single exported constant like SUPPORTED_MAIN_LOCALES = ['en','zh-CN','es','fr'] as const and use it for both the type/union and runtime check (e.g., SUPPORTED_MAIN_LOCALES.includes(locale as any)) inside setMainLocale so the allowed values are centralized and the typeguard stays accurate; update any existing type definitions or references to use this constant for consistency.electron/windows.ts (1)
59-63: nit: tiny DRY refactor opportunitySame darwin workspace-visibility snippet appears twice. Could extract a helper (e.g.
applyMacAllSpaces(win)) to keep future window behavior changes less kinda-cursed.Also applies to: 164-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/windows.ts` around lines 59 - 63, Extract the duplicate macOS workspace-visibility logic into a small helper function (e.g. applyMacAllSpaces) that accepts a BrowserWindow (or the same window type used) and calls win.setVisibleOnAllWorkspaces(true, { visibleOnFullScreen: true }); Replace both occurrences (the snippet using win.setVisibleOnAllWorkspaces at lines around the first instance and the second instance) with a call to applyMacAllSpaces(win) and export or keep the helper local as appropriate to the module..github/workflows/ci.yml (1)
34-45: optional: add a timeout to avoid stuck runsBrowser jobs occasionally hang in CI; a
timeout-minutesontestkeeps queue health sane.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 34 - 45, Add a job-level timeout to the Test job to prevent CI hangs: update the GitHub Actions job named "test" to include a timeout-minutes value (e.g., timeout-minutes: 30) at the same indentation level as "runs-on" and "steps" so the job will be automatically canceled if it exceeds the limit; this change affects the job block labeled "test" in the workflow.src/i18n/config.ts (1)
12-12:Locale = stringis lowkey risky for type safety.
this makes any string compile as a locale. nit: cleaner to keep a “validated locale” type (brand/opaque pattern) and only produce it after support checks, while still allowing dynamic runtime locale discovery.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/config.ts` at line 12, The current export export type Locale = string is too permissive; replace it with a branded/opaque type (e.g., type Locale = string & { __brand: 'Locale' }) and add a validator/factory (e.g., validateLocale or createLocale) that performs supported-locale checks and returns the branded Locale only on success; update places that accept raw strings to call the validator (or use a runtime lookup function like isSupportedLocale) so only validated Locale instances are used across functions/classes that rely on Locale..env.example (1)
6-7: specific developer name in example file?the signing identities have "Samir Patil ()" hardcoded - might want to make these more generic placeholders like:
-SIGN_IDENTITY="Developer ID Application: Samir Patil ()" -CSC_NAME="Samir Patil ()" +SIGN_IDENTITY="Developer ID Application: Your Name (TEAM_ID)" +CSC_NAME="Your Name (TEAM_ID)"not a blocker, just kinda weird for an example file to have a specific person's name. contributors might not realize they need to change it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 6 - 7, The .env.example includes a specific developer's name in SIGN_IDENTITY and CSC_NAME; replace those hardcoded values with generic placeholder values (e.g., SIGN_IDENTITY="Developer ID Application: YOUR_NAME (YOUR_TEAM_ID)" and CSC_NAME="YOUR_NAME (YOUR_TEAM_ID)") so contributors know to update them for their own signing identity; update the SIGN_IDENTITY and CSC_NAME entries to use clear placeholders and optionally add a short comment indicating they must be replaced with the contributor's own Apple signing identity/team ID.src/components/video-editor/SettingsPanel.tsx (1)
76-80: lowkey risky: mutating ref + calling setState during renderthis pattern of updating
prevValue.currentand callingsetDraftdirectly in the render body works but isn't the cleanest. react prefers syncing external props to internal state viauseEffect. right now it technically works because the condition guards against unnecessary updates, but it can cause subtle issues during concurrent rendering.♻️ consider using useEffect for prop-to-state sync
- const prevValue = useRef(value); - if (!isFocused && prevValue.current !== value) { - prevValue.current = value; - setDraft(isPreset ? "" : String(Math.round(value))); - } + useEffect(() => { + if (!isFocused) { + const isPresetNow = SPEED_OPTIONS.some((o) => o.speed === value); + setDraft(isPresetNow ? "" : String(Math.round(value))); + } + }, [value, isFocused]);🤖 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 76 - 80, The render body mutates prevValue.current and calls setDraft directly, which can cause issues during concurrent rendering; move this sync into a useEffect that depends on [value, isFocused, isPreset], and inside the effect compare prevValue.current to value, update prevValue.current, and call setDraft(isPreset ? "" : String(Math.round(value))) so state updates happen after render; reference the existing prevValue (useRef), setDraft, value, isFocused and isPreset to locate and replace the inline logic.src/lib/compositeLayout.ts (1)
221-282: split layout math is solid overallgood handling of edge cases:
Math.max(1, ...)prevents zero/negative dimensions- slot positioning accounts for content centering
- same borderRadius applied to both screen and webcam for visual consistency
one minor nit: line 222-226 calls
centerRectto createscreenRect, but it's only used in the early return (line 229). when webcam exists, you computescreenSlotseparately and return that asscreenRect(line 270). could skip thecenterRectcall when webcam is present, but honestly it's negligible overhead.♻️ nit: could avoid unnecessary centerRect call
if (preset.transform.type === "split") { - const screenRect = centerRect({ - canvasSize, - size: screenSize, - maxSize: maxContentSize, - }); - if (!webcamWidth || !webcamHeight || webcamWidth <= 0 || webcamHeight <= 0) { + const screenRect = centerRect({ + canvasSize, + size: screenSize, + maxSize: maxContentSize, + }); return { screenRect, webcamRect: null }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/compositeLayout.ts` around lines 221 - 282, Call centerRect only for the early-return path when webcam is missing instead of unconditionally at the top of the "split" branch: move the centerRect(...) call into the block that returns { screenRect, webcamRect: null } (the path where !webcamWidth || !webcamHeight ...) and keep the existing screenSlot computation for the case where a webcam exists (the variables screenSlot, webcamSlot, and the returned screenRect). This removes the redundant centerRect call while preserving current behavior for both branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 121-125: Replace the hardcoded signing identity string "Samir
Patil (N26FZ4GW28)" with a single reusable secret/variable (e.g.,
SIGNING_IDENTITY) and reference that variable in the Package .app bundle step
(name: Package .app bundle, env: CSC_NAME) and the other signing/codesign step
mentioned (the block at lines 180-186) so both use the same source of truth;
create/consume a repo secret or workflow variable called SIGNING_IDENTITY and
set CSC_NAME: ${{ secrets.SIGNING_IDENTITY }} (or equivalent) in all places
where the identity is passed.
In `@electron/main.ts`:
- Around line 376-383: The switchToHudWrapper function currently sets
isForceClosing = true and calls mainWindow.close(), which bypasses the
unsaved-changes flow; remove the force-close behavior and do not set
isForceClosing in switchToHudWrapper—just call mainWindow.close() (or if
mainWindow is null, call showMainWindow()). Move the HUD recreation logic (the
showMainWindow() call and mainWindow = null assignment) into the mainWindow
'closed' event handler so the HUD is only recreated after the window actually
closes (and only if the close wasn't cancelled by the unsaved dialog); ensure
the closed handler checks whether we should recreate the HUD and only then calls
showMainWindow() and nulls mainWindow.
In `@electron/preload.ts`:
- Around line 21-26: The new bridge methods (switchToHud and startNewRecording)
expand the ipcRenderer surface and must be gated: modify electron/preload.ts to
only expose these methods on trusted renderer windows (e.g., check a window role
flag passed via context/isTrusted boolean or only attach to windows created with
a specific preload/option), and update the corresponding main-process handlers
for the "switch-to-hud" and "start-new-recording" channels to validate the
sender (e.g., verify sender.webContents.id or origin/role of the window that
created the IPC call) before performing app-state transitions; also ensure
createKalturaBrowseWindow does not load remote CDN content into a window that
uses the privileged preload/electronAPI or else create a separate unprivileged
preload for that window.
In `@scripts/build_macos.sh`:
- Around line 15-23: After sourcing ENV_FILE (the block with set -a; source
"$ENV_FILE"; set +a) add an explicit required-variable preflight that checks the
critical env vars (e.g., APP_NAME, SIGN_IDENTITY, NOTARY_PROFILE and any others
your build requires) are defined and non-empty and prints a clear error listing
missing keys before exiting; implement this by collecting missing names into an
array, echoing a helpful message if any are unset, and exit 1. Also add the same
validation before the later section that uses these vars (the block around where
those vars are referenced later) to avoid abrupt failures with set -u.
In `@src/components/video-editor/BlurSettingsPanel.tsx`:
- Around line 30-33: The panel currently lists only "rectangle" and "oval" in
blurShapeOptions so a selected BlurShape of "freehand" yields no active choice;
update blurShapeOptions (in BlurSettingsPanel.tsx) to include { value:
"freehand", labelKey: "blurShapeFreehand" } (or the appropriate labelKey used
elsewhere) so the UI can represent freehand, and ensure any labelKey is added to
i18n if needed; alternatively, if you prefer not to support freehand here, guard
the panel render in SettingsPanel.tsx (where the component is chosen) to skip
rendering BlurSettingsPanel when the active blur shape === "freehand".
In `@src/components/video-editor/timeline/Item.tsx`:
- Line 17: The Item component currently accepts variant?: "zoom" | "trim" |
"annotation" | "speed" | "blur" but the render branch treats unknowns as the
annotation fallback; update the rendering logic inside Item to handle the "blur"
variant explicitly (e.g., add a case for "blur" where you return the correct
blur icon and label instead of falling back to annotation), and adjust any
helper functions or mappings (e.g., getIconForVariant / getLabelForVariant or
the switch/if that branches on variant) to include "blur" so the timeline shows
the proper icon and semantics for blur edits.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 532-540: handleNewRecordingConfirm calls
window.electronAPI.startNewRecording unconditionally which can clear the current
session and lose unsaved edits; update handleNewRecordingConfirm to first check
hasUnsavedChanges and, if true, present the existing save/discard prompt or
invoke the current save-before-close flow (e.g., call the save handler or
discard path used elsewhere) and only call window.electronAPI.startNewRecording
when the user chooses to save or discard; on cancel do not call
startNewRecording and keep setShowNewRecordingDialog and setError behavior
unchanged for failure cases.
In `@src/components/video-editor/videoPlayback/zoomTransform.ts`:
- Around line 93-96: normalizeProjectEditor() must detect v1 projects and
convert their baseMask-relative focus to the new stage-normalized semantics:
when project.version === 1 (or project.version < 2), compute stagePxX =
baseMask.x + storedFocusX * baseMask.width and stagePxY = baseMask.y +
storedFocusY * baseMask.height, then set normalizedFocusX = stagePxX /
stageSize.width and normalizedFocusY = stagePxY / stageSize.height before
clamping; leave focus values untouched for version >= 2. Ensure baseMask and
stageSize exist before transforming to avoid runtime errors.
In `@src/lib/exporter/frameRenderer.ts`:
- Around line 694-723: readbackVideoCanvas currently allocates a new Uint8Array,
Uint8ClampedArray and ImageData every frame; instead add reusable per-size
buffers on the class (e.g. this._readbackBuf: Uint8Array, this._readbackClamped:
Uint8ClampedArray, this._readbackImageData: ImageData, and this._rowTemp:
Uint8Array) and allocate them only when the canvas dimensions (w,h) change, then
reuse them inside readbackVideoCanvas (call gl.readPixels into
this._readbackBuf, perform the vertical flip into the same buffer using
this._rowTemp, update the existing Uint8ClampedArray view and call putImageData
with the cached ImageData) so no new Uint8Array/Uint8ClampedArray/ImageData are
created per frame.
---
Outside diff comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1374-1392: The blur items are given variant: "blur" in
TimelineEditor (the TimelineRenderItem objects) but Item.tsx's rendering logic
doesn't handle "blur", so blur clips still look like annotations; update the
Item component (where it reads item.variant / the switch/branch that handles
"annotation" and "speed") to add a case for "blur" that supplies the appropriate
icon, color/style and any tooltip/label behavior for blur items (use the same
prop names as other variants like speedValue/label) so blur lane items render
distinctively; ensure TimelineRenderItem type handling in Item.tsx recognizes
"blur" as a valid variant.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 282-331: The snapshot created by currentProjectSnapshot omits
webcamSizePreset, causing dirty-state mismatches; update the object passed into
createProjectSnapshot within the currentProjectSnapshot useMemo to include
webcamSizePreset (same identifier), ensuring the snapshot payload contains
webcamSizePreset so changes to it mark the project dirty and align with
saveProject persistence.
- Around line 641-676: The selection-reset behavior is asymmetric:
handleSelectZoom, handleSelectTrim, and handleSelectAnnotation do not clear
selectedSpeedId, which allows the speed UI/state to remain active incorrectly;
update these handlers (handleSelectZoom, handleSelectTrim,
handleSelectAnnotation) to also call setSelectedSpeedId(null) when an id is
selected, and apply the same fix to the equivalent selection/add handlers in the
other ranges mentioned (the selection handlers around 678-729 and the
selection/delete flows around 905-955) so every selection path consistently
clears selectedZoomId, selectedTrimId, selectedAnnotationId, selectedBlurId, and
selectedSpeedId as appropriate.
---
Duplicate comments:
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 186-211: The language menu opens/closes but doesn't move focus
into the menu or implement roving-keyboard navigation, so implement
keyboard-complete behavior: when isLanguageMenuOpen becomes true (use the
existing isLanguageMenuOpen setter), programmatically move focus from
languageTriggerRef into the menu (languageMenuPanelRef) to the selected or first
menuitem, attach a keydown handler on the menu panel to implement
ArrowDown/ArrowUp/Home/End to move focus between menuitem elements (roving
focus), keep Escape closing via setIsLanguageMenuOpen(false), and ensure
Tab/Shift+Tab behavior is handled (prevent tabbing out when the menu should trap
focus or restore focus to languageTriggerRef on close); update/remove the
temporary key listeners in the current useEffect and add the menu keydown
handler and focus management to the components that use languageTriggerRef and
languageMenuPanelRef (also apply same fix to the other menu instance noted).
---
Nitpick comments:
In @.env.example:
- Around line 6-7: The .env.example includes a specific developer's name in
SIGN_IDENTITY and CSC_NAME; replace those hardcoded values with generic
placeholder values (e.g., SIGN_IDENTITY="Developer ID Application: YOUR_NAME
(YOUR_TEAM_ID)" and CSC_NAME="YOUR_NAME (YOUR_TEAM_ID)") so contributors know to
update them for their own signing identity; update the SIGN_IDENTITY and
CSC_NAME entries to use clear placeholders and optionally add a short comment
indicating they must be replaced with the contributor's own Apple signing
identity/team ID.
In @.github/workflows/ci.yml:
- Around line 34-45: Add a job-level timeout to the Test job to prevent CI
hangs: update the GitHub Actions job named "test" to include a timeout-minutes
value (e.g., timeout-minutes: 30) at the same indentation level as "runs-on" and
"steps" so the job will be automatically canceled if it exceeds the limit; this
change affects the job block labeled "test" in the workflow.
In `@electron/i18n.ts`:
- Around line 26-29: The locale whitelist is duplicated in setMainLocale, which
risks divergence; add a single exported constant like SUPPORTED_MAIN_LOCALES =
['en','zh-CN','es','fr'] as const and use it for both the type/union and runtime
check (e.g., SUPPORTED_MAIN_LOCALES.includes(locale as any)) inside
setMainLocale so the allowed values are centralized and the typeguard stays
accurate; update any existing type definitions or references to use this
constant for consistency.
In `@electron/windows.ts`:
- Around line 59-63: Extract the duplicate macOS workspace-visibility logic into
a small helper function (e.g. applyMacAllSpaces) that accepts a BrowserWindow
(or the same window type used) and calls win.setVisibleOnAllWorkspaces(true, {
visibleOnFullScreen: true }); Replace both occurrences (the snippet using
win.setVisibleOnAllWorkspaces at lines around the first instance and the second
instance) with a call to applyMacAllSpaces(win) and export or keep the helper
local as appropriate to the module.
In `@scripts/build_macos.sh`:
- Around line 70-71: The grep call that checks for the signing identity
currently treats SIGN_IDENTITY as a regex which can mis-match when the identity
contains regex-special characters; update the command that uses grep -q
"$SIGN_IDENTITY" (the check after security find-identity) to use fixed-string
matching and safe argument handling by calling grep -Fq -- "$SIGN_IDENTITY" so
the identity is treated literally and won’t be interpreted as a regex.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 76-80: The render body mutates prevValue.current and calls
setDraft directly, which can cause issues during concurrent rendering; move this
sync into a useEffect that depends on [value, isFocused, isPreset], and inside
the effect compare prevValue.current to value, update prevValue.current, and
call setDraft(isPreset ? "" : String(Math.round(value))) so state updates happen
after render; reference the existing prevValue (useRef), setDraft, value,
isFocused and isPreset to locate and replace the inline logic.
In `@src/i18n/config.ts`:
- Line 12: The current export export type Locale = string is too permissive;
replace it with a branded/opaque type (e.g., type Locale = string & { __brand:
'Locale' }) and add a validator/factory (e.g., validateLocale or createLocale)
that performs supported-locale checks and returns the branded Locale only on
success; update places that accept raw strings to call the validator (or use a
runtime lookup function like isSupportedLocale) so only validated Locale
instances are used across functions/classes that rely on Locale.
In `@src/lib/compositeLayout.ts`:
- Around line 221-282: Call centerRect only for the early-return path when
webcam is missing instead of unconditionally at the top of the "split" branch:
move the centerRect(...) call into the block that returns { screenRect,
webcamRect: null } (the path where !webcamWidth || !webcamHeight ...) and keep
the existing screenSlot computation for the case where a webcam exists (the
variables screenSlot, webcamSlot, and the returned screenRect). This removes the
redundant centerRect call while preserving current behavior for both branches.
🪄 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: 1267e777-d307-4108-b9b3-a61b88dc868a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (81)
.env.example.github/workflows/build.yml.github/workflows/ci.yml.gitignoreelectron-builder.json5electron/electron-env.d.tselectron/i18n.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/windows.tsicons/icons/mac/icon.icnsmacos.entitlementspackage.jsonscripts/build_macos.shscripts/i18n-check.mjssrc/components/launch/LaunchWindow.module.csssrc/components/launch/LaunchWindow.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/BlurSettingsPanel.tsxsrc/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/timeline/Item.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/layoutUtils.tssrc/components/video-editor/videoPlayback/zoomTransform.tssrc/contexts/I18nContext.tsxsrc/hooks/useEditorHistory.tssrc/hooks/useScreenRecorder.tssrc/i18n/config.tssrc/i18n/loader.tssrc/i18n/locales/en/editor.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/en/shortcuts.jsonsrc/i18n/locales/en/timeline.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/es/shortcuts.jsonsrc/i18n/locales/es/timeline.jsonsrc/i18n/locales/fr/common.jsonsrc/i18n/locales/fr/dialogs.jsonsrc/i18n/locales/fr/editor.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/fr/shortcuts.jsonsrc/i18n/locales/fr/timeline.jsonsrc/i18n/locales/ko-KR/common.jsonsrc/i18n/locales/ko-KR/dialogs.jsonsrc/i18n/locales/ko-KR/editor.jsonsrc/i18n/locales/ko-KR/launch.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/ko-KR/shortcuts.jsonsrc/i18n/locales/ko-KR/timeline.jsonsrc/i18n/locales/tr/common.jsonsrc/i18n/locales/tr/dialogs.jsonsrc/i18n/locales/tr/editor.jsonsrc/i18n/locales/tr/launch.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/tr/shortcuts.jsonsrc/i18n/locales/tr/timeline.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-CN/shortcuts.jsonsrc/i18n/locales/zh-CN/timeline.jsonsrc/lib/compositeLayout.test.tssrc/lib/compositeLayout.tssrc/lib/exporter/annotationRenderer.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.browser.test.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/streamingDecoder.tssrc/lib/exporter/videoExporter.browser.test.tssrc/lib/exporter/videoExporter.tssrc/lib/shortcuts.tssrc/vite-env.d.tsvitest.browser.config.ts
💤 Files with no reviewable changes (1)
- src/hooks/useScreenRecorder.ts
✅ Files skipped from review due to trivial changes (35)
- .gitignore
- src/i18n/locales/en/shortcuts.json
- src/i18n/locales/fr/common.json
- src/components/video-editor/AnnotationSettingsPanel.tsx
- src/i18n/locales/en/timeline.json
- src/i18n/locales/es/shortcuts.json
- src/i18n/locales/tr/dialogs.json
- src/i18n/locales/es/timeline.json
- macos.entitlements
- src/i18n/locales/tr/common.json
- src/components/launch/LaunchWindow.module.css
- src/i18n/locales/en/editor.json
- src/i18n/locales/zh-CN/editor.json
- src/i18n/locales/ko-KR/dialogs.json
- src/i18n/locales/en/settings.json
- src/i18n/locales/fr/dialogs.json
- src/i18n/locales/fr/shortcuts.json
- src/i18n/locales/zh-CN/shortcuts.json
- src/i18n/locales/ko-KR/common.json
- src/i18n/locales/fr/editor.json
- src/i18n/locales/tr/editor.json
- src/i18n/locales/fr/launch.json
- src/i18n/locales/tr/launch.json
- src/i18n/locales/tr/shortcuts.json
- src/i18n/locales/ko-KR/editor.json
- src/i18n/locales/ko-KR/launch.json
- src/i18n/locales/zh-CN/timeline.json
- src/i18n/locales/fr/timeline.json
- src/i18n/locales/tr/timeline.json
- src/i18n/locales/ko-KR/shortcuts.json
- src/i18n/locales/zh-CN/settings.json
- src/i18n/locales/fr/settings.json
- src/i18n/locales/tr/settings.json
- src/i18n/locales/ko-KR/settings.json
- src/i18n/locales/ko-KR/timeline.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/contexts/I18nContext.tsx
3 tasks
Collaborator
Author
|
done bro, created a new PR #362 |
siddharthvaddem
approved these changes
Apr 16, 2026
This was referenced May 3, 2026
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 improves the Launch/HUD recording controls UX and language switching flow.
It includes:
en,es, andzh-CNlaunch stringsMotivation
The current recording HUD and language selector behavior felt inconsistent and distracting in active recording flow.
This change improves clarity and usability by:
Type of Change
Related Issue(s)
Fixes #356 & partially #363
Screenshots / Video
Summary by CodeRabbit
Release Notes