feat: settings tab with language picker, extension fixes, and i18n expansion#249
Merged
webadderall merged 9 commits intomainfrom Apr 17, 2026
Merged
feat: settings tab with language picker, extension fixes, and i18n expansion#249webadderall merged 9 commits intomainfrom
webadderall merged 9 commits intomainfrom
Conversation
…pansion - SettingsPanel: new 'settings' section — in-app language picker (all 5 locales), auto-apply fresh recording zooms toggle, connect-neighboring- zooms toggle, keybinds shortcut into KeyboardShortcutsDialog - VideoEditor: wire autoApplyFreshRecordingAutoZooms preference through to SettingsPanel; connect zoom settings into new section - editorPreferences: add autoApplyFreshRecordingAutoZooms field; update tests - extensionMarketplace: filter __MACOSX sidecar dirs when unzipping extensions uploaded from macOS Finder (fixes manifest-not-found on Finder zips) - useExtensions: guard window.electronAPI for SSR safety; reformat to tabs - I18nContext: expose setLocale; persist locale preference - ShortcutsContext: shortcut registration improvements - lib/extensions: renderHooks, extensionHost, fileUrls, cursorCoordinates updated for new extension capability surface - i18n: expand extension locale strings across all 5 supported languages
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:
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Settings & Editor UI src/components/video-editor/SettingsPanel.tsx, src/components/video-editor/VideoEditor.tsx, src/components/video-editor/TutorialHelp.tsx, src/components/video-editor/ExtensionManager.tsx, src/components/video-editor/index.ts |
Refactored SettingsPanel into sectioned renders (settings/zoom/clip), added clip mute/speed and auto-apply fresh-recording auto-zoom props/state, removed default export for SettingsPanel, swapped lucide→@phosphor icons, moved keyboard-shortcuts trigger to props, and adjusted header/toolbar rendering. |
Timeline & Playback Mapping src/components/video-editor/timeline/TimelineEditor.tsx, src/components/video-editor/VideoEditor.tsx |
Converted TimelineEditor to forwardRef exposing TimelineEditorHandle, added playheadTime and playback props, introduced timeline↔source time mapping for playback/seek, remapped zoom regions through timeline mapping, and added timeline-invokable actions (addZoom/suggest/split/addAnnotation/addAudio). |
Types & Persistence src/components/video-editor/types.ts, src/components/video-editor/projectPersistence.ts |
Added muted?: boolean to ClipRegion and trackIndex?: number to Audio/Annotation regions, added exported getClipSourceEndMs(), normalized clip muted and trackIndex in persistence, and constrained zoom mode normalization. |
Preferences & Tests src/components/video-editor/editorPreferences.ts, src/components/video-editor/editorPreferences.test.ts |
Added autoApplyFreshRecordingAutoZooms preference (default true) with normalization; tests updated to include new preference and frame expectations. |
I18n / Locale Files src/contexts/I18nContext.tsx, src/i18n/config.ts, src/i18n/locales/*/{settings,extensions,editor,timeline,launch}.json |
Adjusted locale bundle imports, added/updated translation keys (zoom modes/descriptions, clip labels, toolbar/timeline actions, audio label, toast.enableFailed, floating webcam preview strings). |
Extensions & Hooks Robustness src/hooks/useExtensions.ts, src/contexts/ShortcutsContext.tsx, src/lib/extensions/* |
Made electronAPI access defensive for non-browser contexts, added try/catch/finally around discovery/activation, ensured canvas ctx.save()/restore() around extension hooks, and small API/formatting tweaks. |
Misc / Packaging / Formatting package.json, src/lib/extensions/*, src/lib/extensions/index.ts |
Added @phosphor-icons/react dependency; multiple formatting, import reorder, newline fixes and re-export order tweaks without API-signature changes. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant VideoEditor
participant TimelineEditor
participant SettingsPanel
participant ExportPipeline
rect rgba(100,150,200,0.5)
note over User,VideoEditor: Clip edit flows (mute/speed/split)
User->>VideoEditor: request clip speed change / split / mute
VideoEditor->>VideoEditor: compute getClipSourceEndMs and remap zoom regions
VideoEditor->>TimelineEditor: update zoomRegions & playheadTime
TimelineEditor->>TimelineEditor: render remapped timeline
VideoEditor->>SettingsPanel: update clip controls (mute/speed state)
end
rect rgba(150,100,200,0.5)
note over TimelineEditor,ExportPipeline: Timeline ↔ source mapping for seek/export
User->>TimelineEditor: seek timeline (timelineTime)
TimelineEditor->>VideoEditor: onSeek(timelineTime)
VideoEditor->>VideoEditor: mapTimelineTimeToSourceTime(timelineTime)
VideoEditor->>ExportPipeline: provide source time + effectiveZoomRegions
ExportPipeline->>ExportPipeline: apply clip speeds and zooms for export
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- webadderall/Recordly#124 — Overlaps SettingsPanel/TimelineEditor/types/prefs edits; likely overlapping changes.
- webadderall/Recordly#143 — Related SettingsPanel rendering/zoom-region UI refactor.
- webadderall/Recordly#122 — Touches timeline, zoom/clip APIs and preference/type wiring; high potential for conflicts.
Suggested labels
Checked
Poem
🐰 I hopped through timelines with a tiny drum,
Clips can whisper, speed and zoom become,
Panels rearranged and icons swapped anew,
Languages and mappings stitched into view,
A cheerful rabbit stamps: the editor grew. 🎉
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 25.45% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main feature additions: a settings tab with language picker, extension-related fixes, and i18n expansion. |
| Description check | ✅ Passed | The description covers the key changes well, including settings tab, preferences, extension fixes, i18n context, and locale expansion, matching the template's required sections. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/settings-extensions
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 help to get the list of available commands and usage tips.
![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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/extensions/extensionHost.ts (1)
297-305:⚠️ Potential issue | 🟠 MajorAlways restore the canvas state when a hook throws.
If
hook.hook(context)throws, Line 300 is skipped and the saved transform/clip leaks into later hooks. That makes one bad extension corrupt the rest of the render pipeline.Proposed fix
for (const hook of hooks) { - try { - context.ctx.save(); + context.ctx.save(); + try { hook.hook(context); - context.ctx.restore(); } catch (err) { console.warn( `[extensions] Render hook error (${hook.extensionId}, ${phase}):`, err, ); + } finally { + context.ctx.restore(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/extensions/extensionHost.ts` around lines 297 - 305, The render hook wrapper currently calls context.ctx.save(), then hook.hook(context), and context.ctx.restore() inside a try block so if hook.hook(context) throws the restore is skipped; change the control flow in the render loop around the save/hook/restore (the code that calls context.ctx.save(), hook.hook(context), context.ctx.restore()) to ensure context.ctx.restore() is always executed by moving the restore into a finally block (or equivalent) so every invocation of hook.hook(context) is paired with a restore even on exceptions.src/components/video-editor/VideoEditor.tsx (3)
1776-1815:⚠️ Potential issue | 🟠 MajorThe new auto-zoom preference is never saved.
This effect reruns when
autoApplyFreshRecordingAutoZoomschanges, but the object passed tosaveEditorPreferences()omits that field. The toggle will revert on restart.Proposed fix
saveEditorPreferences({ wallpaper, shadowIntensity, backgroundBlur, zoomMotionBlur, + autoApplyFreshRecordingAutoZooms, connectZooms, zoomInDurationMs,🤖 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 1776 - 1815, The saveEditorPreferences call is missing the auto-zoom preference so autoApplyFreshRecordingAutoZooms never persists; update the object passed to saveEditorPreferences inside the effect to include autoApplyFreshRecordingAutoZooms (use the same property name as the state/prop) so the preference is saved, referencing the saveEditorPreferences invocation and the autoApplyFreshRecordingAutoZooms variable in VideoEditor.tsx.
1724-1773:⚠️ Potential issue | 🟠 MajorToggling this preference reloads the active editor state.
Because
autoApplyFreshRecordingAutoZoomsis in this effect's dependency list, flipping the switch rerunsloadInitialData(), which can reopen the current project/session and reset in-memory edits. Keep the initial load effect stable and updatependingFreshRecordingAutoZoomPathRefin a separate small effect instead.Minimal fix for the immediate reload issue
loadInitialData(); }, [ applyLoadedProject, - autoApplyFreshRecordingAutoZooms, smokeExportConfig.enabled, smokeExportConfig.inputPath, ]);🤖 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 1724 - 1773, The effect that runs loadInitialData should not depend on autoApplyFreshRecordingAutoZooms because toggling it causes a full reload; remove autoApplyFreshRecordingAutoZooms from the dependency array of the useEffect that defines loadInitialData (the effect referencing loadInitialData and setLoading). Instead add a small separate effect that watches autoApplyFreshRecordingAutoZooms (and the current video path state, e.g., videoPath or videoSourcePath) and updates pendingFreshRecordingAutoZoomPathRef.current accordingly: set it to videoPath when autoApplyFreshRecordingAutoZooms is true, otherwise null. This preserves loadInitialData stability while keeping pendingFreshRecordingAutoZoomPathRef in sync.
5123-5126:⚠️ Potential issue | 🟠 MajorFeed timeline-space zooms into
TimelineEditor.
effectiveZoomRegionsare already remapped into source time for playback/export. Passing them into the timeline editor makes the UI render/edit zooms in the wrong coordinate space once clip speed remapping is active.Proposed fix
- zoomRegions={effectiveZoomRegions} + zoomRegions={zoomRegions}🤖 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 5123 - 5126, The TimelineEditor is receiving effectiveZoomRegions (which are remapped into source time) so the timeline UI shows/edit zooms in the wrong coordinate space when clip speed remapping is active; in the VideoEditor where TimelineEditor is rendered (the zoomRegions prop currently set to effectiveZoomRegions), pass timeline-space zooms instead — either the original timeline zoomRegions variable (pre-remap) or run effectiveZoomRegions through the inverse remap function (e.g., remapSourceToTimelineTime / remapZoomsToTimelineSpace) before passing to TimelineEditor (update the prop usage where autoSuggestZoomsTrigger/onZoomAdded are wired).src/components/video-editor/projectPersistence.ts (1)
149-154:⚠️ Potential issue | 🟠 MajorDon't flip legacy projects to the modern exporter by default yet.
Projects that never persisted
exportPipelineModelwill now silently switch export behavior on load, whileExportSettingsMenustill defaults to"legacy"insrc/components/video-editor/ExportSettingsMenu.tsx:45-65. Please align every implicit default first, or add an explicit migration instead of changing the normalizer fallback in place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/projectPersistence.ts` around lines 149 - 154, The current normalizeExportPipelineModel function flips unknown/absent values to "modern", which will silently change legacy projects; change this to preserve the existing implicit default by returning "legacy" for missing/unknown values (or alternatively implement an explicit migration step where you detect absent exportPipelineModel on project load and set it intentionally), and make sure this behavior aligns with ExportSettingsMenu's default ("legacy") so that normalizeExportPipelineModel (in projectPersistence.ts) and ExportSettingsMenu (in ExportSettingsMenu.tsx) are consistent.
🧹 Nitpick comments (1)
src/i18n/locales/en/extensions.json (1)
2-59: Inconsistent indentation style with other locale files.This file uses 8-space indentation while other locale files (
ko,nl,zh-CN,es) use tab-based indentation. For consistency across the codebase, consider reformatting to match the tab-based style used in other locale files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locales/en/extensions.json` around lines 2 - 59, The JSON locale uses 8-space indentation which is inconsistent with other locale files; reformat the file to use tab-based indentation to match the style of existing locales and preserve all keys such as "title", "tabs", "actions", "status", "detail", "empty", "search", and "toast" (ensure values like "submit", "browse", "installedAndEnabled", etc. remain unchanged while switching all leading indentation to tabs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/projectPersistence.ts`:
- Around line 365-384: The normalization in normalizedClipRegions (processing
editor.clipRegions) drops the clip's muted state by rebuilding each ClipRegion
without a muted field; update the mapping inside normalizedClipRegions (the .map
callback) to preserve the region.muted property when present (e.g., set muted:
Boolean(region.muted) or region.muted ?? false) so saved projects retain clip
mute state; ensure the returned object for each region includes the muted flag
alongside id, startMs, endMs, and speed.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 2187-2218: These new controls in SettingsPanel.tsx render
hardcoded English strings (e.g., the labels "Auto-apply fresh recording zooms"
and "Connect neighboring zooms" shown alongside the Switches bound to
autoApplyFreshRecordingAutoZooms/onAutoApplyFreshRecordingAutoZoomsChange and
connectZooms/onConnectZoomsChange); replace each hardcoded copy with the i18n
translation function (e.g., t('settings.autoApplyFreshRecordingZooms') and
t('settings.connectNeighboringZooms')) and wire the same keys into the locale
files so they translate after setLocale(...) — do the same for the other
affected blocks mentioned (lines ~2242-2290 and ~2376-2445) by converting all
literal strings like "Manual", "Mute Audio", "Delete Clip", etc., into t('...')
keys and adding those keys to the translation JSONs.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2655-2665: When splitting a clip in VideoEditor.tsx the newly
created ClipRegion objects (left and right) copy target.speed but omit
target.muted, which causes muted clips to become unmuted after split; update the
left and right ClipRegion creation to include muted: target.muted so both halves
preserve the original muted state (refer to variables left, right, ClipRegion,
target, splitMs, leftId, rightId).
- Around line 3576-3582: The computed backendPreference currently falls back to
"auto" whenever smokeExportConfig.enabled is false, causing the
exportBackendPreference state/menu selection to be ignored; update the
backendPreference calculation (the expression that assigns backendPreference
using pipelineModel and smokeExportConfig) to use the exportBackendPreference
state value when not in smoke-export mode, preserving the legacy => "webcodecs"
mapping for pipelineModel === "legacy" and keeping the smoke-export logic intact
(i.e., when smokeExportConfig.enabled use smokeExportConfig.backendPreference or
smokeExportConfig.useNativeExport), so ModernVideoExporter receives the actual
selected exportBackendPreference instead of always "auto".
In `@src/contexts/ShortcutsContext.tsx`:
- Around line 41-48: The optional chain ends before .then causing a runtime
error when window.electronAPI is undefined; update the mount logic in
ShortcutsContext to first check or await the IPC method instead of chaining
.then on a possibly undefined value: e.g., test for
window.electronAPI?.getShortcuts before calling it (or convert the logic to an
async function and use try/catch around await
window.electronAPI.getShortcuts()), then call
setShortcuts(mergeWithDefaults(saved as Partial<ShortcutsConfig>)) only when
saved is truthy and log or ignore errors safely; target the code around the call
to window.electronAPI?.getShortcuts, setShortcuts, and mergeWithDefaults to
implement this guard/async-await+try/catch pattern.
In `@src/hooks/useExtensions.ts`:
- Around line 146-149: The catch block in the toggleExtension flow (in
useExtensions.ts, the async toggleExtension function that uses
activatingRef.current) should not rethrow the error because callers (e.g.,
ExtensionManager.tsx calling toggleExtension(detailData.ext.manifest.id)) don’t
await/catch it; remove the "throw err" and instead swallow the rejection by
logging the error and returning a resolved value (e.g., return false or the
previous state) so the promise never rejects unexpectedly; ensure the finally
block still runs to call activatingRef.current.delete(id).
- Around line 73-79: discoverAndSync currently returns early when
electronAPI?.extensionsDiscover is missing and can also exit on rejection from
electronAPI.extensionsDiscover() without calling setReady(true), leaving
consumers stuck; modify discoverAndSync so setReady(true) is always executed on
every exit path (for example, call setReady(true) before the early return when
extensions are unavailable and wrap the await
electronAPI.extensionsDiscover()/extensionHost.syncConfiguredExtensions calls in
try/catch/finally, invoking setReady(true) in the finally block), and ensure you
still return an empty array on the unsupported path or rethrow errors as
appropriate.
---
Outside diff comments:
In `@src/components/video-editor/projectPersistence.ts`:
- Around line 149-154: The current normalizeExportPipelineModel function flips
unknown/absent values to "modern", which will silently change legacy projects;
change this to preserve the existing implicit default by returning "legacy" for
missing/unknown values (or alternatively implement an explicit migration step
where you detect absent exportPipelineModel on project load and set it
intentionally), and make sure this behavior aligns with ExportSettingsMenu's
default ("legacy") so that normalizeExportPipelineModel (in
projectPersistence.ts) and ExportSettingsMenu (in ExportSettingsMenu.tsx) are
consistent.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1776-1815: The saveEditorPreferences call is missing the auto-zoom
preference so autoApplyFreshRecordingAutoZooms never persists; update the object
passed to saveEditorPreferences inside the effect to include
autoApplyFreshRecordingAutoZooms (use the same property name as the state/prop)
so the preference is saved, referencing the saveEditorPreferences invocation and
the autoApplyFreshRecordingAutoZooms variable in VideoEditor.tsx.
- Around line 1724-1773: The effect that runs loadInitialData should not depend
on autoApplyFreshRecordingAutoZooms because toggling it causes a full reload;
remove autoApplyFreshRecordingAutoZooms from the dependency array of the
useEffect that defines loadInitialData (the effect referencing loadInitialData
and setLoading). Instead add a small separate effect that watches
autoApplyFreshRecordingAutoZooms (and the current video path state, e.g.,
videoPath or videoSourcePath) and updates
pendingFreshRecordingAutoZoomPathRef.current accordingly: set it to videoPath
when autoApplyFreshRecordingAutoZooms is true, otherwise null. This preserves
loadInitialData stability while keeping pendingFreshRecordingAutoZoomPathRef in
sync.
- Around line 5123-5126: The TimelineEditor is receiving effectiveZoomRegions
(which are remapped into source time) so the timeline UI shows/edit zooms in the
wrong coordinate space when clip speed remapping is active; in the VideoEditor
where TimelineEditor is rendered (the zoomRegions prop currently set to
effectiveZoomRegions), pass timeline-space zooms instead — either the original
timeline zoomRegions variable (pre-remap) or run effectiveZoomRegions through
the inverse remap function (e.g., remapSourceToTimelineTime /
remapZoomsToTimelineSpace) before passing to TimelineEditor (update the prop
usage where autoSuggestZoomsTrigger/onZoomAdded are wired).
In `@src/lib/extensions/extensionHost.ts`:
- Around line 297-305: The render hook wrapper currently calls
context.ctx.save(), then hook.hook(context), and context.ctx.restore() inside a
try block so if hook.hook(context) throws the restore is skipped; change the
control flow in the render loop around the save/hook/restore (the code that
calls context.ctx.save(), hook.hook(context), context.ctx.restore()) to ensure
context.ctx.restore() is always executed by moving the restore into a finally
block (or equivalent) so every invocation of hook.hook(context) is paired with a
restore even on exceptions.
---
Nitpick comments:
In `@src/i18n/locales/en/extensions.json`:
- Around line 2-59: The JSON locale uses 8-space indentation which is
inconsistent with other locale files; reformat the file to use tab-based
indentation to match the style of existing locales and preserve all keys such as
"title", "tabs", "actions", "status", "detail", "empty", "search", and "toast"
(ensure values like "submit", "browse", "installedAndEnabled", etc. remain
unchanged while switching all leading indentation to tabs).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7c8f3841-474b-4feb-a929-0123670284f8
📒 Files selected for processing (20)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/editorPreferences.test.tssrc/components/video-editor/editorPreferences.tssrc/components/video-editor/projectPersistence.tssrc/contexts/I18nContext.tsxsrc/contexts/ShortcutsContext.tsxsrc/hooks/useExtensions.tssrc/i18n/config.tssrc/i18n/locales/en/extensions.jsonsrc/i18n/locales/es/extensions.jsonsrc/i18n/locales/ko/extensions.jsonsrc/i18n/locales/nl/extensions.jsonsrc/i18n/locales/zh-CN/extensions.jsonsrc/lib/extensions/cursorCoordinates.tssrc/lib/extensions/extensionHost.tssrc/lib/extensions/fileUrls.tssrc/lib/extensions/index.tssrc/lib/extensions/renderHooks.tssrc/lib/extensions/types.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/video-editor/timeline/TimelineEditor.tsx (1)
644-700:⚠️ Potential issue | 🟠 MajorKeep annotation cycling on the new playhead time source.
currentTimeMsnow usesplayheadTime ?? currentTime, but the Tab handler below still derives overlap from rawcurrentTime. When those diverge, cycling selects annotations for the wrong moment.🤖 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 644 - 700, The Tab key annotation-cycling logic is still computing overlaps from the raw currentTime while the rest of the component uses currentTimeMs (which incorporates playheadTime); update the Tab handler in TimelineEditor to use currentTimeMs (or Math.round((playheadTime ?? currentTime) * 1000) where currentTimeMs is referenced) when deriving overlap/selection for annotations so cycling selects based on the playhead-aware time source; keep the same units (ms) and fallback behavior.src/components/video-editor/SettingsPanel.tsx (1)
1375-1390:⚠️ Potential issue | 🟡 MinorLocalize the new video-background toasts.
These messages bypass i18n, so this flow stays English even after switching the app language.
🤖 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 1375 - 1390, The toasts in handleVideoUpload are hard-coded English; update handleVideoUpload to use the app i18n strings (e.g., via the translation function used elsewhere such as t or useTranslation) instead of literal text for toast.success and toast.error messages and the "Unsupported format" description; locate handleVideoUpload and replace the plain strings passed to toast.error/toast.success with translated keys (e.g., t('video.added'), t('video.import_failed'), t('video.unsupported_format') or whatever key names your app uses) so the messages follow the app's localization pattern while keeping the same toast calls and surrounding logic (references: handleVideoUpload, isVideoWallpaperSource, setCustomImages, onWallpaperChange, toast.success, toast.error).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/projectPersistence.ts`:
- Around line 305-339: The normalization currently rebuilds each ZoomRegion and
drops the mode field, causing manual-focus zooms to revert; update the mapper
that produces normalizedZoomRegions to copy/validate region.mode into the
returned object (e.g., preserve region.mode when it's a recognized value,
otherwise fall back to the default mode), alongside existing fields like id,
startMs, endMs, depth and focus; reference the normalizedZoomRegions mapping and
DEFAULT_ZOOM_DEPTH to locate where to add the mode preservation and validation
logic.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 2229-2237: The keybind strings are being translated with
tSettings("keyboardShortcuts.*") but those keys live in the editor namespace,
causing fallbacks to English; in SettingsPanel (where SectionLabel and
KeyboardShortcutsDialog are used) replace calls to tSettings for the
keyboardShortcuts keys with the editor namespace translator (e.g., tEditor or
the same translator used elsewhere in this file for editor strings) so the
KeyboardShortcutsDialog triggerLabel and any related labels pull from the editor
namespace translations instead of settings.json.
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1371-1379: The public imperative handle created in
useImperativeHandle exposes toggleCollapsed but it's a no-op and causes
lint/type failures; either implement it to actually toggle the editor collapse
state (hook into the existing collapse state and call the state setter or
collapse handler inside toggleCollapsed) or remove toggleCollapsed from the
exported TimelineEditorHandle type and from the object returned by
useImperativeHandle (update the useImperativeHandle call and the
TimelineEditorHandle definition so they stay consistent). Ensure references to
toggleCollapsed are updated elsewhere if you choose removal and keep other
handles (addZoom, suggestZooms, splitClip, addAnnotation, addAudio, keyframes)
unchanged.
- Around line 690-691: The prop default for onAspectRatioChange in
TimelineEditor (currently "onAspectRatioChange = () => {}") should be removed to
avoid an empty block; instead leave it undefined in the props signature (or
replace with a shared noop constant if your codebase uses one) and update call
sites to use optional chaining (e.g., onAspectRatioChange?.(...)) so missing
wiring isn't masked; modify the props destructuring where onAspectRatioChange is
declared and any places that call onAspectRatioChange to use the safe optional
call.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 4920-5079: The toolbar contains hardcoded user-facing strings
(e.g., the "Add Layer" label in the DropdownMenuTrigger, the DropdownMenuItem
texts "Annotation" and "Audio", button titles like "Add Zoom (Z)", "Suggest
Zooms from Cursor", "Split Clip (C)", playback/control titles "Skip Back",
"Play"/"Pause", "Skip Forward", the timeline collapse titles, and the volume
button title "Mute/Unmute") that bypass localization; replace each literal with
the project's localization calls (e.g., useTranslation().t or i18n.t depending
on your codebase) inside the VideoEditor component so visible labels and title
props call the translation function (e.g., t('videoEditor.addLayer'),
t('videoEditor.annotation'), t('videoEditor.addZoom'), etc.), and ensure you
import/use the same i18n hook already used elsewhere in this file and update the
keys consistently.
- Around line 1774-1777: The ref pendingFreshRecordingAutoZoomPathRef is being
set whenever autoApplyFreshRecordingAutoZooms and videoPath are truthy, which
re-arms fresh-recording auto-zoom for non-fresh videos; change the effect to
also require the "fresh" indicator before setting the ref (e.g., include the
boolean that denotes a fresh recording—such as isFreshRecording or similar in
your component state/props) so only when autoApplyFreshRecordingAutoZooms &&
videoPath && isFreshRecording do you set
pendingFreshRecordingAutoZoomPathRef.current = videoPath, otherwise set it to
null; update the dependency array to include that freshness flag.
- Around line 2718-2725: Validate the incoming speed parameter before doing
duration math: ensure speed is finite and greater than zero (e.g.,
Number.isFinite(speed) && speed > 0), and if invalid, bail out early (return) or
clamp to a safe minimum so you never divide by zero or a non-finite value when
computing sourceDurationMs, newEndMs, and scaleFactor; apply this check at the
start of the arrow handler that references selectedClipId, clipRegions,
clip.speed, oldSpeed, sourceDurationMs, newEndMs, and scaleFactor.
---
Outside diff comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1375-1390: The toasts in handleVideoUpload are hard-coded English;
update handleVideoUpload to use the app i18n strings (e.g., via the translation
function used elsewhere such as t or useTranslation) instead of literal text for
toast.success and toast.error messages and the "Unsupported format" description;
locate handleVideoUpload and replace the plain strings passed to
toast.error/toast.success with translated keys (e.g., t('video.added'),
t('video.import_failed'), t('video.unsupported_format') or whatever key names
your app uses) so the messages follow the app's localization pattern while
keeping the same toast calls and surrounding logic (references:
handleVideoUpload, isVideoWallpaperSource, setCustomImages, onWallpaperChange,
toast.success, toast.error).
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 644-700: The Tab key annotation-cycling logic is still computing
overlaps from the raw currentTime while the rest of the component uses
currentTimeMs (which incorporates playheadTime); update the Tab handler in
TimelineEditor to use currentTimeMs (or Math.round((playheadTime ?? currentTime)
* 1000) where currentTimeMs is referenced) when deriving overlap/selection for
annotations so cycling selects based on the playhead-aware time source; keep the
same units (ms) and fallback behavior.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f41a62b6-88b8-41c2-a10f-785afb056ebc
📒 Files selected for processing (15)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/TutorialHelp.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/contexts/ShortcutsContext.tsxsrc/hooks/useExtensions.tssrc/i18n/locales/en/extensions.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/nl/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/lib/extensions/extensionHost.ts
✅ Files skipped from review due to trivial changes (5)
- src/i18n/locales/ko/settings.json
- src/i18n/locales/es/settings.json
- src/contexts/ShortcutsContext.tsx
- src/i18n/locales/zh-CN/settings.json
- src/lib/extensions/extensionHost.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/i18n/locales/en/extensions.json
- src/hooks/useExtensions.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/video-editor/VideoEditor.tsx (1)
1218-1317:⚠️ Potential issue | 🟠 MajorAdd
frameto this memo's dependency list.
frameis part of the memoized persisted state, but it is missing from the dependency array. If the user changes only the frame, Line 1559 keeps using stale project data until some unrelated state also changes.Proposed fix
[ buildPersistedEditorState, wallpaper, @@ borderRadius, padding, + frame, webcam, zoomRegions,🤖 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 1218 - 1317, currentPersistedEditorState calls buildPersistedEditorState with frame included in the state object but frame is missing from the useMemo dependency array; add frame to the dependency array for the useMemo that defines currentPersistedEditorState so updates to frame will recompute the memoized value (look for the useMemo that declares currentPersistedEditorState and ensure "frame" is present in its dependency list).src/components/video-editor/timeline/TimelineEditor.tsx (1)
1312-1316:⚠️ Potential issue | 🟠 MajorUse the timeline playhead for annotation cycling.
Line 1313 still derives overlap from
currentTimeinstead of the new timeline-aware playhead. In sped-up / remapped clips, Tab will cycle annotations for the wrong moment even though add/seek/cursor logic already usesplayheadTime.Proposed fix
- const currentTimeMs = Math.round(currentTime * 1000); + const currentTimeMs = Math.round((playheadTime ?? currentTime) * 1000);🤖 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 1312 - 1316, The Tab key handler uses currentTime to find overlapping annotations, but the timeline-aware playhead value playheadTime should be used instead; in the block inside the e.key === 'Tab' check (where annotationRegions is filtered/sorted) replace uses of currentTime/currentTimeMs with playheadTime (compute const currentTimeMs = Math.round(playheadTime * 1000)) so cycling uses the timeline playhead consistent with add/seek/cursor logic.
♻️ Duplicate comments (2)
src/components/video-editor/VideoEditor.tsx (2)
5011-5016:⚠️ Potential issue | 🟡 MinorLocalize the play/pause tooltip too.
Line 5016 still hardcodes English even though this PR adds localized playback labels, so language switching leaves this control partially untranslated.
Proposed fix
- title={isPlaying ? "Pause" : "Play"} + title={ + isPlaying + ? t("editor.playback.pause") + : t("editor.playback.play") + }🤖 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 5011 - 5016, The Button's title prop is hardcoded to English; replace title={isPlaying ? "Pause" : "Play"} with the component's existing localized playback labels so the tooltip follows language switching (e.g. title={isPlaying ? playbackLabels.pause : playbackLabels.play} or title={t(isPlaying ? 'pause' : 'play')} depending on the localization helper used). Update the usage near the Button with variant/size/togglePlayPause/isPlaying to reference those localized keys and ensure any needed import or hook (playbackLabels or t) already present in the component is used.
1742-1751:⚠️ Potential issue | 🟠 MajorDon't arm fresh-recording auto-zoom from the generic video path.
Lines 1750-1751 still set
pendingFreshRecordingAutoZoomPathReffor anygetCurrentVideoPath()result, so a normal loaded video can be treated like a fresh recording and trigger auto-suggest.Proposed fix
- pendingFreshRecordingAutoZoomPathRef.current = - autoApplyFreshRecordingAutoZooms ? sourceVideoUrl : null; + pendingFreshRecordingAutoZoomPathRef.current = null;🤖 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 1742 - 1751, The code currently arms pendingFreshRecordingAutoZoomPathRef for any path returned by window.electronAPI.getCurrentVideoPath(), which treats loaded videos like fresh recordings; only set pendingFreshRecordingAutoZoomPathRef.current when the returned result explicitly represents a fresh recording (e.g., check a flag such as result.isFreshRecording or result.isTemporaryRecording) AND autoApplyFreshRecordingAutoZooms is true. Update the getCurrentVideoPath call/contract (or detect via a temp/fresh-recording path pattern) and change the block around setVideoSourcePath/setVideoPath to conditionally set pendingFreshRecordingAutoZoomPathRef.current only when that fresh-recording indicator is present; reference symbols: window.electronAPI.getCurrentVideoPath, pendingFreshRecordingAutoZoomPathRef, autoApplyFreshRecordingAutoZooms, setVideoSourcePath, setVideoPath.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 88-92: TimelineEditor is not reading the passed hideToolbar prop,
so wire it through: add hideToolbar to the TimelineEditor props/interface
(matches the snippet prop name), destructure it in the TimelineEditor component,
and use it to conditionally skip rendering the built-in toolbar (or pass it down
to the internal toolbar component, e.g., TimelineToolbar) so the parent
VideoEditor’s toolbar isn’t duplicated; ensure any child components that render
the toolbar (lines referenced ~643-694) also accept and honor the hideToolbar
boolean.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2655-2677: handleClipSplit currently splits a ClipRegion but
leaves selectedClipId pointing at the removed region; update the selection
inside handleClipSplit so the UI doesn't remain stale. After creating
leftId/rightId (and before returning the new array) check if selectedClipId ===
target.id (use the component state setter setSelectedClipId) and either
setSelectedClipId(leftId) (or rightId) or clear it (null) based on desired UX.
Modify the useCallback for handleClipSplit to import/close over selectedClipId
and call setSelectedClipId when the target is split so selection is remapped
from the removed region to a valid new ClipRegion id.
---
Outside diff comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1312-1316: The Tab key handler uses currentTime to find
overlapping annotations, but the timeline-aware playhead value playheadTime
should be used instead; in the block inside the e.key === 'Tab' check (where
annotationRegions is filtered/sorted) replace uses of currentTime/currentTimeMs
with playheadTime (compute const currentTimeMs = Math.round(playheadTime *
1000)) so cycling uses the timeline playhead consistent with add/seek/cursor
logic.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1218-1317: currentPersistedEditorState calls
buildPersistedEditorState with frame included in the state object but frame is
missing from the useMemo dependency array; add frame to the dependency array for
the useMemo that defines currentPersistedEditorState so updates to frame will
recompute the memoized value (look for the useMemo that declares
currentPersistedEditorState and ensure "frame" is present in its dependency
list).
---
Duplicate comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 5011-5016: The Button's title prop is hardcoded to English;
replace title={isPlaying ? "Pause" : "Play"} with the component's existing
localized playback labels so the tooltip follows language switching (e.g.
title={isPlaying ? playbackLabels.pause : playbackLabels.play} or
title={t(isPlaying ? 'pause' : 'play')} depending on the localization helper
used). Update the usage near the Button with
variant/size/togglePlayPause/isPlaying to reference those localized keys and
ensure any needed import or hook (playbackLabels or t) already present in the
component is used.
- Around line 1742-1751: The code currently arms
pendingFreshRecordingAutoZoomPathRef for any path returned by
window.electronAPI.getCurrentVideoPath(), which treats loaded videos like fresh
recordings; only set pendingFreshRecordingAutoZoomPathRef.current when the
returned result explicitly represents a fresh recording (e.g., check a flag such
as result.isFreshRecording or result.isTemporaryRecording) AND
autoApplyFreshRecordingAutoZooms is true. Update the getCurrentVideoPath
call/contract (or detect via a temp/fresh-recording path pattern) and change the
block around setVideoSourcePath/setVideoPath to conditionally set
pendingFreshRecordingAutoZoomPathRef.current only when that fresh-recording
indicator is present; reference symbols: window.electronAPI.getCurrentVideoPath,
pendingFreshRecordingAutoZoomPathRef, autoApplyFreshRecordingAutoZooms,
setVideoSourcePath, setVideoPath.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7664cdb7-082a-457d-a03d-cfe1f2c30557
📒 Files selected for processing (14)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/i18n/locales/en/editor.jsonsrc/i18n/locales/en/timeline.jsonsrc/i18n/locales/es/editor.jsonsrc/i18n/locales/es/timeline.jsonsrc/i18n/locales/ko/editor.jsonsrc/i18n/locales/ko/timeline.jsonsrc/i18n/locales/nl/editor.jsonsrc/i18n/locales/nl/timeline.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-CN/timeline.json
✅ Files skipped from review due to trivial changes (9)
- src/i18n/locales/en/timeline.json
- src/i18n/locales/es/timeline.json
- src/i18n/locales/ko/timeline.json
- src/i18n/locales/nl/timeline.json
- src/i18n/locales/zh-CN/timeline.json
- src/i18n/locales/en/editor.json
- src/i18n/locales/es/editor.json
- src/i18n/locales/zh-CN/editor.json
- src/components/video-editor/SettingsPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/video-editor/projectPersistence.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
src/components/video-editor/timeline/TimelineEditor.tsx (1)
122-126:⚠️ Potential issue | 🟠 MajorHonor
hideToolbarbefore rendering the built-in toolbar.
hideToolbaris part of the public props, but it is never destructured or checked before Line 1779.VideoEditorcan pass the flag and still get this toolbar rendered, which leaves the parent and child toolbars duplicated.Proposed fix
export interface TimelineEditorProps { @@ onVolumeChange?: (volume: number) => void; hideToolbar?: boolean; } @@ { @@ isCropped = false, videoPath, + hideToolbar = false, }, ref, ) { @@ - return ( - <div className="flex-1 min-h-0 flex flex-col bg-[`#17171a`] overflow-auto"> - <div className="flex items-center gap-2 px-4 py-2 border-b border-white/10 bg-[`#161619`]"> + return ( + <div className="flex-1 min-h-0 flex flex-col bg-[`#17171a`] overflow-auto"> + {!hideToolbar ? ( + <div className="flex items-center gap-2 px-4 py-2 border-b border-white/10 bg-[`#161619`]"> ... - </div> + </div> + ) : null} <divAlso applies to: 697-750, 1777-1779
🤖 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 122 - 126, The TimelineEditor component is not honoring the public prop hideToolbar so the built-in toolbar still renders even when VideoEditor passes hideToolbar; destructure hideToolbar from the component props (alongside isPlaying, onTogglePlayPause, volume, onVolumeChange) and guard the toolbar render paths by checking if (!hideToolbar) before rendering the built-in toolbar UI (the toolbar render block(s) used internally by TimelineEditor/its renderToolbar helper); apply this check to each toolbar rendering location referenced (the header/parent toolbar and the embedded child toolbar render sections) so the toolbar is skipped when hideToolbar is true.src/components/video-editor/VideoEditor.tsx (1)
5047-5048:⚠️ Potential issue | 🟡 MinorPlay/Pause button title bypasses localization.
The title attribute uses hardcoded English strings while surrounding buttons use
t()for localization.Proposed fix
-title={isPlaying ? "Pause" : "Play"} +title={isPlaying ? t("editor.playback.pause", "Pause") : t("editor.playback.play", "Play")}🤖 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 5047 - 5048, The Play/Pause button's title prop currently uses hardcoded English strings ("Pause"/"Play") which bypasses localization; update the title to use the i18n function (t) instead, e.g., call t('pause') / t('play') based on the isPlaying boolean. Locate the button rendering in VideoEditor.tsx where title={isPlaying ? "Pause" : "Play"} and replace the string literals with the appropriate t(...) keys (ensure the keys match existing translations or add them to the locale files) so the button title is localized.
🧹 Nitpick comments (1)
src/components/video-editor/VideoEditor.tsx (1)
658-663: Consider moving pure helper outside component.
formatTimeis a pure function with no dependencies on component state. Moving it outside the component avoids recreating the function reference on every render.+function formatTime(seconds: number) { + if (!isFinite(seconds) || isNaN(seconds) || seconds < 0) return "0:00"; + const mins = Math.floor(seconds / 60); + const secs = Math.floor(seconds % 60); + return `${mins}:${secs.toString().padStart(2, "0")}`; +} + export default function VideoEditor() { const { t } = useI18n(); - function formatTime(seconds: number) { - if (!isFinite(seconds) || isNaN(seconds) || seconds < 0) return "0:00"; - const mins = Math.floor(seconds / 60); - const secs = Math.floor(seconds % 60); - return `${mins}:${secs.toString().padStart(2, "0")}`; - }🤖 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 658 - 663, formatTime is a pure helper currently defined inside the component causing a new function to be created on every render; move the formatTime function to module/top-level scope (outside the VideoEditor component) so it’s declared once, keep the same signature formatTime(seconds: number), and export it if it’s used elsewhere; update usages in the component to call the top-level formatTime and remove any lingering closure references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/ExtensionManager.tsx`:
- Around line 97-107: The extension homepage link is rendered directly from
untrusted metadata; validate the URL scheme before rendering an anchor by
parsing extension.manifest.homepage with the URL constructor and only render a
clickable <a> when the parsed protocol is "http:" or "https:"; otherwise render
the homepage as plain text (or omit the link). Update the rendering logic in
ExtensionManager (the block that reads extension.manifest.homepage and the
similar blocks at the other occurrences) to perform this check and keep the
onClick={(e) => e.stopPropagation()} behavior only for safe links.
- Around line 709-713: The useEffect in ExtensionManager.tsx keeps re-triggering
handleSearch when activeTab === "browse" and marketplaceResults.length === 0
because marketplaceLoading flips to false; add a useRef (e.g.,
marketplaceAutoSearchedRef) to track whether the auto-search for the browse tab
has already run, set it true when calling handleSearch, and include it in the
effect check so handleSearch only runs once per tab entry (reset the ref when
leaving the browse tab or when a manual search is initiated); update the effect
condition to check !marketplaceAutoSearchedRef.current along with activeTab,
marketplaceResults.length === 0, and !marketplaceLoading, and set
marketplaceAutoSearchedRef.current = true immediately before invoking
handleSearch().
- Around line 721-726: When an install succeeds you update marketplace list but
not the modal state, so the detail modal still shows Install; after calling
setMarketplaceResults in the success branch (inside the install handler) also
update the open modal state by setting detailData.ext.installed to true (use the
existing state updater, e.g. setDetailData or the detail state setter used in
ExtensionManager) so the detail modal reflects the installed status; locate the
success block that checks result.success and add a call to update detailData.ext
(or replace detailData with a new object cloning ext and setting installed:
true) to keep the Install button in sync with the marketplaceResults update.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 2225-2231: sceneSectionContent currently renders
renderExtensionPanelsForSections("zoom", ...) causing zoom-related extension
panels to appear under Scene but disappear when the user opens the dedicated
Zoom section; update the switch handling for case "zoom" (and the
zoomItemSectionContent) to also include renderExtensionPanelsForSections("zoom",
"appearance", "zoom", "frame", "crop") (or move the call out of
sceneSectionContent into the Zoom branch) so any extension panels with
parentSection: "zoom" are rendered inside the Zoom section; apply the same
change to the other occurrences noted (around the other ranges mentioned) to
ensure zoom extension panels are consistently rendered when the Zoom page is
opened.
- Around line 204-220: The numeric readout currently falls back to 0 instead of
the slider's default; update the badge rendering to mirror the input's value
logic by showing field.defaultValue when value is not a number. Locate the
slider/value logic around value, field.defaultValue and the onChange that calls
extensionHost.setExtensionSetting(extensionId, field.id, ...) and change the
badge expression (the span that currently uses (typeof value === "number" ?
value : 0).toFixed(1)) to use (typeof value === "number" ? value :
(field.defaultValue as number)).toFixed(1) so the displayed number matches the
actual default used by the input.
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 595-600: The timeline no longer renders or edits
trimRegions/speedRegions because rows for them were removed and
handleItemSpanChange doesn't route their drag/resize updates; reintroduce
filters and row creation for trim and speed just like zoom/clip/annotation/audio
(use trimRegions, speedRegions and the corresponding TRIM_ROW_ID and
SPEED_ROW_ID identifiers) so Timeline builds rows for them, and update
handleItemSpanChange to handle items whose rowId equals TRIM_ROW_ID or
SPEED_ROW_ID (route span changes into the existing trimRegions/speedRegions
normalization and selection/deletion flows) ensuring their drag/resize events
update state and persist via the same normalization logic already used for other
rows.
- Around line 1481-1505: The Tab cycling uses currentTime instead of the
remapped playhead time so overlapping detection is wrong when timeline time is
remapped; in TimelineEditor replace the currentTime usage when computing
currentTimeMs with the mapped time (use playheadTime ?? currentTime) so the
overlapping filter on annotationRegions and subsequent selection logic
(selectedAnnotationId, onSelectAnnotation) operates at the correct time; locate
the block handling the "Tab" key and update the computation of currentTimeMs
accordingly.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 5205-5208: The TimelineEditor usage in VideoEditor is passing
unused props isPlaying, onTogglePlayPause, volume, and onVolumeChange; remove
these four props from the TimelineEditor JSX at the call site (the lines passing
isPlaying={isPlaying}, onTogglePlayPause={togglePlayPause},
volume={previewVolume}, onVolumeChange={setPreviewVolume}) since TimelineEditor
does not destructure or use them and VideoEditor handles playback when
hideToolbar={true}; update the JSX to only pass needed props and run a quick
compile to ensure no references remain to those prop variables in this call
site.
---
Duplicate comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 122-126: The TimelineEditor component is not honoring the public
prop hideToolbar so the built-in toolbar still renders even when VideoEditor
passes hideToolbar; destructure hideToolbar from the component props (alongside
isPlaying, onTogglePlayPause, volume, onVolumeChange) and guard the toolbar
render paths by checking if (!hideToolbar) before rendering the built-in toolbar
UI (the toolbar render block(s) used internally by TimelineEditor/its
renderToolbar helper); apply this check to each toolbar rendering location
referenced (the header/parent toolbar and the embedded child toolbar render
sections) so the toolbar is skipped when hideToolbar is true.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 5047-5048: The Play/Pause button's title prop currently uses
hardcoded English strings ("Pause"/"Play") which bypasses localization; update
the title to use the i18n function (t) instead, e.g., call t('pause') /
t('play') based on the isPlaying boolean. Locate the button rendering in
VideoEditor.tsx where title={isPlaying ? "Pause" : "Play"} and replace the
string literals with the appropriate t(...) keys (ensure the keys match existing
translations or add them to the locale files) so the button title is localized.
---
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 658-663: formatTime is a pure helper currently defined inside the
component causing a new function to be created on every render; move the
formatTime function to module/top-level scope (outside the VideoEditor
component) so it’s declared once, keep the same signature formatTime(seconds:
number), and export it if it’s used elsewhere; update usages in the component to
call the top-level formatTime and remove any lingering closure references.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 533845b9-b902-493d-94dc-ded8720253de
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
package.jsonsrc/components/video-editor/ExtensionManager.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/index.tssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/nl/launch.json
✅ Files skipped from review due to trivial changes (3)
- package.json
- src/i18n/locales/ko/launch.json
- src/i18n/locales/nl/launch.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/i18n/locales/ko/settings.json
- src/components/video-editor/types.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/SettingsPanel.tsx (1)
1364-1379:⚠️ Potential issue | 🟡 MinorLocalize the new video-background toasts.
These new
toast.*(...)messages are still hardcoded English, so this upload flow won't follow the locale selected from the new Settings page. Please route them throughtSettings(...)/t(...)and add matching locale keys.🤖 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 1364 - 1379, The toast messages in handleVideoUpload are hardcoded English; replace each toast.*(...) call in handleVideoUpload with localized strings using tSettings(...) or t(...) (e.g., toast.error(tSettings("video_upload.unsupported_format"), { description: tSettings("video_upload.select_video_description") }) and toast.success(tSettings("video_upload.added"))), ensuring you import/usage of tSettings where handleVideoUpload is defined and update the locale files with keys like video_upload.unsupported_format, video_upload.select_video_description, video_upload.added, and video_upload.import_failed to match the error/success cases used around isVideoWallpaperSource, setCustomImages, and onWallpaperChange.
♻️ Duplicate comments (1)
src/components/video-editor/VideoEditor.tsx (1)
1765-1775:⚠️ Potential issue | 🟠 MajorDon't arm fresh-recording auto-zoom from the generic video-load fallback.
This
getCurrentVideoPath()branch is not freshness-aware, so assigningpendingFreshRecordingAutoZoomPathRefhere can re-trigger auto-suggest for ordinary project/video loads once there is no active recording session. Keep itnullhere unless you have an explicit fresh-recording signal.🤖 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 1765 - 1775, The branch handling window.electronAPI.getCurrentVideoPath() is not freshness-aware so must not arm pendingFreshRecordingAutoZoomPathRef from this generic load; remove or change the assignment that sets pendingFreshRecordingAutoZoomPathRef.current = autoApplyFreshRecordingAutoZooms ? sourceVideoUrl : null and instead always set pendingFreshRecordingAutoZoomPathRef.current = null here (leave setting video via setVideoSourcePath/setVideoPath and clearing project/snapshot as-is), so only explicit fresh-recording code paths arm the ref; locate these symbols: getCurrentVideoPath, pendingFreshRecordingAutoZoomPathRef, autoApplyFreshRecordingAutoZooms, setVideoSourcePath, setVideoPath.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2426-2435: The handler handleSelectZoom should also clear any
selected annotation so zoom controls become accessible; inside the truthy-id
branch (where setSelectedZoomId(id) and setActiveEffectSection("zoom") are
called) add setSelectedAnnotationId(null) alongside setSelectedTrimId(null) and
setSelectedAudioId(null) so AnnotationSettingsPanel won't render for a lingering
selectedAnnotationId.
---
Outside diff comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1364-1379: The toast messages in handleVideoUpload are hardcoded
English; replace each toast.*(...) call in handleVideoUpload with localized
strings using tSettings(...) or t(...) (e.g.,
toast.error(tSettings("video_upload.unsupported_format"), { description:
tSettings("video_upload.select_video_description") }) and
toast.success(tSettings("video_upload.added"))), ensuring you import/usage of
tSettings where handleVideoUpload is defined and update the locale files with
keys like video_upload.unsupported_format,
video_upload.select_video_description, video_upload.added, and
video_upload.import_failed to match the error/success cases used around
isVideoWallpaperSource, setCustomImages, and onWallpaperChange.
---
Duplicate comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1765-1775: The branch handling
window.electronAPI.getCurrentVideoPath() is not freshness-aware so must not arm
pendingFreshRecordingAutoZoomPathRef from this generic load; remove or change
the assignment that sets pendingFreshRecordingAutoZoomPathRef.current =
autoApplyFreshRecordingAutoZooms ? sourceVideoUrl : null and instead always set
pendingFreshRecordingAutoZoomPathRef.current = null here (leave setting video
via setVideoSourcePath/setVideoPath and clearing project/snapshot as-is), so
only explicit fresh-recording code paths arm the ref; locate these symbols:
getCurrentVideoPath, pendingFreshRecordingAutoZoomPathRef,
autoApplyFreshRecordingAutoZooms, setVideoSourcePath, setVideoPath.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5eadbab2-add6-4aff-b1f9-b99dd0194ad6
📒 Files selected for processing (4)
src/components/video-editor/ExtensionManager.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/TimelineEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/video-editor/ExtensionManager.tsx
# Conflicts: # src/components/video-editor/ExtensionManager.tsx # src/components/video-editor/SettingsPanel.tsx # src/components/video-editor/TutorialHelp.tsx # src/components/video-editor/VideoEditor.tsx # src/components/video-editor/index.ts # src/components/video-editor/projectPersistence.ts # src/components/video-editor/timeline/TimelineEditor.tsx # src/components/video-editor/types.ts # src/i18n/locales/nl/launch.json
# Conflicts: # src/components/video-editor/timeline/TimelineEditor.tsx
This was referenced Apr 18, 2026
This was referenced May 1, 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
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.
Summary
New settings section in the editor sidebar, in-app language switching, extension install fixes, and expanded i18n.
Changes
settingssection inSettingsPanel— in-app language picker (all 5 locales), auto-apply fresh recording zooms toggle, connect-neighboring-zooms toggle, keybinds shortcutautoApplyFreshRecordingAutoZoomspreference through toSettingsPaneland zoom machineryautoApplyFreshRecordingAutoZoomsfield with tests__MACOSXsidecar dirs when unzipping extensions uploaded from macOS Finder (fixes manifest-not-found on Finder zips)window.electronAPIaccess for SSR safety; reformatsetLocale; persist locale preferencerenderHooks,extensionHost,fileUrls,cursorCoordinatesupdated for new extension capability surfaceSummary by CodeRabbit
New Features
Improvements