Added the new recording button so that user does not exit the entire application #307
Merged
siddharthvaddem merged 7 commits intosiddharthvaddem:mainfrom Apr 7, 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds two renderer-exposed APIs — Changes
Sequence DiagramsequenceDiagram
participant User
participant VideoEditor as VideoEditor UI
participant Preload as Preload Bridge
participant IPC as IPC Main
participant Main as Main Process
User->>VideoEditor: Click "New Recording"
VideoEditor->>VideoEditor: open confirm Dialog
User->>VideoEditor: Confirm
VideoEditor->>VideoEditor: clear/reset recording state
VideoEditor->>Preload: electronAPI.startNewRecording()
Preload->>IPC: invoke "start-new-recording"
IPC->>IPC: clear recording session state
IPC->>IPC: if provided -> call switchToHud callback
IPC->>Preload: return { success: true } or { success:false, error }
Preload->>VideoEditor: resolve promise with result
alt success
VideoEditor->>VideoEditor: close dialog
else failure
VideoEditor->>VideoEditor: set error state / log
end
Note right of IPC: separate channel "switch-to-hud" may be invoked
VideoEditor->>Preload: electronAPI.switchToHud() (optional/direct)
Preload->>IPC: invoke "switch-to-hud"
IPC->>Main: call registered switchToHud callback
Main->>Main: set isForceClosing = true
Main->>Main: close mainWindow (skip confirmation) and null it
Main->>Main: showMainWindow() -> HUD displayed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/video-editor/VideoEditor.tsx (2)
476-481: Redundant IPC call:setCurrentRecordingSession(null)afterclearCurrentVideoPath().Based on the IPC handler at
electron/ipc/handlers.ts:762-765,clearCurrentVideoPath()already callssetCurrentRecordingSessionState(null)internally. The subsequentsetCurrentRecordingSession(null)call is redundant.🔧 Suggested simplification
const handleNewRecordingConfirm = useCallback(async () => { setShowNewRecordingDialog(false); await window.electronAPI.clearCurrentVideoPath(); - await window.electronAPI.setCurrentRecordingSession(null); await window.electronAPI.switchToHud(); }, []);🤖 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 476 - 481, The handleNewRecordingConfirm callback makes a redundant IPC call: clearCurrentVideoPath() already clears the recording session state, so remove the extra await window.electronAPI.setCurrentRecordingSession(null) call from handleNewRecordingConfirm; keep setShowNewRecordingDialog(false), await window.electronAPI.clearCurrentVideoPath(), and await window.electronAPI.switchToHud() intact and ensure handleNewRecordingConfirm (the useCallback) still has the correct dependencies.
1418-1420: Hardcoded English strings should use internationalization.The "New Recording" button text (line 1473), dialog title (line 1418), and dialog description (lines 1419-1420) are hardcoded in English, while adjacent UI elements use the internationalization system (
ts("project.load"),ts("project.save")).Consider adding appropriate i18n keys for consistency with the rest of the UI.
Also applies to: 1467-1474
🤖 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 1418 - 1420, Replace the hardcoded English UI strings in VideoEditor (the DialogTitle "New Recording", the DialogDescription "Start a new recording? Your current recording will be discarded.", and the "New Recording" button label) with calls to the i18n helper (e.g. ts("recording.new.title"), ts("recording.new.description"), ts("recording.new.button")). Update the JSX where DialogTitle, DialogDescription, and the Button label are rendered to use ts(...) instead of literals, and add corresponding i18n keys to the translation resource files so the new keys resolve.
🤖 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 476-481: The handleNewRecordingConfirm callback makes a redundant
IPC call: clearCurrentVideoPath() already clears the recording session state, so
remove the extra await window.electronAPI.setCurrentRecordingSession(null) call
from handleNewRecordingConfirm; keep setShowNewRecordingDialog(false), await
window.electronAPI.clearCurrentVideoPath(), and await
window.electronAPI.switchToHud() intact and ensure handleNewRecordingConfirm
(the useCallback) still has the correct dependencies.
- Around line 1418-1420: Replace the hardcoded English UI strings in VideoEditor
(the DialogTitle "New Recording", the DialogDescription "Start a new recording?
Your current recording will be discarded.", and the "New Recording" button
label) with calls to the i18n helper (e.g. ts("recording.new.title"),
ts("recording.new.description"), ts("recording.new.button")). Update the JSX
where DialogTitle, DialogDescription, and the Button label are rendered to use
ts(...) instead of literals, and add corresponding i18n keys to the translation
resource files so the new keys resolve.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fcef48cf-e903-4e9f-99ee-91ca1c14faec
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tssrc/components/video-editor/VideoEditor.tsxsrc/vite-env.d.ts
Contributor
Author
|
hi @siddharthvaddem pls do check the pr and pls tell if you expect any further changes or merge if no further issues..looking forward to it...thanks |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1437-1465: The new recording dialog uses hardcoded English strings
in DialogTitle, DialogDescription and the Cancel/Confirm button labels; replace
those literals with the app's i18n calls (e.g., t(...) or ts(...)) so they are
localized, updating the text for DialogTitle, DialogDescription and the two
button children where setShowNewRecordingDialog and handleNewRecordingConfirm
are used; add appropriate translation keys (e.g.,
"videoEditor.newRecording.title", "videoEditor.newRecording.description",
"videoEditor.newRecording.cancel", "videoEditor.newRecording.confirm") and
ensure the same localization pattern is applied to the similar block around
lines 1492-1499.
- Around line 483-488: The handler handleNewRecordingConfirm currently closes
the dialog before awaiting IPC and calls two IPCs that both clear session state;
change it to first sequentially await the necessary IPC calls (remove the
redundant window.electronAPI.setCurrentRecordingSession(null) if
clearCurrentVideoPath already resets session), only close the dialog
(setShowNewRecordingDialog(false)) after the IPCs succeed, and add error
handling around the awaited calls to surface failure to the user (e.g., set an
error state or reopen the dialog and log the error) while keeping a clear
descriptive log message when an IPC fails; target the symbols
handleNewRecordingConfirm, window.electronAPI.clearCurrentVideoPath,
window.electronAPI.setCurrentRecordingSession, window.electronAPI.switchToHud,
and setShowNewRecordingDialog when making these changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 764c5d96-51bf-474c-919e-bea93b86dcc6
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
Contributor
Author
|
hi @siddharthvaddem resolved the issues flagged by coderabbit |
siddharthvaddem
requested changes
Apr 5, 2026
Owner
siddharthvaddem
left a comment
siddharthvaddem
left a comment
There was a problem hiding this comment.
LGTM overall
might want a few modifications wrt the experience. For instance when I save this as a project and then later try to start a new recording, the recording will be discarded is very misleading. because neither the raw source video is being deleted plus the project file is also saved. if that makes sense?
Contributor
Author
|
@siddharthvaddem how about i just change the wordings to "Return to the recoder... Your current session is saved" and also change the colour of the confirm button from red to green for eg... if you confirm i can go ahead and make the changes.. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/video-editor/VideoEditor.tsx (1)
125-125: Prevent double-submit on “Confirm” while IPC is in flight.Rapid clicks can trigger duplicate IPC calls before the first one resolves. Add an in-flight flag and disable both dialog buttons during submission.
Proposed refactor
- const [showNewRecordingDialog, setShowNewRecordingDialog] = useState(false); + const [showNewRecordingDialog, setShowNewRecordingDialog] = useState(false); + const [isStartingNewRecording, setIsStartingNewRecording] = useState(false); const handleNewRecordingConfirm = useCallback(async () => { + if (isStartingNewRecording) return; + setIsStartingNewRecording(true); try { await window.electronAPI.clearCurrentVideoPath(); await window.electronAPI.switchToHud(); setShowNewRecordingDialog(false); } catch (err) { console.error("Failed to start new recording:", err); setError("Failed to start new recording: " + String(err)); + } finally { + setIsStartingNewRecording(false); } - }, []); + }, [isStartingNewRecording]); <button type="button" onClick={() => setShowNewRecordingDialog(false)} + disabled={isStartingNewRecording} className="px-4 py-2 rounded-md bg-white/10 text-white hover:bg-white/20 text-sm font-medium transition-colors" > {t("newRecording.cancel")} </button> <button type="button" onClick={handleNewRecordingConfirm} + disabled={isStartingNewRecording} - className="px-4 py-2 rounded-md bg-red-500/80 text-white hover:bg-red-500 text-sm font-medium transition-colors" + className="px-4 py-2 rounded-md bg-red-500/80 text-white hover:bg-red-500 disabled:opacity-50 disabled:pointer-events-none text-sm font-medium transition-colors" > {t("newRecording.confirm")} </button>Also applies to: 488-497, 1490-1503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` at line 125, Add an "in-flight" boolean state (e.g., isSubmitting) alongside showNewRecordingDialog and use it in the confirm handlers (e.g., handleConfirmNewRecording / the existing confirm IPC call sites around lines 125, 488-497, 1490-1503) to prevent duplicate sends: in the confirm handler, if isSubmitting return early; otherwise set isSubmitting = true, send the IPC call and await its completion, then in finally set isSubmitting = false and close the dialog via setShowNewRecordingDialog(false) only on success; also bind the dialog buttons' disabled prop to isSubmitting so both Confirm and Cancel are disabled while the IPC is in flight.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 488-496: The current handleNewRecordingConfirm calls
window.electronAPI.clearCurrentVideoPath() and window.electronAPI.switchToHud()
as two separate IPCs which can leave the session in a cleared state if the HUD
switch fails; add a new atomic IPC handler in the main process (in
electron/ipc/handlers.ts) named "start-new-recording" that performs the existing
clear-current-video-path mutation and the switch-to-hud operation together and
returns an explicit { success: boolean } result (and error info on failure),
then update handleNewRecordingConfirm to call
window.electronAPI.startNewRecording(), only setShowNewRecordingDialog(false)
when the returned success is true, and surface the returned error info via
setError/console.error on failure.
---
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Line 125: Add an "in-flight" boolean state (e.g., isSubmitting) alongside
showNewRecordingDialog and use it in the confirm handlers (e.g.,
handleConfirmNewRecording / the existing confirm IPC call sites around lines
125, 488-497, 1490-1503) to prevent duplicate sends: in the confirm handler, if
isSubmitting return early; otherwise set isSubmitting = true, send the IPC call
and await its completion, then in finally set isSubmitting = false and close the
dialog via setShowNewRecordingDialog(false) only on success; also bind the
dialog buttons' disabled prop to isSubmitting so both Confirm and Cancel are
disabled while the IPC is in flight.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4b0a5b7-a9ea-4959-ac83-1ac7bdac7eec
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
…(plus lint fixes)
Contributor
Author
|
@siddharthvaddem made the changes.. |
siddharthvaddem
approved these changes
Apr 7, 2026
This was referenced Apr 9, 2026
This was referenced Apr 16, 2026
Closed
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
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
The PR introduces a dedicated button in the application's top toolbar ,letting users cancel their active session in the editor and bounce straight back into the main recording (HUD) overlay
Motivation
Here's a clean write-up you can use in your PR description:
Why This Change Is Needed
Right now, if a user finishes recording and wants to start a fresh one, there is no way to do it from within the app. The only option is to completely quit OpenScreen and reopen it — which is frustrating, breaks the workflow, and feels like a bug rather than intentional design.
This is especially painful for users who record frequently, like developers making demo videos or tutorials, where starting a new recording is something they do repeatedly in a single session.
What Problems It Solves
1. Eliminates the need to quit the app
Users no longer have to force-quit and reopen just to record again. The app stays running the whole time.
2. Faster workflow
Going from "done editing" to "ready to record again" is now just two clicks — button click + confirm. Previously it meant quitting, waiting for relaunch, and setting everything up again.
3. Accidental data loss prevention
The confirmation dialog ("Start a new recording? Your current recording will be discarded.") makes sure nobody loses their work by accidentally clicking the button.
4. Meets basic user expectations
Almost every screen recorder out there has a "start over" or "new recording" option. Not having it made OpenScreen feel incomplete compared to alternatives.
In Short
A small UI addition that removes a genuinely annoying pain point and makes the core recording loop feel smooth and intentional.
Type of Change
Related Issue(s)
#247
Screenshots / Video
Screenshot (if applicable):
Video (if applicable):
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit
New Features
Localization