feat: Add Restart Recording Functionality#216
feat: Add Restart Recording Functionality#216siddharthvaddem merged 3 commits intosiddharthvaddem:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0727b61de7
ℹ️ 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".
…r session handling
📝 WalkthroughWalkthroughAdded a restart capability to screen recording: Changes
Sequence DiagramsequenceDiagram
actor User
participant LaunchWindow
participant useScreenRecorder
participant MediaRecorder as Browser MediaRecorder
User->>LaunchWindow: Click restart button
LaunchWindow->>useScreenRecorder: restartRecording()
useScreenRecorder->>useScreenRecorder: set discardRecording = true
useScreenRecorder->>MediaRecorder: abort/stop current recording
MediaRecorder->>useScreenRecorder: stop event
useScreenRecorder->>useScreenRecorder: detect discardRecording, clear chunks, await stop completion
useScreenRecorder->>MediaRecorder: start new recording
MediaRecorder->>useScreenRecorder: new recording session active
useScreenRecorder->>LaunchWindow: notify recording restarted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/hooks/useScreenRecorder.ts`:
- Around line 380-397: restartRecording currently attaches a "stop" listener
whenever mediaRecorder.current is non-null and sets discardRecording.current
without guarding recorder state or concurrent calls, which can hang awaiting a
stop that already happened and leak discard into the next session; update
restartRecording to be single-flight by using a restarting flag (e.g.,
restarting.current) to ignore duplicate invocations, only attach the "stop"
listener if mediaRecorder.current?.state === "recording" (otherwise resolve
immediately), call stopRecording.current() only when recorder is recording,
await the resolved stop promise, clear restarting.current before/after
startRecording(), and ensure discardRecording.current is reset appropriately so
the next startRecording() isn't affected by a prior pending restart.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b70049e8-38de-4576-b06b-de96b3c134ef
📒 Files selected for processing (2)
src/components/launch/LaunchWindow.tsxsrc/hooks/useScreenRecorder.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/useScreenRecorder.ts (1)
44-44: Consider declaring accurate async return type.The type declares
() => voidbut the implementation isasync, which returnsPromise<void>. WhilestartRecordinghandles its own errors internally so this is unlikely to cause unhandled rejections, an accurate type allows callers to properly await if needed.♻️ Suggested type fix
- restartRecording: () => void; + restartRecording: () => Promise<void>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` at line 44, The declared type for restartRecording is currently () => void but the implementation is async and returns a Promise; update the type signature for restartRecording to () => Promise<void> (or the corresponding async function type in the hook's return/type definition) so callers can await it; locate the restartRecording entry in the useScreenRecorder hook's return/type and change its declaration to Promise<void> while keeping the async implementation unchanged.
🤖 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/hooks/useScreenRecorder.ts`:
- Line 44: The declared type for restartRecording is currently () => void but
the implementation is async and returns a Promise; update the type signature for
restartRecording to () => Promise<void> (or the corresponding async function
type in the hook's return/type definition) so callers can await it; locate the
restartRecording entry in the useScreenRecorder hook's return/type and change
its declaration to Promise<void> while keeping the async implementation
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e71e9a6b-7772-4876-a69d-baba2f9da1c7
📒 Files selected for processing (1)
src/hooks/useScreenRecorder.ts
Pull Request Template
Description
Adds a Restart Recording button to the HUD overlay that discards the current recording and immediately starts a new one from scratch.
Files changed:
Motivation
Users sometimes start recording accidentally or realize they need to redo the beginning (wrong window selected, not ready, etc.). Without this, they'd have to stop recording, wait for the editor to open, discard that file, go back to the HUD, and start again. The restart button short-circuits that by discarding the in-progress recording and instantly starting fresh.
Type of Change
Related Issue(s)
Screenshots / Video
Screenshot (if applicable):
n/a
Video (if applicable):
restart-screen-recording.mov
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit