Skip to content

feat:add countdown before record start#460

Open
Galactic99 wants to merge 6 commits intosiddharthvaddem:mainfrom
Galactic99:feat/countdown-before-record-start
Open

feat:add countdown before record start#460
Galactic99 wants to merge 6 commits intosiddharthvaddem:mainfrom
Galactic99:feat/countdown-before-record-start

Conversation

@Galactic99
Copy link
Copy Markdown

@Galactic99 Galactic99 commented Apr 16, 2026

Description

Implemented a dedicated centered countdown overlay before recording start while keeping the HUD visible and stationary at the bottom.

Key implementation details:

  • Added a separate transparent always-on-top countdown overlay window.
  • Routed a new window type for countdown rendering.
  • Added IPC flow to show, update and hide countdown values.
  • Updated recording flow to run 3-2-1 countdown before start including cancellation handling and race-safety guards.
  • Reused countdown overlay window by hiding it instead of closing it to reduce churn/flicker.
  • Added startup/show timing fixes to reduce first-paint flicker and keep HUD visibility correct.
  • You can cancel the countdown by clicking on the record button while the countdown timer is running

Motivation

Helps the user to prepare themselves before recording actually begins.

Type of Change

  • New Feature

Related Issue(s)

#458 #453

Screenshots / Video

Video :

After-Countdown-Timer.mp4

Testing

  1. Start app.
  2. Verify HUD appears immediately on launch.
  3. Select a recording source.
  4. Click Record and verify centered 3-2-1 countdown overlay appears while HUD remains visible.
  5. Verify recording starts after countdown ends.
  6. Cancel during countdown and verify countdown hides cleanly and no recording starts.
  7. Repeat multiple times to verify no stuck overlay and no race behavior.
  8. Confirm tray interactions still work with HUD/recording flow.

Checklist

  • I have performed a self-review of my code.
  • I have added any necessary screenshots or videos.
  • I have linked related issue(s) and updated the changelog if applicable.

Thank you for contributing!

Summary by CodeRabbit

  • New Features

    • Added a centered, transparent countdown overlay (3‑2‑1) before screen recording: always‑on‑top, non‑interactive, macOS Spaces‑aware, and controllable (show/update/hide). Supports scoped countdown instances and live value subscriptions for real‑time display.
    • Overlay is available as a dedicated window type and rendered when selected.
  • Bug Fixes / Reliability

    • Improved cancellation and validation of countdowns during recording startup to prevent stray or partial recordings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

📝 Walkthrough

Walkthrough

Adds 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 CountdownOverlay component, and countdown orchestration integrated into the screen‑recording hook with run‑ID based cancellation.

Changes

Cohort / File(s) Summary
Type defs & Preload
electron/electron-env.d.ts, electron/preload.ts
Expose showCountdownOverlay, setCountdownOverlayValue, hideCountdownOverlay, and onCountdownOverlayValue on window.electronAPI (invoke + subscription/unsubscribe).
IPC handlers
electron/ipc/handlers.ts
Add overlay state (visible, value, activeRunId), flushCountdownOverlayState, and IPC channels: "countdown-overlay-show", "countdown-overlay-set-value", "countdown-overlay-hide". registerIpcHandlers now accepts create/getCountdownOverlayWindow callbacks. Minor tweaks to start-new-recording and read-binary-file error handling.
Windows & main process
electron/windows.ts, electron/main.ts
New createCountdownOverlayWindow() (420×260, transparent, frameless, always-on-top, ignore mouse). Manage single countdownOverlayWindow instance; macOS activate logic ignores countdown-overlay windows when deciding to show main window.
Renderer entry & UI
src/main.tsx, src/App.tsx, src/components/launch/CountdownOverlay.tsx
Parse windowType from URL, apply transparent backgrounds for overlay windows, render <CountdownOverlay /> for windowType==="countdown-overlay", and add component that subscribes to overlay values and displays the numeric countdown.
Recording hook
src/hooks/useScreenRecorder.ts
Integrate countdown orchestration: countdownRunId, countdownActive, start/cancel countdown flow, safe IPC wrappers (swallow overlay failures), run-validity guards at media/setup stages, and invalidate prior runs during teardown.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • siddharthvaddem

Poem

three… two… one — a little window glows,
ipc murmurs, the big white number shows.
run‑ids guard the chaos, cancel if cursed,
lowkey risky, nit: cleaner on the next burst.
🎛️✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat:add countdown before record start' clearly summarizes the main feature being added—a countdown overlay shown before recording begins—and directly reflects the primary change across the entire changeset.
Description check ✅ Passed The PR description covers all critical sections: detailed description of the feature, clear motivation, type of change marked, related issues linked (#458, #453), testing steps provided, and all checklist items completed. Includes video demonstration.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/hooks/useScreenRecorder.ts Outdated
Comment thread electron/ipc/handlers.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d449a4 and 6526f82.

📒 Files selected for processing (9)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/main.ts
  • electron/preload.ts
  • electron/windows.ts
  • src/App.tsx
  • src/components/launch/CountdownOverlay.tsx
  • src/hooks/useScreenRecorder.ts
  • src/main.tsx

Comment thread electron/ipc/handlers.ts Outdated
Comment thread electron/main.ts
Comment thread electron/windows.ts
Comment thread src/hooks/useScreenRecorder.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/hooks/useScreenRecorder.ts (2)

340-341: nit: inconsistent use of safe wrapper

the cleanup effect calls window.electronAPI.hideCountdownOverlay() directly, while line 452 uses safeHideCountdownOverlay(). 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 works

this 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6526f82 and c10441f.

📒 Files selected for processing (7)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/main.ts
  • electron/preload.ts
  • electron/windows.ts
  • src/components/launch/CountdownOverlay.tsx
  • src/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

Comment thread electron/ipc/handlers.ts Outdated
@Galactic99
Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/hooks/useScreenRecorder.ts (1)

604-639: ⚠️ Potential issue | 🟠 Major

There’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 stacking

If countdown-overlay-show is called multiple times while the window is still loading, you'll stack up multiple did-finish-load listeners (each using .once()). When load completes, all of them fire and flushCountdownOverlayState runs 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d449a4 and c10441f.

📒 Files selected for processing (9)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/main.ts
  • electron/preload.ts
  • electron/windows.ts
  • src/App.tsx
  • src/components/launch/CountdownOverlay.tsx
  • src/hooks/useScreenRecorder.ts
  • src/main.tsx

Comment thread src/components/launch/CountdownOverlay.tsx
Comment thread src/hooks/useScreenRecorder.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between c10441f and f548bef.

📒 Files selected for processing (5)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/preload.ts
  • src/components/launch/CountdownOverlay.tsx
  • src/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

Comment thread src/hooks/useScreenRecorder.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between f548bef and 3cadf9f.

📒 Files selected for processing (1)
  • src/hooks/useScreenRecorder.ts

Comment thread src/hooks/useScreenRecorder.ts
@Galactic99
Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d449a4 and 3845739.

📒 Files selected for processing (9)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/main.ts
  • electron/preload.ts
  • electron/windows.ts
  • src/App.tsx
  • src/components/launch/CountdownOverlay.tsx
  • src/hooks/useScreenRecorder.ts
  • src/main.tsx

Comment thread src/hooks/useScreenRecorder.ts Outdated
@Galactic99
Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/App.tsx (1)

17-33: nit: dependency array might be over-specified

The [windowType] dependency causes the effect to re-run whenever windowType changes, but since windowType is derived from the URL (which doesn't change for a given window), the effect only really needs to run once on mount. This also means loadAllCustomFonts() 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) and hide() before did-finish-load would 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d449a4 and 42dfe05.

📒 Files selected for processing (9)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/main.ts
  • electron/preload.ts
  • electron/windows.ts
  • src/App.tsx
  • src/components/launch/CountdownOverlay.tsx
  • src/hooks/useScreenRecorder.ts
  • src/main.tsx

@Galactic99
Copy link
Copy Markdown
Author

@coderabbitai resume

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

✅ Actions performed

Reviews resumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant