Added the new recording button so that user does not exit the entire application #307
Open
Ayush765-spec wants to merge 2 commits intosiddharthvaddem:mainfrom
Open
Added the new recording button so that user does not exit the entire application #307Ayush765-spec wants to merge 2 commits intosiddharthvaddem:mainfrom
Ayush765-spec wants to merge 2 commits intosiddharthvaddem:mainfrom
Conversation
📝 WalkthroughWalkthroughThe changes introduce a "switch to HUD" feature allowing users to transition from the video editor back to the HUD wrapper interface. New IPC communication routes the request from the renderer through the Electron preload bridge to the main process, which closes the editor window and reinitializes the HUD display. Changes
Sequence DiagramsequenceDiagram
participant User
participant VideoEditor as VideoEditor UI
participant Preload as Preload Bridge
participant IPC as IPC Main
participant MainProcess as Main Process
User->>VideoEditor: Click "New Recording" button
VideoEditor->>VideoEditor: Show "New Recording" dialog
User->>VideoEditor: Confirm dialog
VideoEditor->>VideoEditor: Clear video path & reset session
VideoEditor->>Preload: Call electronAPI.switchToHud()
Preload->>IPC: ipcRenderer.invoke("switch-to-hud")
IPC->>MainProcess: IPC handler triggered
MainProcess->>MainProcess: Set isForceClosing = true
MainProcess->>MainProcess: Close mainWindow (no confirmation)
MainProcess->>MainProcess: Set mainWindow = null
MainProcess->>MainProcess: Call showMainWindow()
MainProcess->>User: Display HUD interface
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 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}
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
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 |
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.
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