feat:add countdown before record start#460
feat:add countdown before record start#460Galactic99 wants to merge 6 commits intosiddharthvaddem:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a countdown-overlay feature across Electron and renderer: new preload API and types, IPC handlers and overlay state, a frameless always‑on‑top overlay window, a React Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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. 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6526f82776
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 362-397: Maintain a small main-process state object (e.g., {
visible, value }) for the countdown overlay and update it from the ipc handlers:
when handling "countdown-overlay-show" (in the ipcMain.handle block that uses
getCountdownOverlayWindow/createCountdownOverlayWindow) set state.visible = true
and state.value = value, create the window if needed, and if
win.webContents.isLoading() attach a did-finish-load listener that, when fired
and win still exists, sends only the latest state via
win.webContents.send("countdown-overlay-value", state.value) and calls
win.showInactive() only if state.visible is still true; similarly, make
"countdown-overlay-set-value" update state.value and send immediately only if
the webContents is not loading, and make "countdown-overlay-hide" set
state.visible = false and either hide immediately (if not loading) or prevent
the did-finish-load handler from showing the window by relying on the updated
state.
In `@electron/main.ts`:
- Around line 331-341: The hidden countdown overlay is keeping getAllWindows() >
0 and breaking app activation; update the app.on("activate") logic to ignore the
overlay window instead of counting it: when deciding to recreate windows, filter
BrowserWindow.getAllWindows() to only visible/non-destroyed windows and exclude
the countdownOverlayWindow (refer to createCountdownOverlayWindowWrapper and the
countdownOverlayWindow variable) — if that filtered list is empty then recreate
the HUD/editor as before. Ensure you check w.isVisible() and !w.isDestroyed()
(or w !== countdownOverlayWindow) so the overlay doesn't prevent activation.
In `@electron/windows.ts`:
- Around line 190-223: The overlay BrowserWindow is created with show: !HEADLESS
which lets it paint before the renderer/first IPC value arrives; change the
BrowserWindow creation in windows.ts (the win = new BrowserWindow({...}) block)
to create the window hidden (show: false) and remove the HEADLESS-based
immediate show, then call win.show() from the existing IPC-ready path in
electron/ipc/handlers.ts (the handler flow around lines 362-379 that sends the
first value) once the renderer signals it's fully loaded/ready — ensure you
expose the BrowserWindow instance or send an IPC "ready-to-show" message from
the renderer and invoke win.show() only after that handshake to prevent the
blank flash.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 404-451: The cancel path can race with the async setup in
startRecordCountdown/startRecording: thread the current run id into
startRecording (pass runId from startRecordCountdown into startRecording) and
inside startRecording re-check countdownRunId.current === runId after every
await (e.g., after getSelectedSource, getUserMedia, any async device/track
setup) and abort if it changed; when aborting, ensure you stop and release any
acquired MediaStreamTracks and hide overlays/cleanup before returning. Update
cancelCountdown to continue bumping countdownRunId but rely on the guarded
checks in startRecording (which now accepts runId) to prevent recording from
starting after cancel.
🪄 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: 3808dbb7-f9f0-4ced-8eb2-d4596b91caab
📒 Files selected for processing (9)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/windows.tssrc/App.tsxsrc/components/launch/CountdownOverlay.tsxsrc/hooks/useScreenRecorder.tssrc/main.tsx
…ead countdown token
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/hooks/useScreenRecorder.ts (2)
340-341: nit: inconsistent use of safe wrapperthe cleanup effect calls
window.electronAPI.hideCountdownOverlay()directly, while line 452 usessafeHideCountdownOverlay(). if the IPC call throws during unmount, it could cause issues. might be cleaner to use the safe wrapper here too.♻️ Suggested change
countdownRunId.current += 1; - void window.electronAPI.hideCountdownOverlay(); + void safeHideCountdownOverlay();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 340 - 341, In the cleanup effect, replace the direct IPC call window.electronAPI.hideCountdownOverlay() with the safe wrapper safeHideCountdownOverlay() (same one used at line 452) to avoid throwing during unmount; update the cleanup to call safeHideCountdownOverlay() and keep incrementing countdownRunId.current as-is so the unmount path is protected by the existing safe IPC wrapper.
419-422: kinda old-school with the alert() but it worksthis matches the existing pattern at line 461. might be worth converting both to toast notifications eventually for UX consistency, but not a blocker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 419 - 422, In useScreenRecorder, avoid using alert(...) for the selectedSource check; replace the alert(t("recording.selectSource")) call with the same toast/notification mechanism used elsewhere in this hook (the other validation branch) so the UX is consistent, keep the translation key t("recording.selectSource") and the early return behavior tied to selectedSource.
🤖 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 417-429: The hide handler for the countdown overlay
(ipcMain.handle "countdown-overlay-hide") only sets
countdownOverlayState.visible = false and clears countdownOverlayState.value but
never actually hides the BrowserWindow; update the handler that uses
getCountdownOverlayWindow() and win.webContents.send("countdown-overlay-value",
...) to call win.hide() after sending the null value (mirroring the show
handler's use of win.showInactive()) so the overlay window is actually hidden
rather than just emptied.
---
Nitpick comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 340-341: In the cleanup effect, replace the direct IPC call
window.electronAPI.hideCountdownOverlay() with the safe wrapper
safeHideCountdownOverlay() (same one used at line 452) to avoid throwing during
unmount; update the cleanup to call safeHideCountdownOverlay() and keep
incrementing countdownRunId.current as-is so the unmount path is protected by
the existing safe IPC wrapper.
- Around line 419-422: In useScreenRecorder, avoid using alert(...) for the
selectedSource check; replace the alert(t("recording.selectSource")) call with
the same toast/notification mechanism used elsewhere in this hook (the other
validation branch) so the UX is consistent, keep the translation key
t("recording.selectSource") and the early return behavior tied to
selectedSource.
🪄 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: ad152987-5c97-4cdc-94d5-039085c5ccd6
📒 Files selected for processing (7)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/windows.tssrc/components/launch/CountdownOverlay.tsxsrc/hooks/useScreenRecorder.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/launch/CountdownOverlay.tsx
- electron/electron-env.d.ts
- electron/windows.ts
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/hooks/useScreenRecorder.ts (1)
604-639:⚠️ Potential issue | 🟠 MajorThere’s still one async cancel gap in
startRecording().
applyConstraints()at Line 605 is awaited, but the next run-id check does not happen until Line 636. If the user cancels during that constraint negotiation, this path still falls through and starts the recorder anyway. same race, just one await later.small guard, big difference
try { await videoTrack.applyConstraints({ frameRate: { ideal: TARGET_FRAME_RATE, max: TARGET_FRAME_RATE }, width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH }, height: { ideal: TARGET_HEIGHT, max: TARGET_HEIGHT }, }); } catch (constraintError) { console.warn( "Unable to lock 4K/60fps constraints, using best available track settings.", constraintError, ); } + + if (!isCountdownRunActive(countdownRunToken)) { + teardownMedia(); + return; + } let {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 604 - 639, startRecording() can race: after awaiting videoTrack.applyConstraints(...) the code may proceed even if the run was cancelled; add an immediate cancellation guard right after the try/catch (and before calling videoTrack.getSettings()/computeBitrate/selectMimeType) by calling isCountdownRunActive(countdownRunToken) and, if false, call teardownMedia() and return; reference applyConstraints, isCountdownRunActive, countdownRunToken, teardownMedia, and startRecording so you place the early-return check in the correct spot.
🧹 Nitpick comments (1)
electron/ipc/handlers.ts (1)
382-400: looks good, small nit on potential listener stackingIf
countdown-overlay-showis called multiple times while the window is still loading, you'll stack up multipledid-finish-loadlisteners (each using.once()). When load completes, all of them fire andflushCountdownOverlayStateruns multiple times. It's harmless - just redundant sends of the same value - but if you want to be tidy:// track pending listener let pendingFlush = false; // in handler: if (win.webContents.isLoading()) { if (!pendingFlush) { pendingFlush = true; win.webContents.once("did-finish-load", () => { pendingFlush = false; if (!win.isDestroyed()) { flushCountdownOverlayState(win); } }); } } else { flushCountdownOverlayState(win); }lowkey optional though - the current approach works fine, just chatty.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/ipc/handlers.ts` around lines 382 - 400, Multiple calls to the ipcMain handler "countdown-overlay-show" can register multiple win.webContents.once("did-finish-load", ...) listeners causing redundant flushCountdownOverlayState calls; fix it by adding a module-scoped boolean flag (e.g., pendingCountdownFlush) and use it inside the handler that references getCountdownOverlayWindow/createCountdownOverlayWindow: when win.webContents.isLoading() only attach the once listener if pendingCountdownFlush is false, set pendingCountdownFlush = true when attaching, and clear it inside the once callback before calling flushCountdownOverlayState (also guard with win.isDestroyed()), so repeated calls while loading will no-op until the load finishes.
🤖 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/launch/CountdownOverlay.tsx`:
- Around line 15-19: The countdown digits in CountdownOverlay (the div with
className "text-white/90 text-[120px] font-bold leading-none tabular-nums") need
better contrast; add a subtle contrast treatment such as a small text-shadow (or
a semi-transparent dark outline/background) to the digits so they remain
readable on bright/translucent backgrounds—update the className or style for
that element (or its wrapper) to include the chosen shadow/outline/backdrop
utility (e.g., add a faint text-shadow or bg-black/20 with rounded padding)
while preserving sizing and pointer-events-none on the outer container.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 428-453: The countdown overlay paint race occurs because IPC
awaits (safeShowCountdownOverlay / safeSetCountdownOverlayValue) can resolve
after the run was cancelled and repaint; to fix it, after every await of
safeShowCountdownOverlay and safeSetCountdownOverlayValue check that
countdownRunId.current === runId before performing any UI updates (i.e., only
call safeSetCountdownOverlayValue or proceed to next steps when still the
current run), and change the finally block to always call
safeHideCountdownOverlay (remove the conditional guard around
safeHideCountdownOverlay) while still only calling setCountdownActive(false)
when countdownRunId.current === runId if you need that guard; reference
functions/variables: safeShowCountdownOverlay, safeSetCountdownOverlayValue,
safeHideCountdownOverlay, countdownRunId, runId, startRecording,
setCountdownActive.
---
Duplicate comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 604-639: startRecording() can race: after awaiting
videoTrack.applyConstraints(...) the code may proceed even if the run was
cancelled; add an immediate cancellation guard right after the try/catch (and
before calling videoTrack.getSettings()/computeBitrate/selectMimeType) by
calling isCountdownRunActive(countdownRunToken) and, if false, call
teardownMedia() and return; reference applyConstraints, isCountdownRunActive,
countdownRunToken, teardownMedia, and startRecording so you place the
early-return check in the correct spot.
---
Nitpick comments:
In `@electron/ipc/handlers.ts`:
- Around line 382-400: Multiple calls to the ipcMain handler
"countdown-overlay-show" can register multiple
win.webContents.once("did-finish-load", ...) listeners causing redundant
flushCountdownOverlayState calls; fix it by adding a module-scoped boolean flag
(e.g., pendingCountdownFlush) and use it inside the handler that references
getCountdownOverlayWindow/createCountdownOverlayWindow: when
win.webContents.isLoading() only attach the once listener if
pendingCountdownFlush is false, set pendingCountdownFlush = true when attaching,
and clear it inside the once callback before calling flushCountdownOverlayState
(also guard with win.isDestroyed()), so repeated calls while loading will no-op
until the load finishes.
🪄 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: cb995814-d481-4616-85cc-62294934d399
📒 Files selected for processing (9)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/windows.tssrc/App.tsxsrc/components/launch/CountdownOverlay.tsxsrc/hooks/useScreenRecorder.tssrc/main.tsx
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 761-767: The current toggleRecording logic checks countdownActive
before recording, which can let a click intended to stop an already-started
recording cancel the countdown instead; change the branching to prioritize
active recording first: if recording then call stopRecording.current(), else if
countdownActive then call cancelCountdown(), else call startRecordCountdown();
apply the same reorder to the duplicate handler that uses recording,
countdownActive, cancelCountdown and startRecordCountdown (the block around
lines 829-833) so both handlers consistently prioritize stopping an active
recording.
🪄 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: 25789ab7-9c5e-43af-819d-63ee8cc5a984
📒 Files selected for processing (5)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/preload.tssrc/components/launch/CountdownOverlay.tsxsrc/hooks/useScreenRecorder.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/launch/CountdownOverlay.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- electron/electron-env.d.ts
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 339-343: The direct calls to
window.electronAPI.hideCountdownOverlay in the cleanup/cancel paths (around
countdownRunId.current and allowAutoFinalize.current) can reject and are
currently uncaught; replace these direct awaits/calls with the existing safe
wrapper used elsewhere in this module (the module-level safe invoke used for
electronAPI calls) so rejections are swallowed/handled, and apply the same
change to the other occurrence around lines 373–378; ensure you pass the
activeRunId (countdownRunId.current) into the safe wrapper instead of calling
hideCountdownOverlay directly.
🪄 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: 219635e7-5ff2-4cb0-92fc-4ef7074a3c39
📒 Files selected for processing (1)
src/hooks/useScreenRecorder.ts
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 418-432: The issue is that the code only reserves a countdownRunId
after awaiting getSelectedSource(), leaving a race where another invocation can
claim the run; to fix, claim and store the runId before any await by
incrementing countdownRunId.current and assigning runId at the top of
startRecordCountdown (before calling window.electronAPI.getSelectedSource()),
then proceed to await getSelectedSource() and only call setCountdownActive(true)
after you verify selectedSource is non-null; keep the early-return behavior if
no source is selected (no need to call setCountdownActive in that case).
🪄 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: e9e36a68-9d5d-4855-958f-7a61f0ace72a
📒 Files selected for processing (9)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/windows.tssrc/App.tsxsrc/components/launch/CountdownOverlay.tsxsrc/hooks/useScreenRecorder.tssrc/main.tsx
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/App.tsx (1)
17-33: nit: dependency array might be over-specifiedThe
[windowType]dependency causes the effect to re-run wheneverwindowTypechanges, but sincewindowTypeis derived from the URL (which doesn't change for a given window), the effect only really needs to run once on mount. This also meansloadAllCustomFonts()gets called again if state somehow changes.lowkey harmless since the window type won't change during the overlay's lifetime, but
[]would be cleaner if the intent is mount-only initialization.nit: cleaner dependency array
- }, [windowType]); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 17 - 33, The effect in useEffect currently depends on [windowType], causing it to re-run when windowType changes even though windowType is derived once from the URL; change the dependency array to [] so the effect runs only on mount, keep the same logic that reads const type = new URLSearchParams(window.location.search).get("windowType") || "" and calls setWindowType(type) if different, and still call loadAllCustomFonts().catch(...); if your linter complains about exhaustive-deps, add an inline eslint-disable-next-line comment for that rule scoped to this useEffect.electron/ipc/handlers.ts (1)
362-439: Would add one regression test for the loading path.lowkey this block is where the race fix lives now. a focused test for
show(3)→setValue(2/1)andhide()beforedid-finish-loadwould make the cancel/restart behavior a lot less likely to get kinda cursed again later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/ipc/handlers.ts` around lines 362 - 439, Add a regression test that exercises the loading path for the countdown overlay: simulate calling the IPC handler "countdown-overlay-show" with value 3, then call "countdown-overlay-set-value" with 2 and 1 and finally "countdown-overlay-hide" all before the window emits "did-finish-load"; verify that when the window finishes loading (did-finish-load) the flushCountdownOverlayState/flush behavior uses the latest state (visible false/value null if hidden, or the last set value if still visible) and that no stale or duplicate sends occur. Target the handlers "countdown-overlay-show", "countdown-overlay-set-value", and "countdown-overlay-hide" and exercise the win.webContents.isLoading branch by creating a mocked countdown window via getCountdownOverlayWindow/createCountdownOverlayWindow and controlling did-finish-load to assert correct sends to "countdown-overlay-value".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@electron/ipc/handlers.ts`:
- Around line 362-439: Add a regression test that exercises the loading path for
the countdown overlay: simulate calling the IPC handler "countdown-overlay-show"
with value 3, then call "countdown-overlay-set-value" with 2 and 1 and finally
"countdown-overlay-hide" all before the window emits "did-finish-load"; verify
that when the window finishes loading (did-finish-load) the
flushCountdownOverlayState/flush behavior uses the latest state (visible
false/value null if hidden, or the last set value if still visible) and that no
stale or duplicate sends occur. Target the handlers "countdown-overlay-show",
"countdown-overlay-set-value", and "countdown-overlay-hide" and exercise the
win.webContents.isLoading branch by creating a mocked countdown window via
getCountdownOverlayWindow/createCountdownOverlayWindow and controlling
did-finish-load to assert correct sends to "countdown-overlay-value".
In `@src/App.tsx`:
- Around line 17-33: The effect in useEffect currently depends on [windowType],
causing it to re-run when windowType changes even though windowType is derived
once from the URL; change the dependency array to [] so the effect runs only on
mount, keep the same logic that reads const type = new
URLSearchParams(window.location.search).get("windowType") || "" and calls
setWindowType(type) if different, and still call
loadAllCustomFonts().catch(...); if your linter complains about exhaustive-deps,
add an inline eslint-disable-next-line comment for that rule scoped to this
useEffect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0c04f99-3cea-4b4d-82ac-f06c9e445502
📒 Files selected for processing (9)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tselectron/windows.tssrc/App.tsxsrc/components/launch/CountdownOverlay.tsxsrc/hooks/useScreenRecorder.tssrc/main.tsx
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Description
Implemented a dedicated centered countdown overlay before recording start while keeping the HUD visible and stationary at the bottom.
Key implementation details:
Motivation
Helps the user to prepare themselves before recording actually begins.
Type of Change
Related Issue(s)
#458 #453
Screenshots / Video
Video :
After-Countdown-Timer.mp4
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit
New Features
Bug Fixes / Reliability