This repository was archived by the owner on Jun 7, 2026. It is now read-only.
Studio Dasboard with Separated UI for future scalability and Fixes#577
Merged
siddharthvaddem merged 4 commits intoMay 31, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR builds out a studio dashboard entry point for the editor with full new-project and import-video workflows, complete with unsaved-changes guards and drag-and-drop project loading. It connects the renderer to Electron and native bridge via new IPC APIs for path-based project loading, adds a react-based empty state component, wires up the File menu with a New Project action, updates dialog handling for multi-flow navigation, and provides comprehensive i18n across 11 locales. ChangesStudio Dashboard & Project Loading
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 370323c417
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
electron/main.ts (1)
114-127:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winLowkey risky: Line 117 can force-close an existing editor and nuke unsaved work.
The concern is valid. At Line 114, if the focused window isn't an editor,
createEditorWindowWrapper()gets called (Line 117). That function force-closesmainWindowby settingisForceClosing = true(Line 365), which bypasses the unsaved-changes save prompt in the close handler (Line 374). So if an editor is already open but unfocused, this drops all unsaved changes without warning.Better approach: first search
BrowserWindow.getAllWindows()for an existing unfocused editor usingisEditorWindow()and route the action there. Only callcreateEditorWindowWrapper()if no editor exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/main.ts` around lines 114 - 127, The current logic calls createEditorWindowWrapper() (which force-closes mainWindow) whenever the focused window isn't an editor, risking loss of unsaved work; change it to first search BrowserWindow.getAllWindows() for an existing editor via isEditorWindow() and route the action to that unfocused editor (set targetWindow to that window and send the channel or wait for did-finish-load if necessary); only call createEditorWindowWrapper() to create a new editor and assign mainWindow if no existing editor window is found. Ensure you reference BrowserWindow.getFocusedWindow(), BrowserWindow.getAllWindows(), isEditorWindow(), createEditorWindowWrapper(), and mainWindow when making the change so you avoid invoking the force-close path when an editor already exists.
🧹 Nitpick comments (8)
src/i18n/locales/zh-CN/settings.json (1)
1-1: 💤 Low valuenit: utf-8 bom at the start
there's a byte order mark (BOM) before the opening brace. most modern parsers handle it fine, but json specs kinda discourage it. lowkey might wanna check if your editor is adding this automatically - could cause weird issues with stricter parsers down the line.
not blocking anything though, electron/node will parse this just fine.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/zh-CN/settings.json` at line 1, The file settings.json contains a UTF-8 BOM (U+FEFF) before the opening brace; remove the BOM so the file starts with "{" to avoid issues with strict JSON parsers, then save the file as UTF-8 without BOM (adjust your editor/IDE or use "Save with encoding" to do this). Optionally add an .editorconfig or CI lint rule to enforce "utf-8" without BOM for JSON files and re-commit the cleaned settings.json.src/i18n/locales/ar/settings.json (1)
1-1: ⚡ Quick winRemove BOM from all locale JSON files for consistency
All 11
settings.jsonfiles insrc/i18n/locales/have a UTF-8 BOM (U+FEFF) at the start. While Node.js 22.22.1 handles this fine, Vite's bundler and the i18n loader work correctly regardless, it's cleaner to strip it. There's already precedent inscripts/test-windows-native-cursor.mjswhere BOM is explicitly stripped.Affected files (all locale variants)
src/i18n/locales/ar/settings.json src/i18n/locales/en/settings.json src/i18n/locales/es/settings.json src/i18n/locales/fr/settings.json src/i18n/locales/ja-JP/settings.json src/i18n/locales/ko-KR/settings.json src/i18n/locales/ru/settings.json src/i18n/locales/tr/settings.json src/i18n/locales/vi/settings.json src/i18n/locales/zh-CN/settings.json src/i18n/locales/zh-TW/settings.json🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/ar/settings.json` at line 1, Remove the UTF-8 BOM (U+FEFF) at the start of each locale settings.json file listed (e.g., src/i18n/locales/ar/settings.json and the other 10 locale variants) so the files begin with the normal JSON '{' character; open each settings.json, delete the invisible BOM character at the very start, save the file as UTF-8 without BOM, and re-run build/tests to confirm the i18n loader and Vite bundler behave the same.src/hooks/useScreenRecorder.ts (1)
776-811: 💤 Low valuenit: the video-only
getUserMediacall is written twice.low priority, but the
audio: false, video: videoConstraintscall shows up in both the systemAudio-catch branch and the else branch. a tiny helper would dedupe and make the control flow easier to read:♻️ optional cleanup
+ const captureVideoOnly = () => + navigator.mediaDevices.getUserMedia({ + audio: false, + video: videoConstraints, + } as unknown as MediaStreamConstraints); + if (systemAudioEnabled) { try { screenMediaStream = await navigator.mediaDevices.getUserMedia({ audio: { mandatory: { chromeMediaSource: CHROME_MEDIA_SOURCE, chromeMediaSourceId: selectedSource.id, }, }, video: videoConstraints, } as unknown as MediaStreamConstraints); } catch (audioErr) { console.warn("System audio capture failed, falling back to video-only:", audioErr); toast.error(t("recording.systemAudioUnavailable")); - screenMediaStream = await navigator.mediaDevices.getUserMedia({ - audio: false, - video: videoConstraints, - } as unknown as MediaStreamConstraints); + screenMediaStream = await captureVideoOnly(); } } else { - screenMediaStream = await navigator.mediaDevices.getUserMedia({ - audio: false, - video: videoConstraints, - } as unknown as MediaStreamConstraints); + screenMediaStream = await captureVideoOnly(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useScreenRecorder.ts` around lines 776 - 811, Duplicate getUserMedia calls for video-only should be extracted into a small helper to reduce repetition: create a function (e.g., getVideoOnlyStream or fetchVideoOnlyStream) that returns navigator.mediaDevices.getUserMedia({ audio: false, video: videoConstraints } as MediaStreamConstraints) and replace both occurrences where screenMediaStream is assigned in the systemAudioEnabled catch block and the else branch; keep existing references to videoConstraints, screenMediaStream, systemAudioEnabled, audioErr, toast and t unchanged so behavior and error handling remain the same.src/i18n/locales/ko-KR/editor.json (1)
1-1: ⚡ Quick winBOM is present, but it's not actually a problem
Yeah, there's a UTF-8 BOM here (
), but honestly this is a non-issue. 32 other translation files across all locales have the same thing, and Vite's JSON loader handles it gracefully at build time. If it was breaking JSON parsing, you'd already be seeing errors in like, half your locales.If it bugs you, stripping BOM from all the translation files (
dialogs.json,editor.json,settings.jsonacross all locales) would be a nice cleanup—but that's optional polish, not a fix. Just keep it consistent with the rest of the codebase if you touch it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/ko-KR/editor.json` at line 1, The file contains a UTF-8 BOM character at the start of src/i18n/locales/ko-KR/editor.json; remove the leading BOM (the invisible U+FEFF character) from this file and, if you touch other locale files, remove the BOM consistently in dialogs.json, editor.json, and settings.json across locales so all translation JSON files start with a plain '{' and remain consistent with the rest of the codebase and Vite's JSON loader.src/components/video-editor/EditorEmptyState.tsx (2)
110-111: Dialog onOpenChange could cause raceline 110:
onOpenChange={(open) => !open && setDropError(null)}works but kinda subtle. if the dialog is opened programmatically (not by user interaction), this callback fires immediately and could clear the error before the animation completes. thelastDropErrorRefpattern saves you here but it's a bit fragile.nit: could be clearer as
onOpenChange={(open) => { if (!open) setDropError(null); }}but current code works fine.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/EditorEmptyState.tsx` around lines 110 - 111, The onOpenChange handler on the Dialog (onOpenChange={(open) => !open && setDropError(null)}) can clear dropError prematurely when the dialog is toggled programmatically; change the handler to explicitly check for a closing transition and only clear after the dialog has fully closed (e.g., use onOpenChange={(open) => { if (!open) setDropError(null); }} combined with waiting for the Dialog's onClose or animation end callback instead of immediately calling setDropError, and remove reliance on the fragile lastDropErrorRef pattern; target the Dialog component and the setDropError usage to implement this safer clear-on-close behavior.
64-75: ⚡ Quick winfile extension check needs case handling
line 64 checks
f.name.endsWith(".openscreen")which is case-sensitive. someone could dropproject.OpenScreenorPROJECT.OPENSCREENand it'd be rejected. probably rare but easy fix.case-insensitive check
- const projectFile = files.find((f) => f.name.endsWith(".openscreen")); + const projectFile = files.find((f) => f.name.toLowerCase().endsWith(".openscreen"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/EditorEmptyState.tsx` around lines 64 - 75, The file extension check in EditorEmptyState.tsx uses files.find((f) => f.name.endsWith(".openscreen")) which is case-sensitive and will reject variants like "project.OpenScreen"; change the predicate to perform a case-insensitive comparison (e.g., compare f.name.toLowerCase() with ".openscreen") and guard against missing/undefined names (use f?.name or String(f.name) before lowercasing); keep the rest of the flow (setting projectFile, setDropError, and calling window.electronAPI.getPathForFile) unchanged.electron/electron-env.d.ts (1)
169-169: ⚡ Quick wingetPathForFile return type might be wrong
the type says
(file: File) => stringbutwebUtils.getPathForFile()can potentially throw or return empty string on failure. might wanna make thisstring | nullorstring | undefinedfor safety, or at least document that it can throw.safer type
- getPathForFile: (file: File) => string; + getPathForFile: (file: File) => string | null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/electron-env.d.ts` at line 169, The declaration for getPathForFile is too narrow: change its return type from (file: File) => string to reflect failures (e.g., (file: File) => string | null or string | undefined) and add a brief JSDoc note that webUtils.getPathForFile may also throw; then update any callers (or ensure callers already check for null/undefined and catch exceptions) to handle the absence of a path or thrown errors. Reference symbols: getPathForFile and webUtils.getPathForFile.src/i18n/locales/en/editor.json (1)
1-1: ⚡ Quick winBOM character at start of file
there's a UTF-8 BOM () at the beginning of this file. this can cause parsing issues with some JSON parsers or tools. most modern parsers handle it but it's cleaner without it.
nit: remove the BOM if it wasn't intentional. most editors can strip it on save.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/en/editor.json` at line 1, The file begins with a UTF-8 BOM character (U+FEFF) immediately before the opening '{' which can break some JSON parsers; remove that BOM so the file starts with '{', re-save it as UTF-8 without BOM, and run the JSON validator/linters to confirm parsing succeeds; also update your editor/save settings (or project pre-commit hook) to prevent writing BOMs in the future.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 1780-1783: Change the case-sensitive endsWith check in the
drop/project validation to compare the file extension case-insensitively: use
path.extname(filePath).toLowerCase() and compare it to ("." +
PROJECT_FILE_EXTENSION).toLowerCase() (or lowercased PROJECT_FILE_EXTENSION)
instead of filePath.endsWith(`.${PROJECT_FILE_EXTENSION}`) in the validation
block inside electron/ipc/handlers.ts so names like "MyProject.OPENSCREEN" are
accepted; ensure path is imported if not already and keep the same early-return
shape ({ success: false, message: ... }) used around this check.
In `@electron/preload.ts`:
- Line 127: Wrap the call to webUtils.getPathForFile(file) inside getPathForFile
in the preload API with a try/catch so any thrown errors (e.g., inaccessible or
synthetic files) are caught and the function returns null instead of
propagating; update the exposed signature in electron-env.d.ts to return string
| null so callers can handle the nullable path. Ensure you reference
getPathForFile (preload) and webUtils.getPathForFile in the change and log or
silently swallow the caught error per project convention.
In `@src/components/video-editor/EditorEmptyState.tsx`:
- Around line 70-75: Wrap the call to
window.electronAPI.getPathForFile(projectFile) in a try/catch inside
EditorEmptyState (the drop handler) so exceptions from getPathForFile are
caught; on catch call setDropError("load-failed") (and optionally log the error)
and return, and keep the existing !filePath check after the try to handle falsy
returns. This ensures projectFile conversion won't throw and crash the handler.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2179-2186: The quick-start click handler currently calls
window.electronAPI.startNewRecording() and ignores its promise, swallowing
failures; change the empty-state branch to reuse the existing
handleNewRecordingConfirm() flow (or call startNewRecording() and await its
result, then run the same success/error path as handleNewRecordingConfirm) so
errors and { success: false } outcomes are handled the same as when
setShowNewRecordingDialog(true) is used; update the onClick branch that checks
videoPath to invoke handleNewRecordingConfirm() (or await and check the returned
value) and propagate errors to the same UI/error handling logic.
- Around line 716-724: doNewProject only clears file/path state; add a full
per-project editor reset by extracting the media-path clears into a new
resetEditorState helper that also clears timeline/history, undo/redo stacks,
crop state, wallpaper, cursor/mode state, selection(s), and any project-scoped
transient UI flags, then call that helper from doNewProject (replace the
existing state clears with a call to resetEditorState) and reuse the same
resetEditorState from the import flow handler(s) (e.g. your
importVideo/handleImport functions) so importing into an empty project can't
carry over previous edits; reference the current symbols (doNewProject,
nativeBridgeClient.project.clearCurrentVideoPath, setVideoPath,
setVideoSourcePath, setWebcamVideoPath, setWebcamVideoSourcePath,
setCurrentProjectPath, setLastSavedSnapshot) when locating where to plug in the
new reset.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 787-811: When getUserMedia with system audio constraints fails
inside the block that checks systemAudioEnabled, call the setter to disable the
system-audio toggle so UI state matches reality (similar to how the mic path
uses setMicrophoneEnabled(false)); specifically, in the catch handling audioErr
for the block that assigns screenMediaStream with
chromeMediaSource/chromeMediaSourceId, add setSystemAudioEnabled(false) before
showing the toast and falling back to the video-only getUserMedia so the
systemAudioEnabled flag is cleared and the toggle won’t remain on.
In `@src/i18n/locales/ja-JP/settings.json`:
- Line 1: The file begins with a UTF-8 BOM character before the opening brace
"{" which should be removed; open the locale file, delete the BOM at the very
start (so the first character is '{'), and re-save the file encoded as UTF-8
without BOM to ensure cleaner, portable JSON (no code changes required beyond
removing the BOM).
In `@src/i18n/locales/ru/settings.json`:
- Line 1: The JSON file ru/settings.json (and the other locale JSON files like
dialogs.json, editor.json, settings.json in each locale) currently contains a
UTF-8 BOM at the start; remove the leading BOM character from the beginning of
these files so the first byte is the opening brace '{' (perform a bulk cleanup
across all 33 files to strip BOMs), verify files are valid UTF-8 without BOM and
commit the cleaned files.
In `@src/i18n/locales/tr/settings.json`:
- Line 1: Remove the UTF-8 BOM (U+FEFF) present at the start of each locale
settings.json (e.g., settings.json in every locale folder) so the files begin
directly with '{'. Locate files that start with the invisible BOM character and
delete that first character (ensure the first byte is not 0xEF,0xBB,0xBF), then
save and run your JSON/lint checks to confirm parsing still succeeds. Keep all
file encoding as UTF-8 without BOM going forward.
In `@src/i18n/locales/vi/settings.json`:
- Line 1: The file src/i18n/locales/vi/settings.json contains a UTF-8 BOM at the
start which can break some JSON parsers; remove the BOM bytes so the file begins
directly with "{" (i.e., re-save the file without BOM/UTF-8 signature using your
editor or run a tool/command that strips BOM) and ensure the file is saved as
plain UTF-8 without BOM so json loaders consume it correctly.
---
Outside diff comments:
In `@electron/main.ts`:
- Around line 114-127: The current logic calls createEditorWindowWrapper()
(which force-closes mainWindow) whenever the focused window isn't an editor,
risking loss of unsaved work; change it to first search
BrowserWindow.getAllWindows() for an existing editor via isEditorWindow() and
route the action to that unfocused editor (set targetWindow to that window and
send the channel or wait for did-finish-load if necessary); only call
createEditorWindowWrapper() to create a new editor and assign mainWindow if no
existing editor window is found. Ensure you reference
BrowserWindow.getFocusedWindow(), BrowserWindow.getAllWindows(),
isEditorWindow(), createEditorWindowWrapper(), and mainWindow when making the
change so you avoid invoking the force-close path when an editor already exists.
---
Nitpick comments:
In `@electron/electron-env.d.ts`:
- Line 169: The declaration for getPathForFile is too narrow: change its return
type from (file: File) => string to reflect failures (e.g., (file: File) =>
string | null or string | undefined) and add a brief JSDoc note that
webUtils.getPathForFile may also throw; then update any callers (or ensure
callers already check for null/undefined and catch exceptions) to handle the
absence of a path or thrown errors. Reference symbols: getPathForFile and
webUtils.getPathForFile.
In `@src/components/video-editor/EditorEmptyState.tsx`:
- Around line 110-111: The onOpenChange handler on the Dialog
(onOpenChange={(open) => !open && setDropError(null)}) can clear dropError
prematurely when the dialog is toggled programmatically; change the handler to
explicitly check for a closing transition and only clear after the dialog has
fully closed (e.g., use onOpenChange={(open) => { if (!open) setDropError(null);
}} combined with waiting for the Dialog's onClose or animation end callback
instead of immediately calling setDropError, and remove reliance on the fragile
lastDropErrorRef pattern; target the Dialog component and the setDropError usage
to implement this safer clear-on-close behavior.
- Around line 64-75: The file extension check in EditorEmptyState.tsx uses
files.find((f) => f.name.endsWith(".openscreen")) which is case-sensitive and
will reject variants like "project.OpenScreen"; change the predicate to perform
a case-insensitive comparison (e.g., compare f.name.toLowerCase() with
".openscreen") and guard against missing/undefined names (use f?.name or
String(f.name) before lowercasing); keep the rest of the flow (setting
projectFile, setDropError, and calling window.electronAPI.getPathForFile)
unchanged.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 776-811: Duplicate getUserMedia calls for video-only should be
extracted into a small helper to reduce repetition: create a function (e.g.,
getVideoOnlyStream or fetchVideoOnlyStream) that returns
navigator.mediaDevices.getUserMedia({ audio: false, video: videoConstraints } as
MediaStreamConstraints) and replace both occurrences where screenMediaStream is
assigned in the systemAudioEnabled catch block and the else branch; keep
existing references to videoConstraints, screenMediaStream, systemAudioEnabled,
audioErr, toast and t unchanged so behavior and error handling remain the same.
In `@src/i18n/locales/ar/settings.json`:
- Line 1: Remove the UTF-8 BOM (U+FEFF) at the start of each locale
settings.json file listed (e.g., src/i18n/locales/ar/settings.json and the other
10 locale variants) so the files begin with the normal JSON '{' character; open
each settings.json, delete the invisible BOM character at the very start, save
the file as UTF-8 without BOM, and re-run build/tests to confirm the i18n loader
and Vite bundler behave the same.
In `@src/i18n/locales/en/editor.json`:
- Line 1: The file begins with a UTF-8 BOM character (U+FEFF) immediately before
the opening '{' which can break some JSON parsers; remove that BOM so the file
starts with '{', re-save it as UTF-8 without BOM, and run the JSON
validator/linters to confirm parsing succeeds; also update your editor/save
settings (or project pre-commit hook) to prevent writing BOMs in the future.
In `@src/i18n/locales/ko-KR/editor.json`:
- Line 1: The file contains a UTF-8 BOM character at the start of
src/i18n/locales/ko-KR/editor.json; remove the leading BOM (the invisible U+FEFF
character) from this file and, if you touch other locale files, remove the BOM
consistently in dialogs.json, editor.json, and settings.json across locales so
all translation JSON files start with a plain '{' and remain consistent with the
rest of the codebase and Vite's JSON loader.
In `@src/i18n/locales/zh-CN/settings.json`:
- Line 1: The file settings.json contains a UTF-8 BOM (U+FEFF) before the
opening brace; remove the BOM so the file starts with "{" to avoid issues with
strict JSON parsers, then save the file as UTF-8 without BOM (adjust your
editor/IDE or use "Save with encoding" to do this). Optionally add an
.editorconfig or CI lint rule to enforce "utf-8" without BOM for JSON files and
re-commit the cleaned settings.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad976fb7-5aa5-486b-848e-bbd49c9a6da7
📒 Files selected for processing (51)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/ipc/nativeBridge.tselectron/main.tselectron/native-bridge/services/projectService.tselectron/preload.tselectron/windows.tsindex.htmlsrc/App.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/video-editor/EditorEmptyState.tsxsrc/components/video-editor/UnsavedChangesDialog.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/hooks/useScreenRecorder.tssrc/i18n/locales/ar/dialogs.jsonsrc/i18n/locales/ar/editor.jsonsrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/dialogs.jsonsrc/i18n/locales/en/editor.jsonsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/dialogs.jsonsrc/i18n/locales/es/editor.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/dialogs.jsonsrc/i18n/locales/fr/editor.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/ja-JP/dialogs.jsonsrc/i18n/locales/ja-JP/editor.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/dialogs.jsonsrc/i18n/locales/ko-KR/editor.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/ru/dialogs.jsonsrc/i18n/locales/ru/editor.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/tr/dialogs.jsonsrc/i18n/locales/tr/editor.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/vi/dialogs.jsonsrc/i18n/locales/vi/editor.jsonsrc/i18n/locales/vi/settings.jsonsrc/i18n/locales/zh-CN/dialogs.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/dialogs.jsonsrc/i18n/locales/zh-TW/editor.jsonsrc/i18n/locales/zh-TW/settings.jsonsrc/native/client.tssrc/native/contracts.ts
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
Author
|
Hey @siddharthvaddem, I've sent you an email about this Feature as this is kinda gigantic like it was a 5-6hr session, that explains some things plus there's an unlisted video there that explains. Also I've added now with the last commit in a6e91e3, for some reason after figuring out the edge case for the custom zoom, maybe a later merge had it removed or something, now it's more clearer (I'm explaining the bug in the video sent via email) Hope you like so far, I'll check the cursor later (in the current main windows cursor gets baked in either way when cursor button is toggled) I really hope that I tested the most of the edge cases (more than 20-30 QAs ran before shipping) |
5 tasks
makaradam
force-pushed
the
feature/open-studio-and-import
branch
from
52ddaf4 to
a6e91e3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/native-bridge/cursor/recording/windowsNativeRecordingSession.ts`:
- Around line 227-246: The current emission block caches
lastEmittedCursorType/lastEmittedAssetId even when currentAssetId is present but
its bitmap/nativeAsset is not yet available, which suppresses a later real asset
emission; change the logic in the emission branch so you first resolve
nativeAsset = currentAssetId ? this.assets.get(currentAssetId) : null, then: if
currentAssetId && !nativeAsset, call
this.options.onCursorTypeChange(currentType, null) but do NOT update
this.lastEmittedCursorType or this.lastEmittedAssetId (so the real bitmap can
later trigger); otherwise (nativeAsset exists OR currentAssetId is null meaning
no custom asset) update lastEmittedCursorType/lastEmittedAssetId and emit the
payload including the nativeAsset properties when present; this uses
this.assets, this.options.onCursorTypeChange, this.lastEmittedCursorType, and
this.lastEmittedAssetId to locate the code to change.
In `@electron/windows.ts`:
- Around line 274-280: The overlay window is using
screen.getPrimaryDisplay().bounds so it only covers the primary monitor; replace
that with virtual-desktop bounds computed from screen.getAllDisplays() (or, if
you want the overlay on the display with the cursor, use
screen.getCursorScreenPoint() + screen.getDisplayNearestPoint()) and use that
computed bounds when constructing the BrowserWindow (update where bounds is read
and the BrowserWindow options in the win creation). Ensure you compute combined
x/y/width/height from displays' bounds (union) and then pass those values into
new BrowserWindow instead of screen.getPrimaryDisplay().bounds.
In `@src/components/launch/CursorOverlay.tsx`:
- Around line 243-246: The onCursorTypeChange handler currently only calls
setLiveAsset when asset is truthy, which leaves the previous bitmap set when the
main process sends null; update the handler registered via
window.electronAPI.onCursorTypeChange to always call setLiveAsset(asset) (or
setLiveAsset(asset ?? null)) regardless of truthiness and keep
setCursorType(type) as-is so the state clears correctly and the fallback SVG can
render; refer to the onCursorTypeChange registration, setCursorType, and
setLiveAsset symbols in CursorOverlay.tsx when making the change.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1399-1419: The UI strings in SettingsPanel.tsx are hardcoded
(e.g., "Cursor Style", "Native OS", "Custom cursor", "Size") — update these to
use the existing i18n helper (t(...)) like the rest of the panel: replace
literal labels inside the Select/SelectTrigger/SelectItem blocks and other
occurrences around the cursor controls (including the range 1429-1490) with
t('key') calls, using the same translation keys/style used elsewhere in this
component; ensure cursorDisplayMode and onCursorDisplayModeChange behavior is
unchanged and only the displayed strings are wrapped with t(...) so translations
render correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f2c8d81-b664-4712-92bf-a5600e33948d
📒 Files selected for processing (12)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/native-bridge/cursor/recording/factory.tselectron/native-bridge/cursor/recording/windowsNativeRecordingSession.tselectron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.tselectron/preload.tselectron/windows.tssrc/App.tsxsrc/components/launch/CursorOverlay.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- electron/electron-env.d.ts
- src/App.tsx
- src/components/video-editor/VideoEditor.tsx
Owner
|
@makaradam good to merge once updated based off latest |
…or anti-flash - preload + native bridge: getPathForFile, loadProjectFileFromPath, and onMenuNewProject / onMenuImportVideo listeners. - main process: New Project File-menu item (Cmd/Ctrl+N) + menu-new-project. - handlers: broaden import video formats (.m4v, .wmv, .flv, .ts). - windows: keep main's HUD geometry + draggable handler; re-apply the show-on-ready-to-show anti-flash for the HUD and editor windows.
- EditorEmptyState shown when no video is loaded: Import Video, Load Project, and drag-drop of .openscreen files. - "Open Studio" button in the recording HUD (replaces the two separate open-video / open-project buttons). - New Project flow that clears back to the dashboard, prompting to save on unsaved changes; Load Project now prompts too (variant-aware dialog). - Rebuilt on current main, preserving main's export refactor, cursor-clip / zoom-preview / trim-waveform work and the vertical/horizontal HUD tray.
makaradam
force-pushed
the
feature/open-studio-and-import
branch
from
a6e91e3 to
00191c4
Compare
SourceSelector.test.tsx uses toBeInTheDocument but never imported @testing-library/jest-dom, and vitest.config.ts has no setupFiles to register the matchers globally. It only passed when test ordering happened to leak the matcher from another file, making CI intermittently red on main (e.g. the siddharthvaddem#652 merge). Importing jest-dom directly makes the test self-sufficient and deterministic.
Contributor
Author
|
finally got this rebased onto the latest main and sorted out all the conflicts. it's mergeable + CI's green now squashed my commits into 4 cleaner ones few notes on how i handled the conflicts:
Extra: also ended up fixing a flaky test while i was in there (SourceSelector.test.tsx was missing the jest-dom import, it's actually been randomly failing CI on main too, like on the #652 merge). tested locally and the empty-state dashboard, new project (with the unsaved prompt), importing a video, and zoom all worked for me fine. should be good to merge |
siddharthvaddem
approved these changes
May 31, 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Pull Request Template
Description
Studio / Editor Dashboard (Empty State)
Closes #576
Studio / Editor Dashboard (Empty State)
TOOLBAR BEFORE
TOOLBAR AFTER
Removed Open Video and Load Project from the main recorder toolbar; replaced with a single Studio button that opens the editor — a clean entry point that can scale to tutorials, FAQ, changelogs, or sponsor spots later
Built the editor empty state dashboard with Import Video File… and Load Project… buttons, supported formats hint, and a drag & drop zone
Project Management Logic
currentProjectPathwas never nulled out, so switching to the recorder and back would reopen the discarded projectDesign Decision — No "Import Video" in the File Menu
The File menu intentionally does not include an Import Video option.
The editor currently operates on a single video / single timeline model.
Adding File → Import Video would have effectively triggered a "new project"
action under the hood — silently discarding the current session and loading
a new video — which would be deeply confusing for end users with no visual
indication that their work was replaced.
The deliberate choice is to keep video import on the empty state dashboard
only, where the context is unambiguous (there is nothing open to lose).
Once the editor evolves to support multi-clip timelines or video stitching,
a proper File → Import Video flow can be introduced with the right UX
scaffolding around it.
Drag & Drop
.openscreenproject files only.openscreenfile shows an "Unsupported Format" dialog explaining to use Import Video insteadFile.pathwas removed in Electron 32+ (this project runs Electron 41); replaced withwebUtils.getPathForFile()Editor Loading & White Flash Fix
index.htmlnow setsbackground: #09090bas an inline body style so the dark background is applied instantly before any JavaScript loads;VideoEditoris now lazy-loaded viaReact.lazy+Suspensewith a custom dark-background spinner as the fallback, so the code-split chunk download no longer shows a blank white screenBug Fixes
bottom-[68px]vs the actual80pxtoolbar height); corrected so they float above it properlyEditorEmptyStateandVideoEditorwere independently callingloadProjectFile(); fixed by unifying to a single callbackCustom Zoom Regression (Restored Fix)
The image represents that even if you wanted to use "Custom zoom" to put it to the corner, you couldn't because of an "invisible boundary". In the previous PR when "Custom-zoom" was created I already handled this edge case, but for some reason with a possible merge from an earlier version, the code might have been erased causing the bug again.
MOST functional bug fixes (based on recent main):
1103c93 — When isNativeWindowsCaptureAvailable returned reason="missing-helper", the code was throwing an error instead of gracefully falling back. This killed the recording entirely rather than continuing with the standard web MediaRecorder. Fixed by treating missing-helper the same as unsupported-os — silently return false and let the web recorder take over.
28819bd — Even after that fallback was unblocked, recording still failed on Windows because the fallback code path used navigator.mediaDevices.getDisplayMedia(), which requires a setDisplayMediaRequestHandler registered in the main process — that handler was never implemented. Fixed by replacing the platform-branched capture logic with a single getUserMedia + chromeMediaSource: "desktop" path that works on both macOS and Windows.
Localization
Motivation
Type of Change
Related Issue(s)
Closes #576
Screenshots / Video
Video (if applicable):
I'll send an unlisted YouTube Video for this, as this is quite of a big PR
Test Plan
Empty State & Navigation
Drag & Drop
.openscreenfile → project opensUnsaved Changes Guard
UI & Layout
Localization
Checklist
Edge Cases & Fixes
1103c93+28819bd— Recording fallback on WindowsNative Windows capture helper is absent in dev mode and optional in
production. The code was throwing instead of falling back, killing
recording entirely. Fixed by treating
missing-helperas gracefulfallback to web
getUserMedia + chromeMediaSource: "desktop"— workson both macOS and Windows without the binary.
4cf966a— Drag & drop broken on Electron 32+ (File.pathremoved)File.pathwas silently returningundefinedon Electron 41 — droppedfiles appeared to load but nothing happened. Replaced with
webUtils.getPathForFile()exposed viacontextBridgein preload.61de5ea— New Project didn't actually reset stateclearCurrentVideoPath()only nulledcurrentVideoPath.currentProjectPathstayed set in the main process, so switching to the recorder and reopening
the editor reloaded the discarded project. Fixed by also clearing
currentProjectPathandcurrentRecordingSession.c46aea5— Double file picker on Load ProjectBoth
EditorEmptyStateandVideoEditorwere independently callingloadProjectFile(), opening two file pickers back to back. Fixed byunifying to a single callback passed down as a prop.
4694ea4+2faa676— Dialog content flash during close animationRadix UI's closing animation sets the controlled value to
nullbeforethe exit animation completes. Both
UnsavedChangesDialog(New Project vsClose variants) and the drop error dialog (Unsupported Format vs Could Not
Open variants) were snapping to the wrong content mid-animation. Fixed with
a frozen ref that preserves the last non-null variant for the duration of
the animation.
801f1f6— Webcam state leak between projectsAfter discarding a project or importing a new video, the webcam preview
path from the previous session was still mounted. Fixed by explicitly
clearing webcam state on both New Project and video import.
7dd438e— Device selector panels hidden behind toolbarWebcam and microphone selector panels had
bottom-[68px]but the toolbaris actually
80pxtall, causing them to render behind it. Corrected tothe right offset.
7e4595f+7700c19— Imported/recorded video not treated as unsavedImporting a video or finishing a recording bypassed the unsaved changes
flag, so closing or starting a new project skipped the Save dialog
entirely. Fixed by marking the project dirty on both entry points.
96dc6d5— Custom zoom drag stuck near canvas edgesThe drag boundary clamping used a hardcoded scale instead of the custom
zoom scale, creating invisible walls near the canvas edges. The zoom region
couldn't be dragged to the intended position. Restored the correct
scale-aware boundary calculation.
edf5953+3c84f11+770f05f— White flash on editor openThree compounding causes: (1)
<body>had no background so the OS defaultwhite showed before React painted; (2)
VideoEditorwas eagerly bundled,so the JS parse delay showed a blank screen; (3) an
insertCSScall wasn'tapplied early enough. Fixed with an inline
background: #09090bon<body>in
index.htmland lazy-loadingVideoEditorviaReact.lazy + Suspensewith a dark spinner fallback.
Thank you for contributing!
Summary by CodeRabbit
New Features
.openscreenproject files.m4v,.wmv,.flv,.ts)Improvements
Localization