feat(launch): refine recording HUD and language switching UX#362
Open
imAaryash wants to merge 92 commits intosiddharthvaddem:mainfrom
Open
feat(launch): refine recording HUD and language switching UX#362imAaryash wants to merge 92 commits intosiddharthvaddem:mainfrom
imAaryash wants to merge 92 commits intosiddharthvaddem:mainfrom
Conversation
add 3x, 4x, 5x speed presets and a custom playback speed input field that accepts any integer value up to 16x. change PlaybackSpeed type from a fixed union to number with min/max constants and clamp utility. update project persistence to validate any speed in range instead of exact value matching. add i18n keys for en, es, zh-CN. closes siddharthvaddem#252
…onment variables for notarization
On Linux/Wayland the implicit GPU-to-2D texture-sharing path used by drawImage(webglCanvas) fails silently (EGL/Ozone), producing green frames. Use explicit gl.readPixels to copy from GPU to CPU memory, bypassing that path.
…(plus lint fixes)
Updated CI workflow to include E2E tests conditionally.
📝 WalkthroughWalkthroughAdds system-language detection and a confirmation modal; moves language picker into a HUD sidebar portal; implements blur annotation tooling end-to-end (UI, types, timeline, rendering, exporter); introduces webcam size presets and threading; expands i18n locales and test infra; plus macOS build/signing tooling and related Electron IPC/preload APIs. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
|
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. |
…CN-missing-newRecording-translation fix(i18n): add missing zh-CN translation for newRecording dialog
…293-fix/restart-recording-windows Revert "fix: prevent double-finalize race condition in restartRecording on Windos"
siddharthvaddem
requested changes
Apr 11, 2026
Fix SUPPORTED_LOCALES array syntax
feat(i18n): add Korean (ko-KR) localization
…ws-spaces fix: HUD overlay and source selector follow across macOS Spaces
fix: adjust icon size for macOS platform compatibility
feat: configure macOS hardened runtime, entitlements, and build envir…
…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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Contributor
Author
|
done bro, created a new PR #362 |
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
New Features
UI / Bug Fixes
Localization
Tests