feat: ui overhaul — Phosphor icons, redesigned launcher, editor layout, and component refresh#246
Merged
webadderall merged 4 commits intomainfrom Apr 17, 2026
Merged
feat: ui overhaul — Phosphor icons, redesigned launcher, editor layout, and component refresh#246webadderall merged 4 commits intomainfrom
webadderall merged 4 commits intomainfrom
Conversation
Contributor
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Build & Scripts scripts/build-native-helpers.mjs, scripts/build-whisper-runtime.mjs, scripts/build-windows-capture.mjs, scripts/benchmark-export-queues.mjs, scripts/i18n-check.mjs, scripts/native-helper-manifest.mjs, scripts/postinstall.mjs |
macOS Swift target bumped to 14.0; helper binary names renamed (openscreen-* → recordly-*); CMake generator arch made dynamic; removed rmSync import while still used (runtime risk); calculateDelta now returns deltaPercent: null for non-finite; formatting and i18n deterministic ordering added. |
Config & Tooling .eslintrc.cjs, biome.json, postcss.config.cjs, tailwind.config.cjs, tsconfig.json, tsconfig.node.json, vite.config.ts, package.json |
Formatting/quoting standardization, Biome rule overrides and Tailwind Tailwind directives enabled, added compilerOptions.ignoreDeprecations, added dependency @phosphor-icons/react@^2.1.10, removed lucide-react from devDependencies. |
Docs / README README.md, README.zh-CN.md |
macOS minimum updated to 14.0+, ScreenCaptureKit note now “audio + microphone”, and “Extensions” section replaced with an “Extensions & Marketplace” description linking the Recordly Marketplace. |
Electron IPC & Types electron/electron-env.d.ts, electron/ipc/handlers.ts |
Added electronAPI.getAssetBasePath() and electronAPI.muxExportedVideoAudio(...) typings; native helper binary names updated to recordly-*; hardened uiohook typings and normalized some fs error handlers. |
Icon Migration (UI & Editor) src/components/... src/components/launch/LaunchWindow.tsx, src/components/video-editor/*, src/components/ui/*, src/components/video-editor/ExtensionIcon.tsx, src/components/video-editor/ExtensionManager.tsx |
Systematic replacement of lucide-react with @phosphor-icons/react, added icon alias resolution/fallback, updated usages and some icon props (e.g., weight="fill"). |
Video Editor Types & Timing src/components/video-editor/types.ts, src/components/video-editor/projectPersistence.ts, src/components/video-editor/VideoEditor.tsx, src/components/video-editor/ExportSettingsMenu.tsx |
Added optional fields (pressure, muted, trackIndex), exported getClipSourceEndMs and switched clipsToTrims to use it, default exportPipelineModel changed "legacy"→"modern", and VideoEditor computes effectiveSpeedRegions. |
Annotation & Overlay src/components/video-editor/AnnotationOverlay.tsx, src/components/video-editor/AnnotationSettingsPanel.tsx |
Large edits including icon migration, font handling, UI refinements, and a pointer-capture behavior change (overlay pointerEvents now unconditional "auto"). High-diff, behavior-impacting surface. |
Accessibility & Interaction src/components/launch/SourceSelector.tsx, src/components/ui/content-clamp.tsx, src/components/video-editor/SliderControl.tsx |
Added keyboard handlers, focus/ARIA attributes, popover keyboard support, and aria-* for range inputs. |
Styling & CSS src/index.css, src/App.css, src/components/launch/SourceSelector.module.css |
Normalized line endings/indentation; added global *:focus { outline: none !important } and tap-highlight suppression; extended range track styling; minor color literal normalization. |
Utilities, Tests & Minor Formatting src/lib/*, src/utils/*, src/components/video-editor/* (helpers/tests) |
Wide-ranging quoting/formatting normalization (tabs, semicolons, double quotes); added small utility getClipSourceEndMs; most behavior unchanged except noted files. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- webadderall/Recordly#122 — overlaps video-editor surface and types changes (types.ts, AnnotationOverlay, AnnotationSettingsPanel).
- webadderall/Recordly#163 — related LaunchWindow/webcam preview and stream attach logic changes.
- webadderall/Recordly#111 — touches native postinstall/build scripts and Windows CMake flow similar to this PR.
Suggested labels
Checked
Poem
🐰 I nibble through imports with speed and cheer,
Phosphor wings replace the lucide deer,
Tabs and quotes all gleam anew,
macOS fourteen hops into view,
Tiny fixes and big refactors — a joyful chew!
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 8.06% 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 and clearly summarizes the primary change: a comprehensive UI overhaul including icon replacement, redesigned launcher, editor updates, and component refresh. |
| Description check | ✅ Passed | The PR description provides a clear summary of changes organized by category, though the template sections (Motivation, Type of Change, Related Issue, Testing Guide, Screenshots/Video, Checklist) are not filled out. |
✏️ 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/ui-overhaul
…t, and component refresh - Replace all lucide-react imports with @phosphor-icons/react across every component (LaunchWindow, AnnotationSettingsPanel, ExtensionManager, ExtensionIcon, VideoPlayback, AnnotationOverlay, CropControl, TutorialHelp, PlaybackControls, all shadcn ui/ primitives, etc.) - ExtensionIcon: new Phosphor-based resolver with Lucide name alias map for backwards-compatible extension manifests - LaunchWindow / SourceSelector: full redesign — new icon rail layout, updated backgrounds, improved source selection UX, audio/video controls, countdown overlay - VideoPlayback: overhaul playback controls and preview panel layout - Timeline items: border-radius 16px→8px, cyan→blue unification, pill height 85% for breathing room, resize handle vertical centering - shadcn ui/: refresh accordion, dialog, dropdown, select, slider, switch, tabs, toggle, popover, button, card, input, label, sonner, content-clamp, audio-level-meter - App.tsx / index.css / App.css / tailwind.config: global style updates - types.ts, captionLayout.ts, captionStyle.ts, webcamOverlay.ts: data model updates to support new timeline and export features - package.json: add @phosphor-icons/react, bump dependencies - scripts/, .eslintrc, biome, tsconfig, vite.config: tooling alignment - extension-sources: update built-in extension bundles
webadderall
force-pushed
the
feat/ui-overhaul
branch
from
c9a6a66 to
9a5ff5c
Compare
![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: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
scripts/build-windows-capture.mjs (1)
19-20:⚠️ Potential issue | 🟠 MajorMatch CMake generator architecture to the bundle path.
The bundle directory switches between
win32-arm64andwin32-x64based onprocess.arch, but both CMake invocations (lines 123 and 132) hardcode-A x64. On a Windows ARM64 host, this builds an x64 binary and stages it under the ARM64 bundle directory, causing an architecture mismatch.🔧 Proposed fix
const bundledDir = path.join( projectRoot, "electron", "native", "bin", process.arch === "arm64" ? "win32-arm64" : "win32-x64", ); const bundledExePath = path.join(bundledDir, "wgc-capture.exe"); const helperId = "wgc-capture"; +const generatorArch = process.arch === "arm64" ? "ARM64" : "x64"; ... - execSync(`${cmake} .. -G "Visual Studio 17 2022" -A x64`, { + execSync(`${cmake} .. -G "Visual Studio 17 2022" -A ${generatorArch}`, { cwd: buildDir, stdio: "inherit", timeout: 120000, }); ... - execSync(`${cmake} .. -G "Visual Studio 16 2019" -A x64`, { + execSync(`${cmake} .. -G "Visual Studio 16 2019" -A ${generatorArch}`, { cwd: buildDir, stdio: "inherit", timeout: 120000, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-windows-capture.mjs` around lines 19 - 20, The script sets the bundle directory based on process.arch ("win32-arm64" vs "win32-x64") but the CMake invocations still hardcode '-A x64', causing ARM64 hosts to build x64 binaries; update the CMake generator argument to match the detected architecture (derive a cmakeArch variable from process.arch, e.g. ARM64 for arm64 else x64) and replace the hardcoded '-A x64' in the CMake invocation sites with the computed '-A ${cmakeArch}' so the generated binaries match the bundle path.src/components/video-editor/ExportSettingsMenu.tsx (1)
90-100:⚠️ Potential issue | 🟠 MajorExpose toggle state to assistive tech (
aria-pressed).On Line 90, Line 131, Line 195, Line 235, Line 286, Line 337, and Line 374 these buttons behave as toggles but don’t expose pressed state. Add
aria-pressed={isActive}(or move to proper tab/radio semantics) for accessibility parity.Example fix pattern (apply to each toggle button)
<button key={option.value} type="button" onClick={() => onExportFormatChange?.(option.value)} + aria-pressed={isActive} className={cn( "relative flex-1 overflow-hidden rounded-xl border py-2 text-xs font-medium transition-colors", isActive ? "border-[`#2563EB`]/50 text-white" : "border-white/10 bg-white/5 text-slate-400 hover:bg-white/10 hover:text-slate-200", )} >Also applies to: 131-136, 195-200, 235-240, 286-291, 337-342, 374-381
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/ExportSettingsMenu.tsx` around lines 90 - 100, The toggle-like buttons in ExportSettingsMenu (buttons that call onExportFormatChange or similar handlers and use the isActive boolean to set styling) do not expose their pressed state to assistive tech; update each such button element (the ones using key={option.value}, onClick={() => onExportFormatChange?.(option.value)} and other comparable toggle buttons) to include aria-pressed={isActive} (or replace with proper role="tab"/"radio" semantics if switching to a true tab/radio pattern) so assistive technologies can detect the toggle state.src/components/video-editor/types.ts (2)
302-308:⚠️ Potential issue | 🟠 MajorDon't drop
AudioRegion.trackIndexin project normalization.
src/components/video-editor/projectPersistence.ts:459-475reconstructsAudioRegionwithouttrackIndex, so any multi-track placement will be lost after reload.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/types.ts` around lines 302 - 308, The normalization in projectPersistence.ts is dropping AudioRegion.trackIndex when reconstructing regions, which loses multi-track placement on reload; update the reconstruction/normalization logic to copy the optional trackIndex from the source region into the rebuilt AudioRegion (or preserve it when present) so that AudioRegion.trackIndex is retained after normalization/reload.
131-137:⚠️ Potential issue | 🟠 MajorPersist
ClipRegion.mutedduring project load/save.
src/components/video-editor/projectPersistence.ts:343-362rebuildsClipRegionobjects without copyingmuted, so reopening a saved project will silently unmute clips. This type addition needs the matching persistence update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/types.ts` around lines 131 - 137, The ClipRegion.muted property is not being preserved when project files are reconstructed in projectPersistence (the code that rebuilds ClipRegion objects during project load/save); update the deserialization/reconstruction logic that creates ClipRegion instances to copy the muted value (e.g., when mapping saved regions into ClipRegion objects) so muted is carried through, and ensure the serialization logic also includes muted when saving projects.
🟡 Minor comments (8)
vite.config.ts-27-29 (1)
27-29:⚠️ Potential issue | 🟡 MinorFix typos in renderer guidance comments.
Line 27 has
Ployfill→Polyfill, and Line 28 reads awkwardly (If you want use Node.js). Please clean these to avoid confusion in operational docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.ts` around lines 27 - 29, Fix the typos and improve clarity in the renderer guidance comment: change "Ployfill the Electron and Node.js API for Renderer process." to "Polyfill the Electron and Node.js API for the renderer process." and change "If you want use Node.js in Renderer process, the `nodeIntegration` needs to be enabled in the Main process." to "If you want to use Node.js in the renderer process, enable `nodeIntegration` in the main process." Keep the existing reference to the plugin URL (https://github.com/electron-vite/vite-plugin-electron-renderer) unchanged; update the two comment lines in vite.config.ts where those exact phrases appear.scripts/benchmark-export-queues.mjs-465-466 (1)
465-466:⚠️ Potential issue | 🟡 MinorAvoid reporting
0%when baseline is0(undefined delta percent).
0%here is misleading; percent delta is not computable when the reference is zero. Returnnullso downstream formatting renders-, and reuse the same helper for summary logs to keep semantics consistent.Proposed fix
function calculateDelta(referenceValue, nextValue) { if ( typeof referenceValue !== "number" || !Number.isFinite(referenceValue) || typeof nextValue !== "number" || !Number.isFinite(nextValue) ) { return { deltaMs: null, deltaPercent: null }; } return { deltaMs: nextValue - referenceValue, - deltaPercent: - referenceValue > 0 ? ((nextValue - referenceValue) / referenceValue) * 100 : 0, + deltaPercent: + referenceValue > 0 ? ((nextValue - referenceValue) / referenceValue) * 100 : null, }; }const deltaMs = tuned.averageElapsedMs - baseline.averageElapsedMs; - const percent = - baseline.averageElapsedMs > 0 ? (deltaMs / baseline.averageElapsedMs) * 100 : 0; + const { deltaPercent: percent } = calculateDelta( + baseline.averageElapsedMs, + tuned.averageElapsedMs, + ); const medianDeltaMs = tuned.medianElapsedMs - baseline.medianElapsedMs; - const medianPercent = - baseline.medianElapsedMs > 0 ? (medianDeltaMs / baseline.medianElapsedMs) * 100 : 0; + const { deltaPercent: medianPercent } = calculateDelta( + baseline.medianElapsedMs, + tuned.medianElapsedMs, + ); const backendLabel = result.request.backend ?? "default"; console.log( - `[benchmark-export-queues] ${backendLabel} tuned vs baseline: ${deltaMs}ms (${percent.toFixed(1)}%)`, + `[benchmark-export-queues] ${backendLabel} tuned vs baseline: ${deltaMs}ms (${typeof percent === "number" ? percent.toFixed(1) : "-"}%)`, ); console.log( - `[benchmark-export-queues] ${backendLabel} tuned vs baseline (median): ${medianDeltaMs}ms (${medianPercent.toFixed(1)}%)`, + `[benchmark-export-queues] ${backendLabel} tuned vs baseline (median): ${medianDeltaMs}ms (${typeof medianPercent === "number" ? medianPercent.toFixed(1) : "-"}%)`, );Also applies to: 955-960
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/benchmark-export-queues.mjs` around lines 465 - 466, Replace the inline ternary computing deltaPercent so it returns null when referenceValue is 0 instead of 0; extract the logic into a small helper (e.g., computeDeltaPercent(referenceValue, nextValue)) that returns ((nextValue - referenceValue)/referenceValue)*100 when referenceValue > 0 and null otherwise, then use that helper for the deltaPercent field and also reuse it where summary logs are created (the same logic referenced around lines 955-960) so semantics are consistent between per-item output and summary logging.src/components/video-editor/CropControl.tsx-67-69 (1)
67-69:⚠️ Potential issue | 🟡 MinorGuard against zero-sized container before normalizing pointer coordinates.
At Line 67-69 and Line 79-80, dividing by
rect.width/rect.heightcan yieldNaN/Infinitywhen layout is not ready (or hidden), causing invalid crop updates.Suggested fix
const handlePointerDown = (e: React.PointerEvent, handle: DragHandle) => { e.stopPropagation(); e.preventDefault(); setIsDragging(handle); const rect = getContainerRect(); + if (rect.width <= 0 || rect.height <= 0) return; setDragStart({ x: (e.clientX - rect.left) / rect.width, y: (e.clientY - rect.top) / rect.height, }); setInitialCrop(cropRegion); e.currentTarget.setPointerCapture(e.pointerId); }; const handlePointerMove = (e: React.PointerEvent) => { if (!isDragging) return; const rect = getContainerRect(); + if (rect.width <= 0 || rect.height <= 0) return; const currentX = (e.clientX - rect.left) / rect.width; const currentY = (e.clientY - rect.top) / rect.height;Also applies to: 79-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/CropControl.tsx` around lines 67 - 69, The pointer-to-normalized-coordinate logic in CropControl.tsx can divide by zero when the container rect is not ready; update the handlers that compute x/y (the pointer normalization code used in the pointer event handlers—e.g., the functions handling pointer down/move that call (e.clientX - rect.left) / rect.width and (e.clientY - rect.top) / rect.height) to first read rect = containerRef.current.getBoundingClientRect() and guard that rect.width and rect.height are > 0; if either is zero, return early (no crop update) or clamp coordinates safely to [0,1] to avoid NaN/Infinity, and apply the same guard in both places where normalization occurs (the two spots referenced in the diff).src/index.css-82-84 (1)
82-84:⚠️ Potential issue | 🟡 MinorFix lint rule violation at Line 83 (
declaration-empty-line-before).Add the required empty line between
@applyand the next declaration to satisfy current Stylelint config.Suggested fix
body { `@apply` bg-background text-foreground; + -webkit-user-select: none; user-select: none; font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.css` around lines 82 - 84, Insert a single blank line between the `@apply` rule and the following user-select declarations to satisfy the Stylelint declaration-empty-line-before rule; specifically, add an empty line after "@apply bg-background text-foreground;" so that the subsequent "-webkit-user-select: none;" and "user-select: none;" are separated by one blank line from the `@apply` statement.README.zh-CN.md-64-68 (1)
64-68:⚠️ Potential issue | 🟡 MinorRemove the stale extension section later in this file.
This new marketplace section now overlaps with the older
## 扩展系统section on Lines 392-396, which still sends readers topublic/builtin-extensions/andEXTENSIONS.md. Please update or remove that older section so the Chinese README has one consistent extension story.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.zh-CN.md` around lines 64 - 68, The README contains a stale older section titled "## 扩展系统" that still points to public/builtin-extensions/ and EXTENSIONS.md; remove or update that section so the Chinese README has a single consistent extension story that matches the new "## 扩展与市场" marketplace copy—either delete the entire "## 扩展系统" block (the heading plus the paragraph that references public/builtin-extensions/ and EXTENSIONS.md) or rewrite it to reference the marketplace link (https://marketplace.recordly.dev/extensions) and the new guidance in the "## 扩展与市场" section.src/components/launch/SourceSelector.tsx-160-163 (1)
160-163:⚠️ Potential issue | 🟡 MinorLocalize the empty-state strings.
"No screens available"and"No windows available"bypassuseScopedT, so this screen will partly fall back to English even when the rest of the selector is translated.Also applies to: 202-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/SourceSelector.tsx` around lines 160 - 163, The empty-state hardcoded strings in SourceSelector.tsx are bypassing localization; use the useScopedT hook (e.g., const t = useScopedT()) to replace "No screens available" and "No windows available" with localized keys (for example t('noScreens') and t('noWindows')), updating the render branches that check screenSources and windowSources and adding those keys to the component's scope translations so the selector fully localizes.package.json-40-40 (1)
40-40:⚠️ Potential issue | 🟡 MinorComplete migration from
lucide-reactto@phosphor-icons/reactbefore removing the old dependency.
lucide-reactis still actively imported across the codebase in:
src/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxMigrate all icon imports to
@phosphor-icons/reactin these files, then removelucide-reactfrompackage.jsonto avoid maintaining duplicate icon libraries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 40, Replace all remaining imports from "lucide-react" with matching components from "@phosphor-icons/react" in the listed files (src/components/video-editor/timeline/Item.tsx, TimelineEditor.tsx, SettingsPanel.tsx, VideoEditor.tsx): locate import lines that reference lucide-react and swap to the equivalent Phosphor icon component names, adjust props (e.g., size/weight/color) to Phosphor's prop names, update any renamed icon identifiers in the JSX (ensure default/ named import differences are handled), run a quick compile to fix any symbol name mismatches, and only then remove "lucide-react" from package.json and uninstall the package.src/components/video-editor/ShortcutsConfigDialog.tsx-50-89 (1)
50-89:⚠️ Potential issue | 🟡 MinorStale closure:
draftmissing from dependency array.The
handleCapturecallback usesdraft(line 71) to check for conflicts viafindConflict(binding, captureFor, draft), butdraftis not included in the dependency array. This can cause the conflict check to use a stale version of the shortcuts configuration.Scenario: User changes binding A, then immediately clicks to capture binding B. The conflict check will use the old
draftstate, potentially missing or incorrectly detecting conflicts.🐛 Proposed fix
}, [captureFor, t]); + }, [captureFor, draft, t]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/ShortcutsConfigDialog.tsx` around lines 50 - 89, The effect's keydown handler (handleCapture) reads the current shortcuts state via draft when calling findConflict(binding, captureFor, draft) but draft is not listed in the useEffect dependency array, causing a stale-closure bug; update the useEffect dependencies to include draft (so useEffect(..., [captureFor, draft, t])) or otherwise ensure draft's latest value is used (e.g., use a ref), so handleCapture, its conflict check via findConflict, and state updates (setDraft, setConflict, setCaptureFor) always operate on the current draft.
🧹 Nitpick comments (10)
scripts/i18n-check.mjs (2)
7-10: Prefer deterministic directory/file ordering for stable CI output.
fs.readdirSyncordering can vary by filesystem. SortinglocalesandnamespaceFileskeeps logs stable and easier to diff across environments.Suggested diff
-const locales = fs.readdirSync(localesDir).filter((entry) => { +const locales = fs + .readdirSync(localesDir) + .filter((entry) => { const fullPath = path.join(localesDir, entry); return fs.statSync(fullPath).isDirectory(); -}); + }) + .sort(); const baseLocaleDir = path.join(localesDir, "en"); -const namespaceFiles = fs.readdirSync(baseLocaleDir).filter((file) => file.endsWith(".json")); +const namespaceFiles = fs + .readdirSync(baseLocaleDir) + .filter((file) => file.endsWith(".json")) + .sort();Also applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/i18n-check.mjs` around lines 7 - 10, The directory listings are not deterministic because fs.readdirSync can return unsorted entries; update the logic that builds locales (using localesDir and locales) to sort the array after filtering (e.g., call sort() on locales) and likewise sort namespaceFiles after reading them so both locales and namespaceFiles are in a stable, deterministic order for CI and diffs; ensure you apply the same sort step where namespaceFiles is created (around the code referenced at lines 45-46).
17-19: Add file-path context when JSON parsing fails.A malformed locale file currently throws without explicit file attribution from this helper. Wrapping parse/read errors with the path will speed up triage.
Suggested diff
function loadJson(filePath) { - return JSON.parse(fs.readFileSync(filePath, "utf8")); + try { + return JSON.parse(fs.readFileSync(filePath, "utf8")); + } catch (error) { + throw new Error(`i18n-check: failed to parse ${filePath}: ${error.message}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/i18n-check.mjs` around lines 17 - 19, The helper loadJson should wrap fs.readFileSync and JSON.parse in a try/catch so any read/parse failure includes the file path; update the loadJson function to catch errors from fs.readFileSync(filePath, "utf8") and JSON.parse(...) and throw a new Error that contains the filePath (and original error message or set the original error as the cause) so logs and stack traces clearly show which locale file failed to parse.src/utils/platformUtils.ts (1)
17-17: Consider replacing deprecatednavigator.platformin future work.The
navigator.platformAPI is deprecated. While this is pre-existing code and functions correctly as a fallback, consider migrating to a more modern approach in a future PR. Options include using a user-agent parsing library or relying solely on the Electron API for platform detection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/platformUtils.ts` at line 17, The code uses the deprecated navigator.platform check (the expression using navigator.platform with /Mac|iPhone|iPad|iPod/); replace it with a modern, non-deprecated strategy: prefer navigator.userAgentData.platform or parse navigator.userAgent as a fallback in browsers, and when running under Electron use process.platform (or Electron API) for reliable OS detection; wrap this logic in a small helper (e.g., isMacPlatform or detectPlatform) that returns the boolean previously computed and replace the direct navigator.platform usage with that helper.extension-sources/webadderall.more-wallpapers/index.js (1)
124-126: Optional: Remove explicitreturn undefined;The
deactivate()method signature inRecordlyExtensionModulespecifiesvoid | Promise<void>, and the extension loader atsrc/lib/extensions/extensionHost.ts:252does not capture or use the return value. The explicitreturn undefined;is unnecessary—functions that returnvoidimplicitly returnundefined.♻️ Simplify to implicit return
export function deactivate() { - return undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension-sources/webadderall.more-wallpapers/index.js` around lines 124 - 126, The deactivate function currently returns an explicit undefined; remove the explicit "return undefined;" so the function has an implicit void return (e.g., keep export function deactivate() { } ), updating the function named deactivate to return void implicitly rather than returning undefined.extension-sources/insomniachooman.cool-cursors/index.js (1)
45-47: Optional: Remove explicitreturn undefined;As with
extension-sources/webadderall.more-wallpapers/index.js:124-126, the explicitreturn undefined;is unnecessary. Thedeactivate()method signature specifiesvoid | Promise<void>, and the extension loader does not use the return value. Functions returningvoidimplicitly returnundefined.♻️ Simplify to implicit return
export function deactivate() { - return undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extension-sources/insomniachooman.cool-cursors/index.js` around lines 45 - 47, The deactivate function currently returns undefined explicitly; remove the explicit "return undefined;" from the export function deactivate() implementation (function name: deactivate) so it uses the implicit void/undefined return instead—simply leave the function body empty or omit the return statement to simplify the code.biome.json (2)
58-58: Avoid redundant rule disabling foruseComponentExportOnlyModules.At Line 58 this rule is already globally
"off", so the added override in Lines 162-170 is redundant noise.Suggested cleanup
{ "includes": ["src/components/ui/**/*.tsx"], "linter": { "rules": { "style": { - "useComponentExportOnlyModules": "off" + "useComponentExportOnlyModules": "off" } } } },(Or remove this entire override block, since global config already sets it to
"off".)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@biome.json` at line 58, The override block that sets "useComponentExportOnlyModules" to "off" is redundant because the global setting already disables it; remove the redundant rule entry (or remove the whole override block) referring to the override that contains "useComponentExportOnlyModules" so the config is cleaner and avoids duplicate settings.
172-182: Scope test rule relaxations more narrowly.Disabling
noUnusedVariablesandnoExplicitAnyfor all**/*.test.ts(x)is unnecessarily broad. Of 18 test files, only 4 actually use type assertions (as any):
src/components/video-editor/audio.test.ts(15 instances)src/lib/exporter/frameRenderer.test.ts(13 instances)src/lib/exporter/localMediaSource.test.ts(3 instances)src/lib/exporter/modernFrameRenderer.test.ts(1 instance)The other 14 test files don't need this suppression, allowing real regressions to be caught. Narrow the override to only the specific files or directories that require it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@biome.json` around lines 172 - 182, The linter overrides in the "linter.rules" block currently disable "noUnusedVariables" and "noExplicitAny" for all test files via the includes pattern "**/*.test.ts" and "**/*.test.tsx"; change this to scope the relaxations only to the specific test files that actually use "as any" (e.g., "src/components/video-editor/audio.test.ts", "src/lib/exporter/frameRenderer.test.ts", "src/lib/exporter/localMediaSource.test.ts", "src/lib/exporter/modernFrameRenderer.test.ts") by replacing the broad include patterns with an explicit list or a narrower glob for those paths and keep the default rules for the remaining tests so "noUnusedVariables" and "noExplicitAny" remain enabled elsewhere.tsconfig.json (1)
8-8: Time-box or document the deprecation suppression in TypeScript config.At Line 8,
"ignoreDeprecations": "5.0"suppresses a broad class of compiler warnings globally. Since the project uses TypeScript^5.2.2(not strictly pinned), future updates could inherit this suppression indefinitely, delaying necessary migrations. Add a comment explaining the scope and timeline for removal, or consider using a more targeted suppression approach if specific deprecations must be ignored during a migration period.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` at line 8, The tsconfig.json entry "ignoreDeprecations": "5.0" is a global suppression that should be time-boxed or documented; update tsconfig.json to either replace this broad suppression with a targeted approach (e.g., remove the global "ignoreDeprecations" and apply scoped suppression or compiler flags only for the affected files) or add a clear comment next to the "ignoreDeprecations": "5.0" key stating why it exists, what deprecations it covers, and the planned removal date/issue number (and create a tracking issue/PR if none exists) so future TypeScript upgrades won’t silently inherit the suppression.src/components/video-editor/AddCustomFontDialog.tsx (1)
111-112: Consider resetting form state when the dialog closes.On Line 111 and Line 162, closing the dialog keeps previous URL/name values. Clearing fields on close makes repeated opens less error-prone.
Optional cleanup on close
-<Dialog open={open} onOpenChange={setOpen}> +<Dialog + open={open} + onOpenChange={(next) => { + setOpen(next); + if (!next) { + setImportUrl(""); + setFontName(""); + } + }} +>Also applies to: 162-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/AddCustomFontDialog.tsx` around lines 111 - 112, The dialog keeps previous form values after closing; update AddCustomFontDialog to reset the name/url form state when the Dialog closes by detecting the open->false transition (either in the Dialog onOpenChange handler that calls setOpen or via a useEffect watching open) and call the form state setters (e.g., setFontName and setFontUrl or the existing name/url state setters) to clear values; ensure this runs both when the user closes the dialog via DialogTrigger or programmatic setOpen(false).src/components/launch/UpdateToastWindow.tsx (1)
102-115: Remove unnecessaryvoidstatements.The
void payload.phase,void payload.version, andvoid payload.progressPercentexpressions are no-ops that don't affect behavior. Since the dependency array is[payload], the effect already re-runs when the payload object changes. These void statements don't make the effect "more reactive" to those specific properties.If the intent was to document which properties trigger the reset, consider using a comment instead.
♻️ Suggested simplification
useEffect(() => { - if (payload) { - void payload.phase; - void payload.version; - void payload.progressPercent; - } - setDragOffsetX(0); dragState.current = { pointerId: null, startX: 0, active: false, }; }, [payload]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/UpdateToastWindow.tsx` around lines 102 - 115, The effect contains no-op expressions `void payload.phase`, `void payload.version`, and `void payload.progressPercent` that should be removed; update the useEffect in UpdateToastWindow so it only resets state (call setDragOffsetX(0) and set dragState.current) and, if you need to document why these properties matter, add a short comment mentioning payload.phase/version/progressPercent rather than using `void` expressions; keep the dependency array as [payload].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@biome.json`:
- Line 47: Restore the useExhaustiveDependencies ESLint rule in biome.json from
"off" to at least "warn": locate the "useExhaustiveDependencies" entry
(currently "off") and change its value to "warn" so React hooks
(useEffect/useMemo/useCallback) are linted as warnings across the codebase,
enabling incremental fixes while preventing stale-closure bugs.
In `@scripts/build-native-helpers.mjs`:
- Around line 29-41: The macOS helper rename in scripts/build-native-helpers.mjs
(outputs like "recordly-screencapturekit-helper", "recordly-window-list",
"recordly-system-cursors", "recordly-native-cursor-monitor") no longer matches
the helper names expected by ensureSwiftHelperBinary calls in
electron/ipc/handlers.ts; update the calls to ensureSwiftHelperBinary (and any
provenance/name checks) to use the new "recordly-*" names, or revert the output
names in build-native-helpers.mjs to the legacy "openscreen-*" names so both
build and runtime agree (search for ensureSwiftHelperBinary in handlers.ts and
the output mappings in build-native-helpers.mjs to apply the change).
In `@src/components/launch/SourceSelector.tsx`:
- Around line 166-170: The source cards are only clickable and not
keyboard-focusable; update the Card rendering in SourceSelector (the Card
elements using key={source.id}, className with styles.sourceCard and
selectedSource check, and onClick={() => handleSourceSelect(source)}) to be
keyboard-accessible by rendering them as real buttons or by adding
role="button", tabIndex={0}, and an onKeyDown handler that triggers
handleSourceSelect(source) when Enter or Space is pressed; ensure the existing
onClick remains, preserve selected styling (selectedSource?.id === source.id),
and add visible focus styles (or use the existing styles.selected) so keyboard
users can both focus and activate sources; apply the same changes to the other
Card instances referenced around the 208-212 region.
In `@src/components/ui/content-clamp.tsx`:
- Around line 55-71: The current PopoverTrigger span using className and
truncatedText only handles hover via handleMouseEnter/handleMouseLeave and is
not focusable or keyboard-accessible; make the trigger reachable and operable by
adding onFocus and onBlur that call handleMouseEnter/handleMouseLeave (or
unified open/close handlers), add an onKeyDown handler to open the popover on
Enter/Space, and ensure the element is focusable (replace the span with a
semantic <button> or add tabIndex={0} and role="button"), and expose ARIA state
such as aria-haspopup and aria-expanded tied to the popover open state so
keyboard and assistive tech users can open/read the clamped content (update
PopoverTrigger and related handlers accordingly and keep PopoverContent’s
onMouseEnter/onMouseLeave behavior).
In `@src/components/video-editor/AnnotationOverlay.tsx`:
- Around line 167-179: The overlay sets style.pointerEvents to "none" when
isSelected is false, which prevents the onClick handler in AnnotationOverlay.tsx
from ever firing and blocks selecting unselected annotations; remove or change
that pointerEvents logic so clicks still register (e.g., always allow
pointerEvents or conditionally only disable specific child interactions) and
rely on disableDragging={!isSelected} to prevent edits; update the style block
that references isSelected and pointerEvents and ensure onClick(annotation.id)
can be invoked for unselected items.
In `@src/components/video-editor/AnnotationSettingsPanel.tsx`:
- Around line 550-565: The arrow-direction buttons in AnnotationSettingsPanel
are icon-only and need accessible names: add an aria-label (e.g.,
`aria-label={`Arrow ${direction}`}` or more descriptive like `aria-label={`Set
arrow direction to ${direction}`}`) to the button rendered in the loop where
`direction` is used; locate the button that constructs `newFigureData:
FigureData` and calls `onFigureDataChange`, and add the aria-label there so each
icon-only button conveys its direction to assistive tech.
In `@src/components/video-editor/CropControl.tsx`:
- Around line 38-48: The draw loop in CropControl's useEffect schedules a new
requestAnimationFrame inside draw but the cleanup only cancels the initial
rafId, so the loop can continue after unmount; fix by tracking the most recent
RAF id (e.g., use a mutable ref/latestRafId variable) and update it each time
requestAnimationFrame(draw) is called, then cancel that latest id in the effect
cleanup (optionally set a mounted/cancelled flag checked by draw to avoid
queuing new frames after unmount); update references to draw,
rafId/requestAnimationFrame, and cancelAnimationFrame accordingly.
In `@src/components/video-editor/ExtensionManager.tsx`:
- Around line 655-664: handleToggleExtension catches toggleExtension failures
and calls toast.error(t("toast.enableFailed")) but the translation key
toast.enableFailed is missing; update the i18n or call site: either add
"toast.enableFailed": "Failed to enable extension" to
src/i18n/locales/en/extensions.json under the toast object, or change the toast
call in handleToggleExtension to use an existing key (e.g.,
"toast.uninstallFailed" or another appropriate key) or supply a fallback like
t("toast.enableFailed", { defaultValue: "Failed to enable extension" }) so users
see a proper message; reference symbols: handleToggleExtension, toggleExtension,
toast.error, and t.
In `@src/components/video-editor/SliderControl.tsx`:
- Around line 55-63: The range input in SliderControl.tsx (inside the
SliderControl component) is missing an accessible name; update the <input
type="range"> to include an aria-label or aria-labelledby attribute so screen
readers can identify the control (e.g., add an ariaLabel prop to SliderControl
and wire it to the input as aria-label={ariaLabel}, or accept/resolve an id for
a visible label and use aria-labelledby); ensure the prop is optional with a
sensible default and keep the existing onChange, value, min, max, and step
behavior intact.
In `@src/index.css`:
- Around line 7-10: The global rule `*:focus { outline: none !important; }`
removes keyboard focus indicators site-wide; change focus handling so only
non-keyboard focus loses outline and provide a visible fallback: replace the
blanket `*:focus` rule with a rule targeting `*:focus:not(:focus-visible)` (or
`:focus:not(.focus-visible)` polyfill) to suppress outlines for mouse/touch, and
add a `*:focus-visible` rule that restores an accessible visible focus style
(e.g. clear outline or box-shadow with sufficient contrast) so keyboard users
retain clear focus cues; update selectors referenced (`*:focus`,
`:focus-visible`) accordingly.
In `@src/lib/assetPath.ts`:
- Around line 35-40: The current catch silently falls back to a web-style URL
(`/${encodedRelativePath}`) even when Electron's asset resolution via
window.electronAPI.getAssetBasePath() fails, which hides real errors in file:
contexts; update the catch in assetPath (where getAssetBasePath is called and
encodedRelativePath is used) to only return the web fallback when the page is
served over http(s) (e.g., check window.location.protocol startsWith('http')),
and otherwise either rethrow the caught error or preserve/return the original
Electron-style path behavior so the failure surfaces instead of returning a
browser URL that won't resolve.
In `@src/lib/customFonts.ts`:
- Around line 179-183: The isValidGoogleFontsUrl function currently only checks
hostname and family param; tighten validation by also ensuring the URL uses the
HTTPS protocol (urlObj.protocol === "https:") and optionally restrict the
pathname to the newer CSS2 endpoint (e.g., urlObj.pathname startsWith("/css2")
or matches known allowed paths like "/css") before returning true; update
isValidGoogleFontsUrl to perform these additional checks so insecure or
non-standard Google Fonts URLs are rejected.
- Around line 37-43: The addCustomFont function currently returns the existing
fonts array when a duplicate is detected, making duplicates indistinguishable
from a successful add; update addCustomFont to make duplicate handling explicit
by either (A) returning a discriminated result object (e.g., { fonts:
CustomFont[], added: boolean } or { success: true/false, fonts }) and set
added=false when a font with matching id or fontFamily is found, or (B) throw a
typed DuplicateFontError (create a DuplicateFontError class) when exists is
true; update callers of addCustomFont to handle the new result/exception
accordingly. Ensure the change is applied inside addCustomFont (where exists is
computed) and preserve the existing behavior of returning the fonts list on
success.
---
Outside diff comments:
In `@scripts/build-windows-capture.mjs`:
- Around line 19-20: The script sets the bundle directory based on process.arch
("win32-arm64" vs "win32-x64") but the CMake invocations still hardcode '-A
x64', causing ARM64 hosts to build x64 binaries; update the CMake generator
argument to match the detected architecture (derive a cmakeArch variable from
process.arch, e.g. ARM64 for arm64 else x64) and replace the hardcoded '-A x64'
in the CMake invocation sites with the computed '-A ${cmakeArch}' so the
generated binaries match the bundle path.
In `@src/components/video-editor/ExportSettingsMenu.tsx`:
- Around line 90-100: The toggle-like buttons in ExportSettingsMenu (buttons
that call onExportFormatChange or similar handlers and use the isActive boolean
to set styling) do not expose their pressed state to assistive tech; update each
such button element (the ones using key={option.value}, onClick={() =>
onExportFormatChange?.(option.value)} and other comparable toggle buttons) to
include aria-pressed={isActive} (or replace with proper role="tab"/"radio"
semantics if switching to a true tab/radio pattern) so assistive technologies
can detect the toggle state.
In `@src/components/video-editor/types.ts`:
- Around line 302-308: The normalization in projectPersistence.ts is dropping
AudioRegion.trackIndex when reconstructing regions, which loses multi-track
placement on reload; update the reconstruction/normalization logic to copy the
optional trackIndex from the source region into the rebuilt AudioRegion (or
preserve it when present) so that AudioRegion.trackIndex is retained after
normalization/reload.
- Around line 131-137: The ClipRegion.muted property is not being preserved when
project files are reconstructed in projectPersistence (the code that rebuilds
ClipRegion objects during project load/save); update the
deserialization/reconstruction logic that creates ClipRegion instances to copy
the muted value (e.g., when mapping saved regions into ClipRegion objects) so
muted is carried through, and ensure the serialization logic also includes muted
when saving projects.
---
Minor comments:
In `@package.json`:
- Line 40: Replace all remaining imports from "lucide-react" with matching
components from "@phosphor-icons/react" in the listed files
(src/components/video-editor/timeline/Item.tsx, TimelineEditor.tsx,
SettingsPanel.tsx, VideoEditor.tsx): locate import lines that reference
lucide-react and swap to the equivalent Phosphor icon component names, adjust
props (e.g., size/weight/color) to Phosphor's prop names, update any renamed
icon identifiers in the JSX (ensure default/ named import differences are
handled), run a quick compile to fix any symbol name mismatches, and only then
remove "lucide-react" from package.json and uninstall the package.
In `@README.zh-CN.md`:
- Around line 64-68: The README contains a stale older section titled "## 扩展系统"
that still points to public/builtin-extensions/ and EXTENSIONS.md; remove or
update that section so the Chinese README has a single consistent extension
story that matches the new "## 扩展与市场" marketplace copy—either delete the entire
"## 扩展系统" block (the heading plus the paragraph that references
public/builtin-extensions/ and EXTENSIONS.md) or rewrite it to reference the
marketplace link (https://marketplace.recordly.dev/extensions) and the new
guidance in the "## 扩展与市场" section.
In `@scripts/benchmark-export-queues.mjs`:
- Around line 465-466: Replace the inline ternary computing deltaPercent so it
returns null when referenceValue is 0 instead of 0; extract the logic into a
small helper (e.g., computeDeltaPercent(referenceValue, nextValue)) that returns
((nextValue - referenceValue)/referenceValue)*100 when referenceValue > 0 and
null otherwise, then use that helper for the deltaPercent field and also reuse
it where summary logs are created (the same logic referenced around lines
955-960) so semantics are consistent between per-item output and summary
logging.
In `@src/components/launch/SourceSelector.tsx`:
- Around line 160-163: The empty-state hardcoded strings in SourceSelector.tsx
are bypassing localization; use the useScopedT hook (e.g., const t =
useScopedT()) to replace "No screens available" and "No windows available" with
localized keys (for example t('noScreens') and t('noWindows')), updating the
render branches that check screenSources and windowSources and adding those keys
to the component's scope translations so the selector fully localizes.
In `@src/components/video-editor/CropControl.tsx`:
- Around line 67-69: The pointer-to-normalized-coordinate logic in
CropControl.tsx can divide by zero when the container rect is not ready; update
the handlers that compute x/y (the pointer normalization code used in the
pointer event handlers—e.g., the functions handling pointer down/move that call
(e.clientX - rect.left) / rect.width and (e.clientY - rect.top) / rect.height)
to first read rect = containerRef.current.getBoundingClientRect() and guard that
rect.width and rect.height are > 0; if either is zero, return early (no crop
update) or clamp coordinates safely to [0,1] to avoid NaN/Infinity, and apply
the same guard in both places where normalization occurs (the two spots
referenced in the diff).
In `@src/components/video-editor/ShortcutsConfigDialog.tsx`:
- Around line 50-89: The effect's keydown handler (handleCapture) reads the
current shortcuts state via draft when calling findConflict(binding, captureFor,
draft) but draft is not listed in the useEffect dependency array, causing a
stale-closure bug; update the useEffect dependencies to include draft (so
useEffect(..., [captureFor, draft, t])) or otherwise ensure draft's latest value
is used (e.g., use a ref), so handleCapture, its conflict check via
findConflict, and state updates (setDraft, setConflict, setCaptureFor) always
operate on the current draft.
In `@src/index.css`:
- Around line 82-84: Insert a single blank line between the `@apply` rule and the
following user-select declarations to satisfy the Stylelint
declaration-empty-line-before rule; specifically, add an empty line after
"@apply bg-background text-foreground;" so that the subsequent
"-webkit-user-select: none;" and "user-select: none;" are separated by one blank
line from the `@apply` statement.
In `@vite.config.ts`:
- Around line 27-29: Fix the typos and improve clarity in the renderer guidance
comment: change "Ployfill the Electron and Node.js API for Renderer process." to
"Polyfill the Electron and Node.js API for the renderer process." and change "If
you want use Node.js in Renderer process, the `nodeIntegration` needs to be
enabled in the Main process." to "If you want to use Node.js in the renderer
process, enable `nodeIntegration` in the main process." Keep the existing
reference to the plugin URL
(https://github.com/electron-vite/vite-plugin-electron-renderer) unchanged;
update the two comment lines in vite.config.ts where those exact phrases appear.
---
Nitpick comments:
In `@biome.json`:
- Line 58: The override block that sets "useComponentExportOnlyModules" to "off"
is redundant because the global setting already disables it; remove the
redundant rule entry (or remove the whole override block) referring to the
override that contains "useComponentExportOnlyModules" so the config is cleaner
and avoids duplicate settings.
- Around line 172-182: The linter overrides in the "linter.rules" block
currently disable "noUnusedVariables" and "noExplicitAny" for all test files via
the includes pattern "**/*.test.ts" and "**/*.test.tsx"; change this to scope
the relaxations only to the specific test files that actually use "as any"
(e.g., "src/components/video-editor/audio.test.ts",
"src/lib/exporter/frameRenderer.test.ts",
"src/lib/exporter/localMediaSource.test.ts",
"src/lib/exporter/modernFrameRenderer.test.ts") by replacing the broad include
patterns with an explicit list or a narrower glob for those paths and keep the
default rules for the remaining tests so "noUnusedVariables" and "noExplicitAny"
remain enabled elsewhere.
In `@extension-sources/insomniachooman.cool-cursors/index.js`:
- Around line 45-47: The deactivate function currently returns undefined
explicitly; remove the explicit "return undefined;" from the export function
deactivate() implementation (function name: deactivate) so it uses the implicit
void/undefined return instead—simply leave the function body empty or omit the
return statement to simplify the code.
In `@extension-sources/webadderall.more-wallpapers/index.js`:
- Around line 124-126: The deactivate function currently returns an explicit
undefined; remove the explicit "return undefined;" so the function has an
implicit void return (e.g., keep export function deactivate() { } ), updating
the function named deactivate to return void implicitly rather than returning
undefined.
In `@scripts/i18n-check.mjs`:
- Around line 7-10: The directory listings are not deterministic because
fs.readdirSync can return unsorted entries; update the logic that builds locales
(using localesDir and locales) to sort the array after filtering (e.g., call
sort() on locales) and likewise sort namespaceFiles after reading them so both
locales and namespaceFiles are in a stable, deterministic order for CI and
diffs; ensure you apply the same sort step where namespaceFiles is created
(around the code referenced at lines 45-46).
- Around line 17-19: The helper loadJson should wrap fs.readFileSync and
JSON.parse in a try/catch so any read/parse failure includes the file path;
update the loadJson function to catch errors from fs.readFileSync(filePath,
"utf8") and JSON.parse(...) and throw a new Error that contains the filePath
(and original error message or set the original error as the cause) so logs and
stack traces clearly show which locale file failed to parse.
In `@src/components/launch/UpdateToastWindow.tsx`:
- Around line 102-115: The effect contains no-op expressions `void
payload.phase`, `void payload.version`, and `void payload.progressPercent` that
should be removed; update the useEffect in UpdateToastWindow so it only resets
state (call setDragOffsetX(0) and set dragState.current) and, if you need to
document why these properties matter, add a short comment mentioning
payload.phase/version/progressPercent rather than using `void` expressions; keep
the dependency array as [payload].
In `@src/components/video-editor/AddCustomFontDialog.tsx`:
- Around line 111-112: The dialog keeps previous form values after closing;
update AddCustomFontDialog to reset the name/url form state when the Dialog
closes by detecting the open->false transition (either in the Dialog
onOpenChange handler that calls setOpen or via a useEffect watching open) and
call the form state setters (e.g., setFontName and setFontUrl or the existing
name/url state setters) to clear values; ensure this runs both when the user
closes the dialog via DialogTrigger or programmatic setOpen(false).
In `@src/utils/platformUtils.ts`:
- Line 17: The code uses the deprecated navigator.platform check (the expression
using navigator.platform with /Mac|iPhone|iPad|iPod/); replace it with a modern,
non-deprecated strategy: prefer navigator.userAgentData.platform or parse
navigator.userAgent as a fallback in browsers, and when running under Electron
use process.platform (or Electron API) for reliable OS detection; wrap this
logic in a small helper (e.g., isMacPlatform or detectPlatform) that returns the
boolean previously computed and replace the direct navigator.platform usage with
that helper.
In `@tsconfig.json`:
- Line 8: The tsconfig.json entry "ignoreDeprecations": "5.0" is a global
suppression that should be time-boxed or documented; update tsconfig.json to
either replace this broad suppression with a targeted approach (e.g., remove the
global "ignoreDeprecations" and apply scoped suppression or compiler flags only
for the affected files) or add a clear comment next to the "ignoreDeprecations":
"5.0" key stating why it exists, what deprecations it covers, and the planned
removal date/issue number (and create a tracking issue/PR if none exists) so
future TypeScript upgrades won’t silently inherit the suppression.
🪄 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: 03f4a4cd-572d-4dd0-8c21-8aee76b86da7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (78)
.eslintrc.cjsREADME.mdREADME.zh-CN.mdbiome.jsonextension-sources/insomniachooman.cool-cursors/index.jsextension-sources/insomniachooman.cool-cursors/recordly-extension.jsonextension-sources/webadderall.more-wallpapers/index.jsextension-sources/webadderall.more-wallpapers/recordly-extension.jsonpackage.jsonpostcss.config.cjsscripts/benchmark-export-queues.mjsscripts/build-native-helpers.mjsscripts/build-whisper-runtime.mjsscripts/build-windows-capture.mjsscripts/i18n-check.mjsscripts/native-helper-manifest.mjsscripts/postinstall.mjssrc/App.csssrc/App.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/launch/SourceSelector.module.csssrc/components/launch/SourceSelector.tsxsrc/components/launch/UpdateToastWindow.tsxsrc/components/ui/accordion.tsxsrc/components/ui/audio-level-meter.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/content-clamp.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/popover.tsxsrc/components/ui/select.tsxsrc/components/ui/slider.tsxsrc/components/ui/sonner.tsxsrc/components/ui/switch.tsxsrc/components/ui/tabs.tsxsrc/components/ui/toggle-group.tsxsrc/components/ui/toggle.tsxsrc/components/video-editor/AddCustomFontDialog.tsxsrc/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/ArrowSvgs.tsxsrc/components/video-editor/CropControl.tsxsrc/components/video-editor/ExportSettingsMenu.tsxsrc/components/video-editor/ExtensionIcon.tsxsrc/components/video-editor/ExtensionManager.tsxsrc/components/video-editor/FormatSelector.tsxsrc/components/video-editor/KeyboardShortcutsHelp.tsxsrc/components/video-editor/PlaybackControls.tsxsrc/components/video-editor/ProjectBrowserDialog.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/components/video-editor/SliderControl.tsxsrc/components/video-editor/TutorialHelp.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/audio.test.tssrc/components/video-editor/captionLayout.tssrc/components/video-editor/captionStyle.tssrc/components/video-editor/index.tssrc/components/video-editor/types.tssrc/components/video-editor/webcamOverlay.tssrc/index.csssrc/lib/assetPath.tssrc/lib/customFonts.tssrc/lib/geometry/squircle.tssrc/lib/mediaTiming.test.tssrc/lib/mediaTiming.tssrc/lib/shortcuts.tssrc/lib/utils.tssrc/lib/wallpapers.tssrc/main.tsxsrc/utils/aspectRatioUtils.tssrc/utils/platformUtils.tstailwind.config.cjstsconfig.jsontsconfig.node.jsonvite.config.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/index.css (1)
7-10:⚠️ Potential issue | 🟠 MajorRestore accessible keyboard focus indicators.
Line 8 removes outlines globally (
outline: none !important), which hides keyboard focus cues app-wide. This is still an accessibility blocker.Suggested fix
- *:focus { - outline: none !important; - -webkit-tap-highlight-color: transparent; - } + *:focus { + -webkit-tap-highlight-color: transparent; + } + + *:focus:not(:focus-visible) { + outline: none; + } + + *:focus-visible { + outline: 2px solid hsl(var(--ring)); + outline-offset: 2px; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.css` around lines 7 - 10, The global rule "*:focus { outline: none !important; }" removes keyboard focus indicators; remove that declaration and replace it with an accessible focus style using :focus-visible (or restrict to specific interactive elements) and a visible outline/box-shadow that meets contrast and size guidelines, and avoid !important; update the rule referencing the "*:focus" selector to use ":focus-visible" or element-specific selectors (e.g., button, a, input) and provide a clear visible focus style instead.
🤖 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/types.ts`:
- Around line 139-143: getClipSourceEndMs mixes timeline start (clip.startMs)
with source duration when applying speed, causing coordinate mismatches in
downstream clipsToTrims/trimRegions; change it to compute the end in the clip's
source coordinate space by using the clip's source start property (e.g.,
clip.sourceStartMs or the clip's source-start field) instead of clip.startMs,
and return Math.round(sourceStart + displayDurationMs * speed) so trimRegions
and clipsToTrims compare like-for-like coordinates.
In `@src/index.css`:
- Line 83: The declaration "-webkit-user-select: none;" is violating the
stylelint rule declaration-empty-line-before; open src/index.css, locate the CSS
rule containing -webkit-user-select and make its blank-line spacing match the
project's convention used elsewhere (either remove the empty line immediately
before the declaration or insert the required empty line to be consistent with
surrounding declarations), ensuring the same spacing pattern as neighboring
properties, then re-run stylelint to confirm the violation is resolved.
---
Duplicate comments:
In `@src/index.css`:
- Around line 7-10: The global rule "*:focus { outline: none !important; }"
removes keyboard focus indicators; remove that declaration and replace it with
an accessible focus style using :focus-visible (or restrict to specific
interactive elements) and a visible outline/box-shadow that meets contrast and
size guidelines, and avoid !important; update the rule referencing the "*:focus"
selector to use ":focus-visible" or element-specific selectors (e.g., button, a,
input) and provide a clear visible focus style instead.
🪄 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: 48393310-edec-41ae-adaf-1e7fe678a34c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (74)
.eslintrc.cjsREADME.mdREADME.zh-CN.mdbiome.jsonpackage.jsonpostcss.config.cjsscripts/benchmark-export-queues.mjsscripts/build-native-helpers.mjsscripts/build-whisper-runtime.mjsscripts/build-windows-capture.mjsscripts/i18n-check.mjsscripts/native-helper-manifest.mjsscripts/postinstall.mjssrc/App.csssrc/App.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/launch/SourceSelector.module.csssrc/components/launch/SourceSelector.tsxsrc/components/launch/UpdateToastWindow.tsxsrc/components/ui/accordion.tsxsrc/components/ui/audio-level-meter.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/content-clamp.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/popover.tsxsrc/components/ui/select.tsxsrc/components/ui/slider.tsxsrc/components/ui/sonner.tsxsrc/components/ui/switch.tsxsrc/components/ui/tabs.tsxsrc/components/ui/toggle-group.tsxsrc/components/ui/toggle.tsxsrc/components/video-editor/AddCustomFontDialog.tsxsrc/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/ArrowSvgs.tsxsrc/components/video-editor/CropControl.tsxsrc/components/video-editor/ExportSettingsMenu.tsxsrc/components/video-editor/ExtensionIcon.tsxsrc/components/video-editor/ExtensionManager.tsxsrc/components/video-editor/FormatSelector.tsxsrc/components/video-editor/KeyboardShortcutsHelp.tsxsrc/components/video-editor/PlaybackControls.tsxsrc/components/video-editor/ProjectBrowserDialog.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/components/video-editor/SliderControl.tsxsrc/components/video-editor/TutorialHelp.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/audio.test.tssrc/components/video-editor/captionLayout.tssrc/components/video-editor/captionStyle.tssrc/components/video-editor/index.tssrc/components/video-editor/types.tssrc/components/video-editor/webcamOverlay.tssrc/index.csssrc/lib/assetPath.tssrc/lib/customFonts.tssrc/lib/geometry/squircle.tssrc/lib/mediaTiming.test.tssrc/lib/mediaTiming.tssrc/lib/shortcuts.tssrc/lib/utils.tssrc/lib/wallpapers.tssrc/main.tsxsrc/utils/aspectRatioUtils.tssrc/utils/platformUtils.tstailwind.config.cjstsconfig.jsontsconfig.node.jsonvite.config.ts
✅ Files skipped from review due to trivial changes (55)
- tsconfig.json
- package.json
- src/components/video-editor/captionStyle.ts
- src/lib/mediaTiming.test.ts
- tsconfig.node.json
- README.md
- postcss.config.cjs
- src/components/ui/sonner.tsx
- scripts/postinstall.mjs
- scripts/build-whisper-runtime.mjs
- src/lib/utils.ts
- src/App.css
- src/main.tsx
- src/components/video-editor/webcamOverlay.ts
- src/components/video-editor/PlaybackControls.tsx
- src/components/video-editor/ProjectBrowserDialog.tsx
- tailwind.config.cjs
- src/components/ui/toggle.tsx
- src/components/ui/accordion.tsx
- src/components/video-editor/SliderControl.tsx
- src/components/video-editor/AnnotationOverlay.tsx
- src/lib/geometry/squircle.ts
- src/components/ui/audio-level-meter.tsx
- vite.config.ts
- src/components/ui/dialog.tsx
- src/components/video-editor/index.ts
- src/components/ui/label.tsx
- .eslintrc.cjs
- scripts/i18n-check.mjs
- src/utils/platformUtils.ts
- src/components/video-editor/audio.test.ts
- scripts/build-native-helpers.mjs
- src/components/launch/SourceSelector.module.css
- src/components/ui/button.tsx
- src/components/ui/slider.tsx
- src/components/ui/toggle-group.tsx
- scripts/benchmark-export-queues.mjs
- src/components/ui/card.tsx
- src/components/ui/select.tsx
- src/components/ui/popover.tsx
- src/lib/wallpapers.ts
- src/components/video-editor/ShortcutsConfigDialog.tsx
- biome.json
- src/components/ui/tabs.tsx
- src/lib/mediaTiming.ts
- scripts/build-windows-capture.mjs
- src/components/launch/UpdateToastWindow.tsx
- src/components/ui/dropdown-menu.tsx
- src/components/video-editor/ArrowSvgs.tsx
- src/utils/aspectRatioUtils.ts
- src/components/video-editor/captionLayout.ts
- scripts/native-helper-manifest.mjs
- src/components/launch/SourceSelector.tsx
- src/lib/customFonts.ts
- src/lib/shortcuts.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/components/video-editor/FormatSelector.tsx
- src/App.tsx
- src/components/video-editor/CropControl.tsx
- src/components/ui/switch.tsx
- src/components/video-editor/ExportSettingsMenu.tsx
- src/components/ui/input.tsx
- src/components/video-editor/KeyboardShortcutsHelp.tsx
- src/components/video-editor/AddCustomFontDialog.tsx
- src/components/video-editor/ExtensionIcon.tsx
- src/components/ui/content-clamp.tsx
- src/lib/assetPath.ts
- src/components/video-editor/TutorialHelp.tsx
- src/components/video-editor/AnnotationSettingsPanel.tsx
- README.zh-CN.md
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 (3)
scripts/build-windows-capture.mjs (1)
19-24:⚠️ Potential issue | 🟠 MajorFail fast for unsupported architectures instead of silently forcing x64.
Lines 19 and 23 map every non-
arm64host tox64without validation. This silently produces mismatched artifacts on unsupported architectures (e.g.,ia32), especially problematic sincegeneratorArchis passed to CMake as-A ${generatorArch}at lines 124 and 133. While the script validates the platform at line 25, there is no corresponding architecture check.🔧 Proposed fix
-const bundledDir = path.join( - projectRoot, - "electron", - "native", - "bin", - process.arch === "arm64" ? "win32-arm64" : "win32-x64", -); +const archConfig = { + x64: { bundleDir: "win32-x64", generatorArch: "x64" }, + arm64: { bundleDir: "win32-arm64", generatorArch: "ARM64" }, +}[process.arch]; + +if (!archConfig) { + console.error(`[build-windows-capture] Unsupported architecture: ${process.arch}`); + process.exit(1); +} + +const bundledDir = path.join(projectRoot, "electron", "native", "bin", archConfig.bundleDir); const bundledExePath = path.join(bundledDir, "wgc-capture.exe"); const helperId = "wgc-capture"; -const generatorArch = process.arch === "arm64" ? "ARM64" : "x64"; +const generatorArch = archConfig.generatorArch;Note:
build-cursor-monitor.mjsline 19 uses the same pattern and should be updated similarly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-windows-capture.mjs` around lines 19 - 24, The script currently maps any non-arm64 host to x64 using process.arch when computing the target triple and generatorArch (see variables used: bundledExePath, helperId, generatorArch), which silently produces mismatched artifacts on unsupported architectures; change this to explicitly validate process.arch (allow only "arm64" and "x64"/"x86_64" as appropriate), throw/exit immediately with a clear error for unsupported architectures, and compute the target triple and generatorArch only after this check; apply the same validation fix to the analogous logic in build-cursor-monitor.mjs.src/components/video-editor/VideoEditor.tsx (1)
2272-2293:⚠️ Potential issue | 🟠 MajorUse
effectiveSpeedRegionsfor preview audio sync too.This derived list now carries clip-level speed changes into rendering/export, but the preview audio sync logic still reads raw
speedRegions. The result is that changing a clip speed updates the video path while companion/audio tracks continue syncing at 1x in the editor.🤖 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 2272 - 2293, The preview audio sync logic is still reading raw speedRegions instead of the merged effectiveSpeedRegions, so clip-level speed changes aren't applied to preview audio; update any usages in VideoEditor.tsx where preview/audio sync reads speedRegions (e.g., in the preview audio sync selector, effect, or function that computes audio playback rate or timeline mapping) to use effectiveSpeedRegions instead. Search for references to speedRegions within the preview/audio sync code paths and replace them with the effectiveSpeedRegions memoized variable (or pass effectiveSpeedRegions into the audio-sync functions/props) so preview audio follows the same merged speed regions used for rendering/export.src/components/video-editor/SettingsPanel.tsx (1)
1503-1530:⚠️ Potential issue | 🟡 MinorFilter custom videos out of the image tab.
customImagesnow contains both uploaded image data URLs and picked video paths, but this grid renders the whole array. A custom video added viahandleVideoUpload()will therefore show up under both Image and Video, which makes the tab model inconsistent.Suggested fix
- {customImages.map((imageUrl, idx) => { + {customImages + .filter((imageUrl) => !isVideoWallpaperSource(imageUrl)) + .map((imageUrl, idx) => { const isSelected = getWallpaperTileState(imageUrl); return renderWallpaperImageTile(imageUrl, isSelected, { key: `custom-${idx}`, @@ ), }); - })} + })}🤖 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 1503 - 1530, The image-grid is rendering customImages which now includes video paths, causing uploaded videos to appear in the Image tab; fix by filtering out video entries before rendering. Update the map call that renders renderWallpaperImageTile to iterate over customImages.filter(img => !isVideoWallpaperSource(img)) (or equivalent predicate) so only non-video data URLs are passed to renderWallpaperImageTile; keep the existing callbacks and handlers (onWallpaperChange, handleRemoveCustomImage) and the isVideoWallpaperSource checks can be removed or will always be false for items in this filtered list.
🧹 Nitpick comments (5)
src/components/launch/UpdateToastWindow.tsx (1)
106-108: Unreachable guard in drag reset effect.
dragResetKeyis never falsy here ("empty"fallback), so this check can’t trigger. Either remove the guard or make the fallback nullable so the guard is meaningful.Suggested cleanup
- const dragResetKey = payload + const dragResetKey = payload ? `${payload.phase}:${payload.version}:${payload.progressPercent ?? ""}:${payload.detail}:${payload.delayMs}:${payload.isPreview ? "1" : "0"}:${payload.primaryAction ?? ""}` - : "empty"; + : null; useEffect(() => { - if (!dragResetKey) { + if (dragResetKey === null) { return; } setDragOffsetX(0); dragState.current = { pointerId: null, startX: 0, active: false, }; }, [dragResetKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/UpdateToastWindow.tsx` around lines 106 - 108, The guard "if (!dragResetKey) return" inside the drag reset useEffect is unreachable because dragResetKey is always a non-empty string (the selector provides an "empty" fallback); either remove the guard altogether or make the selector/fallback nullable so the check can be meaningful—update the selector that supplies dragResetKey (or its default value "empty") to return null/undefined when no key exists, or simply delete the guard in UpdateToastWindow.tsx and rely on the existing logic that handles the "empty" sentinel.electron/ipc/handlers.ts (2)
1214-1229: Missingpressurefield inHookCursorEventtype.The
CursorTelemetryPointinterface (insrc/components/video-editor/types.ts) includes an optionalpressure?: numberfield, but the newHookCursorEventtype does not define apressureproperty. If uiohook or platform-specific hooks ever emit pressure data (e.g., from stylus input), it will be silently discarded.If pressure telemetry is intended for future stylus/pen support, consider adding:
🔧 Proposed fix to add pressure field
type HookCursorEvent = { button?: number mouseButton?: number x?: number y?: number screenX?: number screenY?: number + pressure?: number data?: { button?: number mouseButton?: number x?: number y?: number screenX?: number screenY?: number + pressure?: number } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/ipc/handlers.ts` around lines 1214 - 1229, The HookCursorEvent type is missing an optional pressure field so any stylus/pen pressure telemetry emitted by hooks will be dropped; update the HookCursorEvent type declaration to include pressure?: number (and likewise inside data if present) so it matches CursorTelemetryPoint's optional pressure field and preserves pressure telemetry from hooks (reference HookCursorEvent and CursorTelemetryPoint).
1231-1233: Type aliasHookEventNameis unused.The type
HookEventNameis defined butUiohookLikeuses inline string literals'mousedown' | 'mouseup' | 'mousemove'instead. Consider either usingHookEventNameinUiohookLikefor consistency or removing the unused type.♻️ Proposed fix to use the type alias consistently
type UiohookLike = { - on?: (eventName: HookEventName, listener: HookCursorHandler) => void - off?: (eventName: HookEventName, listener: HookCursorHandler) => void - removeListener?: (eventName: HookEventName, listener: HookCursorHandler) => void + on?: (eventName: 'mousedown' | 'mouseup' | 'mousemove', listener: HookCursorHandler) => void + off?: (eventName: 'mousedown' | 'mouseup' | 'mousemove', listener: HookCursorHandler) => void + removeListener?: (eventName: 'mousedown' | 'mouseup' | 'mousemove', listener: HookCursorHandler) => void start?: () => void stop?: () => void uIOhook?: UiohookLike uiohook?: UiohookLike Uiohook?: UiohookLike default?: UiohookLike }Or alternatively, use
HookEventNameinUiohookLike:+type HookEventName = 'mousedown' | 'mouseup' | 'mousemove' + type UiohookLike = { - on?: (eventName: HookEventName, listener: HookCursorHandler) => void + on?: (eventName: HookEventName, listener: HookCursorHandler) => void // ... (already using HookEventName) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/ipc/handlers.ts` around lines 1231 - 1233, The type alias HookEventName is declared but not used; update the UiohookLike type to reference HookEventName instead of repeating the inline union (or remove HookEventName if you prefer to keep inline literals). Locate the UiohookLike declaration and replace its event name union `'mousedown' | 'mouseup' | 'mousemove'` with the HookEventName alias so the type is used consistently across the file (or delete the unused HookEventName if you opt for the inline literals approach).scripts/build-windows-capture.mjs (1)
129-131: Keep and log the original VS 2022 configure error before fallback.Line 129 swallows the original error, so unrelated failures are mislabeled as “generator not found.”
🛠️ Proposed improvement
-} catch { - console.log("[build-windows-capture] VS 2022 generator not found, trying VS 2019..."); +} catch (error) { + console.warn( + `[build-windows-capture] VS 2022 configure failed (${error?.message ?? "unknown error"}). Trying VS 2019...`, + ); try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-windows-capture.mjs` around lines 129 - 131, The catch block after attempting the VS 2022 configure is currently swallowing the thrown error; update that catch to capture the error (e.g., catch (err)) and log the original error before falling back to VS 2019 so unrelated failures aren't misreported. Specifically, modify the try/catch around the VS 2022 configure in scripts/build-windows-capture.mjs to log the captured error (using console.error or processLogger.error) with a clear prefix like "[build-windows-capture] VS 2022 configure error:" and then continue to the existing fallback logic that tries VS 2019.src/components/ui/content-clamp.tsx (1)
64-80: Preventive refactor: Consider composing handlers or reordering props to prevent accidental override.The
{...props}on line 79 (and also line 53 in the non-truncated path) comes after internal handlers, which means consumer-providedonMouseEnter,onMouseLeave,onFocus,onBlur,onClick,onKeyDown,role, ortabIndexwould silently override core functionality. While no current usages pass these props, the type signature (ContentClampProps extends React.HTMLAttributes<HTMLDivElement>) allows it, creating a maintainability risk.Composing handlers or applying internal behavior after the spread would be more defensive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/content-clamp.tsx` around lines 64 - 80, The span in ContentClamp (the element that uses {...props}) currently spreads consumer props after defining internal handlers (openPopover, scheduleClose, setOpen, handleKeyDown) which allows consumers to silently override core behavior; fix by either moving {...props} before the internal props or, better, compose/merge consumer handlers with the internals: read consumer onMouseEnter/onMouseLeave/onFocus/onBlur/onClick/onKeyDown/role/tabIndex from props, create composed handlers that call the internal handler then the consumer handler (or vice versa) and use those composed handlers on the span, and pass through non-conflicting remaining props; update references to setOpen/openPopover/scheduleClose/handleKeyDown accordingly so internal behavior always runs while still honoring consumer callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 990-1014: Add startup migration logic that detects and renames old
openscreen-* helper binaries to the new recordly-* names so existing users
aren’t broken; specifically, in the app initialization run a small migration
routine that looks in app.getPath('userData')/native-tools for the old filenames
(e.g. openscreen-screencapturekit-helper, openscreen-system-cursors,
openscreen-native-cursor-monitor, openscreen-window-list), and if they exist and
the corresponding new paths returned by getSystemCursorHelperBinaryPath,
getNativeCursorMonitorBinaryPath, getNativeWindowListBinaryPath (and the
screencapture helper path function) do not exist, move/rename them (use
fs.rename or fs.promises.rename), handle and log errors, and ensure this runs
before any code that spawns or reads those binaries so the rest of the code
continues to use the new recordly-* names.
In `@src/components/ui/content-clamp.tsx`:
- Around line 28-32: The delayed close scheduled by scheduleClose uses
timeoutRef.current and can race with keyboard and click toggles; update the
keyboard handler (the onKeyDown or similar keyboard toggle) and the click
handler (the element that toggles open via click) to first clear any pending
timeout (clearTimeout(timeoutRef.current) and set timeoutRef.current = null)
before calling setOpen or toggling, or factor that logic into a small helper
(e.g., clearCloseTimeout) and call it from both handlers so a pending scheduled
close cannot immediately re-close the popover after a manual reopen.
In `@src/components/video-editor/projectPersistence.ts`:
- Around line 320-338: The normalization currently rebuilds each zoom region but
omits the ZoomRegion.mode, losing "manual"/"auto" semantics; update the mapper
that builds the region object (the block returning id, startMs, endMs, depth,
focus) to preserve mode by adding a mode property set from region.mode (and
validate/guard it — e.g., allow "manual" or "auto" and fallback to a sensible
default such as "manual" if missing/invalid). Keep the rest of the normalization
(clamp, isFiniteNumber, DEFAULT_ZOOM_DEPTH) unchanged.
- Around line 383-385: The project persistence currently accepts any finite
number for region.speed which can later be cast into SpeedRegion["speed"] in
VideoEditor.tsx via clip.speed and produce unsupported playback rates; update
the code that builds clipRegions (and any place that reads region.speed) to
validate and normalize speeds into the allowed playback range before
storing/returning them — e.g., use the existing isFiniteNumber check then
clamp/convert region.speed to the supported SpeedRegion["speed"] domain (or
fallback to 1) so effectiveSpeedRegions and playback/export never receive
malformed rates (look for clipRegions, region.speed, clip.speed,
effectiveSpeedRegions, and SpeedRegion["speed"] to apply the validation).
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2568-2588: handleClipSplit currently replaces a clip in
setClipRegions with two new clips but leaves selectedClipId pointing at the
removed region; update handleClipSplit to clear or reselect selection after
splitting by calling setSelectedClipId (or clearing it) to point at one of the
new ids (e.g., leftId or rightId) or null so the footer state isn't stale.
Locate handleClipSplit, nextClipIdRef, and setClipRegions and add a
setSelectedClipId(...) call after generating leftId/rightId (or inside the
setClipRegions updater) to set selection to a valid id or null.
- Around line 1454-1455: When loading normalizedEditor data, don't mark the
timeline as uninitialized just because clipRegions is empty when legacy
trimRegions exist; update the clipInitializedRef setting in the block that calls
setClipRegions(normalizedEditor.clipRegions) so it becomes true if either
normalizedEditor.clipRegions has items OR normalizedEditor.trimRegions exists
and has items. In other words, change the condition that sets
clipInitializedRef.current to consider normalizedEditor.trimRegions (so the
later initialize-full-track-clip path and the clipsToTrims(...) effect won't
overwrite legacy trims).
- Around line 3463-3469: The branch computing backendPreference ignores the
user's ExportSettings when smokeExportConfig.enabled is false and pipelineModel
!== "legacy"; change the fallback for the non-smoke branch to use the persisted
export setting instead of hardcoding "auto". Specifically, keep the legacy case
(pipelineModel === "legacy" => "webcodecs"), keep the smokeExportConfig logic
as-is, and for the else branch read the backend preference from the
ExportSettings object (e.g., exportSettings.backendPreference ?? "auto") so the
UI-selected backend is respected for normal MP4 exports.
---
Outside diff comments:
In `@scripts/build-windows-capture.mjs`:
- Around line 19-24: The script currently maps any non-arm64 host to x64 using
process.arch when computing the target triple and generatorArch (see variables
used: bundledExePath, helperId, generatorArch), which silently produces
mismatched artifacts on unsupported architectures; change this to explicitly
validate process.arch (allow only "arm64" and "x64"/"x86_64" as appropriate),
throw/exit immediately with a clear error for unsupported architectures, and
compute the target triple and generatorArch only after this check; apply the
same validation fix to the analogous logic in build-cursor-monitor.mjs.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1503-1530: The image-grid is rendering customImages which now
includes video paths, causing uploaded videos to appear in the Image tab; fix by
filtering out video entries before rendering. Update the map call that renders
renderWallpaperImageTile to iterate over customImages.filter(img =>
!isVideoWallpaperSource(img)) (or equivalent predicate) so only non-video data
URLs are passed to renderWallpaperImageTile; keep the existing callbacks and
handlers (onWallpaperChange, handleRemoveCustomImage) and the
isVideoWallpaperSource checks can be removed or will always be false for items
in this filtered list.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2272-2293: The preview audio sync logic is still reading raw
speedRegions instead of the merged effectiveSpeedRegions, so clip-level speed
changes aren't applied to preview audio; update any usages in VideoEditor.tsx
where preview/audio sync reads speedRegions (e.g., in the preview audio sync
selector, effect, or function that computes audio playback rate or timeline
mapping) to use effectiveSpeedRegions instead. Search for references to
speedRegions within the preview/audio sync code paths and replace them with the
effectiveSpeedRegions memoized variable (or pass effectiveSpeedRegions into the
audio-sync functions/props) so preview audio follows the same merged speed
regions used for rendering/export.
---
Nitpick comments:
In `@electron/ipc/handlers.ts`:
- Around line 1214-1229: The HookCursorEvent type is missing an optional
pressure field so any stylus/pen pressure telemetry emitted by hooks will be
dropped; update the HookCursorEvent type declaration to include pressure?:
number (and likewise inside data if present) so it matches
CursorTelemetryPoint's optional pressure field and preserves pressure telemetry
from hooks (reference HookCursorEvent and CursorTelemetryPoint).
- Around line 1231-1233: The type alias HookEventName is declared but not used;
update the UiohookLike type to reference HookEventName instead of repeating the
inline union (or remove HookEventName if you prefer to keep inline literals).
Locate the UiohookLike declaration and replace its event name union `'mousedown'
| 'mouseup' | 'mousemove'` with the HookEventName alias so the type is used
consistently across the file (or delete the unused HookEventName if you opt for
the inline literals approach).
In `@scripts/build-windows-capture.mjs`:
- Around line 129-131: The catch block after attempting the VS 2022 configure is
currently swallowing the thrown error; update that catch to capture the error
(e.g., catch (err)) and log the original error before falling back to VS 2019 so
unrelated failures aren't misreported. Specifically, modify the try/catch around
the VS 2022 configure in scripts/build-windows-capture.mjs to log the captured
error (using console.error or processLogger.error) with a clear prefix like
"[build-windows-capture] VS 2022 configure error:" and then continue to the
existing fallback logic that tries VS 2019.
In `@src/components/launch/UpdateToastWindow.tsx`:
- Around line 106-108: The guard "if (!dragResetKey) return" inside the drag
reset useEffect is unreachable because dragResetKey is always a non-empty string
(the selector provides an "empty" fallback); either remove the guard altogether
or make the selector/fallback nullable so the check can be meaningful—update the
selector that supplies dragResetKey (or its default value "empty") to return
null/undefined when no key exists, or simply delete the guard in
UpdateToastWindow.tsx and rely on the existing logic that handles the "empty"
sentinel.
In `@src/components/ui/content-clamp.tsx`:
- Around line 64-80: The span in ContentClamp (the element that uses {...props})
currently spreads consumer props after defining internal handlers (openPopover,
scheduleClose, setOpen, handleKeyDown) which allows consumers to silently
override core behavior; fix by either moving {...props} before the internal
props or, better, compose/merge consumer handlers with the internals: read
consumer
onMouseEnter/onMouseLeave/onFocus/onBlur/onClick/onKeyDown/role/tabIndex from
props, create composed handlers that call the internal handler then the consumer
handler (or vice versa) and use those composed handlers on the span, and pass
through non-conflicting remaining props; update references to
setOpen/openPopover/scheduleClose/handleKeyDown accordingly so internal behavior
always runs while still honoring consumer callbacks.
🪄 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: 19495d27-00a9-4990-83c8-45276c7cd207
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
README.zh-CN.mdbiome.jsonelectron/electron-env.d.tselectron/ipc/handlers.tspackage.jsonscripts/benchmark-export-queues.mjsscripts/build-windows-capture.mjsscripts/i18n-check.mjssrc/components/launch/SourceSelector.tsxsrc/components/launch/UpdateToastWindow.tsxsrc/components/ui/content-clamp.tsxsrc/components/video-editor/AddCustomFontDialog.tsxsrc/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/CropControl.tsxsrc/components/video-editor/ExportSettingsMenu.tsxsrc/components/video-editor/ExtensionManager.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/components/video-editor/SliderControl.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/index.csssrc/lib/assetPath.tssrc/lib/customFonts.tssrc/utils/platformUtils.tstsconfig.jsonvite.config.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/video-editor/timeline/Item.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- package.json
- biome.json
- scripts/i18n-check.mjs
- src/components/video-editor/ShortcutsConfigDialog.tsx
- src/components/video-editor/ExportSettingsMenu.tsx
- src/components/launch/SourceSelector.tsx
- src/components/video-editor/CropControl.tsx
- README.zh-CN.md
- src/components/video-editor/SliderControl.tsx
- src/components/video-editor/AddCustomFontDialog.tsx
- src/components/video-editor/ExtensionManager.tsx
- scripts/benchmark-export-queues.mjs
- src/components/video-editor/AnnotationOverlay.tsx
- src/components/video-editor/AnnotationSettingsPanel.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/video-editor/VideoEditor.tsx (2)
2591-2628: Consider clamping shifted zoom regions to timeline bounds.The logic to shift overlapping zoom regions when moving a clip is a nice UX improvement. However, the shifted zoom region timestamps are not clamped to valid timeline bounds.
If a clip near the end of the timeline is moved further right, associated zoom regions could end up with
startMsorendMsvalues exceeding the video duration, which may cause issues in playback or export.Consider adding bounds clamping
if (isMove) { const delta = startDelta; setZoomRegions((prev) => prev.map((zoom) => { const overlaps = zoom.startMs < oldClip.endMs && zoom.endMs > oldClip.startMs; if (overlaps) { + const totalMs = Math.round(duration * 1000); + const newStart = Math.max(0, zoom.startMs + delta); + const newEnd = Math.min(totalMs, zoom.endMs + delta); return { ...zoom, - startMs: zoom.startMs + delta, - endMs: zoom.endMs + delta, + startMs: newStart, + endMs: Math.max(newStart + 1, newEnd), }; } return zoom; }), ); }Note: This would require adding
durationto the callback's dependencies.🤖 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 2591 - 2628, In handleClipSpanChange, when shifting overlapping zoom regions via setZoomRegions, clamp the computed startMs and endMs to the valid timeline range (e.g., Math.max(0, Math.min(value, duration))) so zoom.startMs and zoom.endMs never go below 0 or above the video duration; update the mapping that returns the adjusted zoom to apply these clamps and add duration to the hook dependency array so the callback re-evaluates when duration changes.
2568-2589: Selection update addresses the stale selection issue, but consider cleaner pattern.The fix correctly addresses the past concern by setting
selectedClipIdto the left clip after splitting. However, callingsetSelectedClipId(leftId)inside thesetClipRegionsupdater function is an unusual pattern that can make the state flow harder to follow.Consider moving the selection update outside the updater:
Suggested cleaner pattern
const handleClipSplit = useCallback((splitMs: number) => { + let leftId: string | null = null; setClipRegions((prev) => { const target = prev.find((c) => splitMs > c.startMs && splitMs < c.endMs); if (!target) return prev; - const leftId = `clip-${nextClipIdRef.current++}`; + leftId = `clip-${nextClipIdRef.current++}`; const rightId = `clip-${nextClipIdRef.current++}`; - setSelectedClipId(leftId); const left: ClipRegion = { id: leftId, startMs: target.startMs, endMs: Math.round(splitMs), speed: target.speed, }; const right: ClipRegion = { id: rightId, startMs: Math.round(splitMs), endMs: target.endMs, speed: target.speed, }; return prev.flatMap((c) => (c.id === target.id ? [left, right] : [c])); }); + if (leftId) { + setSelectedClipId(leftId); + } }, []);🤖 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 2568 - 2589, handleClipSplit currently calls setSelectedClipId inside the setClipRegions updater which mixes state updates; instead, generate the new IDs (using nextClipIdRef.current and incrementing it) before calling setClipRegions, call setClipRegions with an updater that uses those precomputed leftId/rightId to produce the new regions, and then call setSelectedClipId(leftId) immediately after setClipRegions. This keeps handleClipSplit, nextClipIdRef, setClipRegions and setSelectedClipId logic linear and avoids side-effects inside the updater.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2591-2628: In handleClipSpanChange, when shifting overlapping zoom
regions via setZoomRegions, clamp the computed startMs and endMs to the valid
timeline range (e.g., Math.max(0, Math.min(value, duration))) so zoom.startMs
and zoom.endMs never go below 0 or above the video duration; update the mapping
that returns the adjusted zoom to apply these clamps and add duration to the
hook dependency array so the callback re-evaluates when duration changes.
- Around line 2568-2589: handleClipSplit currently calls setSelectedClipId
inside the setClipRegions updater which mixes state updates; instead, generate
the new IDs (using nextClipIdRef.current and incrementing it) before calling
setClipRegions, call setClipRegions with an updater that uses those precomputed
leftId/rightId to produce the new regions, and then call
setSelectedClipId(leftId) immediately after setClipRegions. This keeps
handleClipSplit, nextClipIdRef, setClipRegions and setSelectedClipId logic
linear and avoids side-effects inside the updater.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 27ffa930-ce19-43d5-ad83-a97e290bf923
📒 Files selected for processing (5)
electron/ipc/handlers.tssrc/components/ui/content-clamp.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/types.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/ui/content-clamp.tsx
This was referenced Apr 17, 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
Full UI overhaul across the launcher, editor, and all shared components.
Changes
lucide-reactimports with@phosphor-icons/reacteverywhere.ExtensionIcongains a Lucide→Phosphor alias map for backwards-compatible extension manifestsLaunchWindowandSourceSelectorfully redesigned — new icon rail layout, updated backgrounds, improved source selection UX, audio/video controls, countdown overlayVideoPlaybackoverhauled playback controls and preview panel layout; timeline pill visual refresh (border-radius 16→8px, cyan→blue, 85% height)types.ts,captionLayout.ts,captionStyle.ts,webcamOverlay.tsupdated to support new timeline and export featurespackage.jsonadds@phosphor-icons/react, aligns biome/eslint/tsconfig/vite/tailwindSummary by CodeRabbit
New Features
Bug Fixes
Updates