feat(launch): add a collapsible microphone test#281
Open
meiiie wants to merge 4 commits intowebadderallorg:mainfrom
Open
feat(launch): add a collapsible microphone test#281meiiie wants to merge 4 commits intowebadderallorg:mainfrom
meiiie wants to merge 4 commits intowebadderallorg:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a microphone self-test feature: new Changes
Sequence DiagramsequenceDiagram
participant User
participant Launch as LaunchWindow<br/>Component
participant Hook as useMicrophoneTest<br/>Hook
participant Media as Browser Media APIs
participant Audio as AudioContext &<br/>AnalyserNode
participant Playback as HTMLAudioElement
User->>Launch: Click "Start Test"
Launch->>Hook: startTest()
Hook->>Media: navigator.mediaDevices.getUserMedia({ audio: { deviceId } })
Media-->>Hook: MediaStream
Hook->>Media: new MediaRecorder(stream)
rect rgba(0, 150, 255, 0.5)
Note over Hook,Audio: Recording phase (metering + recorder)
Hook->>Audio: create AudioContext + AnalyserNode + RAF loop
Audio-->>Hook: level updates
Launch->>Launch: show AudioLevelMeter
end
User->>Launch: Click "Stop Test"
Launch->>Hook: stopTest()
Hook->>Media: stop MediaRecorder / stop tracks
Hook->>Hook: assemble Blob from chunks
rect rgba(150, 0, 150, 0.5)
Note over Hook,Playback: Playback phase
Hook->>Playback: create audio element with blob URL and play()
Playback-->>Hook: play events / ended / error
end
Playback->>Hook: ended
Hook->>Hook: revoke URL / reset state
Launch->>User: show "play last" ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/hooks/microphoneTestMimeType.ts (1)
11-15: Optional: guard the default globals for safer standalone use.
MediaRecorder.isTypeSupported(...)anddocument.createElement("audio")will throwReferenceErrorif the utility is ever invoked outside a browser-like environment (e.g., SSR or a non-jsdom test). It's currently gated byuseMicrophoneTest'ssupportedcheck, so this is not an active bug — but a tiny guard would make the function self-sufficient.♻️ Suggested guard
const isTypeSupported = - options.isTypeSupported ?? ((type: string) => MediaRecorder.isTypeSupported(type)); + options.isTypeSupported ?? + ((type: string) => + typeof MediaRecorder !== "undefined" && MediaRecorder.isTypeSupported(type)); const canPlayType = options.canPlayType ?? - ((type: string) => document.createElement("audio").canPlayType(type)); + ((type: string) => + typeof document !== "undefined" ? document.createElement("audio").canPlayType(type) : "");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/microphoneTestMimeType.ts` around lines 11 - 15, The default functions assigned to isTypeSupported and canPlayType should guard the globals to avoid ReferenceError in non-browser environments: replace the current defaults so isTypeSupported uses MediaRecorder.isTypeSupported only if typeof MediaRecorder !== "undefined" and typeof MediaRecorder.isTypeSupported === "function", otherwise return a safe fallback (e.g., () => false); likewise make canPlayType use document.createElement("audio").canPlayType only if typeof document !== "undefined" and document.createElement is a function, otherwise return a safe fallback (e.g., () => ""). Update the assignments for isTypeSupported and canPlayType in microphoneTestMimeType.ts accordingly so the hook is self-sufficient outside browser-like runtimes.
🤖 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/LaunchWindow.tsx`:
- Around line 1471-1493: The collapsed preview subtitle currently uses
microphoneTestHasPlayback to choose between "microphoneTestReady" and
"microphoneTestIdle", which hides the unsupported message when
microphoneTestSupported is false; change the subtitle logic in the mic preview
row to reuse the existing microphoneTestStatusText (or at least check
microphoneTestSupported and show the unsupported branch) so the unsupported
reason is visible even when the start button is disabled; locate the subtitle
rendering in LaunchWindow.tsx (the element referencing microphoneTestHasPlayback
/ microphoneTestReady / microphoneTestIdle) and replace it with a call/condition
that uses microphoneTestStatusText or the unsupported branch while leaving the
button handlers (setMicrophoneTestExpanded, startTest) and disabled prop
(canRunMicrophoneTest) unchanged.
In `@src/hooks/useMicrophoneTest.ts`:
- Around line 176-210: The playback handlers and the play() catch block in
playCurrentSample currently only avoid clearing playbackAudioRef but still call
setStatus/setError even when the audio is stale; update playbackAudio.onended,
playbackAudio.onerror, and the catch handler in playCurrentSample to first check
if playbackAudioRef.current === playbackAudio and early-return if not, so stale
events don't call setStatus/setError or mutate state, and ensure
clearPlaybackAudio/stopTest still set state for the active audio; add a
unit/test that starts a playback, calls stopTest() or starts a second
playCurrentSample() before the first play() promise settles, and asserts the
stale rejection does not overwrite the current state.
- Around line 279-290: After awaiting beginMetering(stream, session) add a
session staleness check (compare the current session reference/id stored in your
hook to the local session captured before the await) and return early if they
differ so you don't call setHasPlayback, setError, setStatus("recording"), or
recorder.start() for a stale session; likewise, in the catch block only call
stopInputStream() if the session at the time of error still matches the original
session (guard stopInputStream() with the same session check) so you don't shut
down a newer session's stream. Ensure you reference the same session variable
used around beginMetering, recorder, stopInputStream, setStatus, setError and
setHasPlayback when implementing the comparisons.
In `@src/i18n/locales/zh-CN/launch.json`:
- Line 37: Update the translation for the key "microphoneTestUnsupported" so it
no longer blames the device; replace the string "此设备不支持麦克风测试。" with wording that
references the current environment or browser (for example, "当前环境或浏览器不支持麦克风测试。")
so it reflects missing runtime APIs driven by useMicrophoneTest rather than
implying a hardware fault.
---
Nitpick comments:
In `@src/hooks/microphoneTestMimeType.ts`:
- Around line 11-15: The default functions assigned to isTypeSupported and
canPlayType should guard the globals to avoid ReferenceError in non-browser
environments: replace the current defaults so isTypeSupported uses
MediaRecorder.isTypeSupported only if typeof MediaRecorder !== "undefined" and
typeof MediaRecorder.isTypeSupported === "function", otherwise return a safe
fallback (e.g., () => false); likewise make canPlayType use
document.createElement("audio").canPlayType only if typeof document !==
"undefined" and document.createElement is a function, otherwise return a safe
fallback (e.g., () => ""). Update the assignments for isTypeSupported and
canPlayType in microphoneTestMimeType.ts accordingly so the hook is
self-sufficient outside browser-like runtimes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46049cc3-ceb5-476d-a1c7-d2ac7033babc
📒 Files selected for processing (11)
src/components/launch/LaunchWindow.module.csssrc/components/launch/LaunchWindow.tsxsrc/hooks/microphoneTestMimeType.test.tssrc/hooks/microphoneTestMimeType.tssrc/hooks/useMicrophoneTest.tssrc/hooks/useScreenRecorder.tssrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/zh-CN/launch.json
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/launch/LaunchWindow.tsx`:
- Around line 286-288: The microphone self-test stays enabled whenever
micDropdownOpen is true, allowing it to run during recording/countdown; update
the enable logic for useMicrophoneTest (the enabled prop where micDropdownOpen
&& devices.length > 0 and/or the canRunMicrophoneTest check) to also require
!recording && !countdownActive so the test is disabled during countdown or
active recording, and additionally call whatever UI handler closes the mic
dropdown (e.g., setMicDropdownOpen(false) or closeMicDropdown) when recording or
countdown starts so the test UI cannot remain interactive over live recording
controls.
In `@src/hooks/useMicrophoneTest.ts`:
- Around line 330-336: The onstop handler mutates shared refs
(recorderRef.current and stream via stopInputStream()) before verifying the
stopped recorder belongs to the active session, which can stop a newer session;
change recorder.onstop to first check sessionRef.current === session (and
optionally that recorderRef.current === recorder) and return early if not, only
then clear recorderRef.current and call stopInputStream(); add a regression test
that triggers an old recorder.onstop after a new session becomes current to
ensure the guard prevents stopping the new stream.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a578f620-103a-4f04-89ec-c2a61674ca14
📒 Files selected for processing (5)
src/components/launch/LaunchWindow.tsxsrc/hooks/microphoneTestMimeType.tssrc/hooks/useMicrophoneTest.test.tssrc/hooks/useMicrophoneTest.tssrc/i18n/locales/zh-CN/launch.json
✅ Files skipped from review due to trivial changes (1)
- src/i18n/locales/zh-CN/launch.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/microphoneTestMimeType.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/launch/LaunchWindow.tsx (1)
272-342: Mic-test wiring and state machine look solid.The
useMicrophoneTestgating (micDropdownOpen && !recording && !countdownActive && devices.length > 0), thecanRunMicrophoneTestcheck, and the three effects (auto-collapse on dropdown close, auto-close dropdown + collapse on recording/countdown, auto-expand on busy/error) together give a clean state machine. The auto-expand effect's dependency list ([microphoneTestBusy, microphoneTestStatus]) also correctly avoids re-expanding an error card after a manual collapse, since deps don't change between renders while the status is stable.One optional refactor:
microphoneTestStatusTextis computed via an IIFE on every render. Wrapping inuseMemo(or extracting a pure helper) would make intent clearer and avoid the micro re-work on unrelated renders.♻️ Suggested tweak
- const microphoneTestStatusText = (() => { - if (!microphoneTestSupported) { - return t("recording.microphoneTestUnsupported"); - } - - switch (microphoneTestStatus) { - case "recording": - return t("recording.microphoneTestRecording"); - case "playing": - return t("recording.microphoneTestPlaying"); - case "error": - switch (microphoneTestError) { - case "permission-denied": - return t("recording.microphoneTestPermissionDenied"); - case "playback-failed": - return t("recording.microphoneTestPlaybackFailed"); - case "capture-failed": - return t("recording.microphoneTestCaptureFailed"); - default: - return t("recording.microphoneTestUnsupported"); - } - case "idle": - default: - return microphoneTestHasPlayback - ? t("recording.microphoneTestReady") - : t("recording.microphoneTestIdle"); - } - })(); + const microphoneTestStatusText = useMemo(() => { + if (!microphoneTestSupported) { + return t("recording.microphoneTestUnsupported"); + } + switch (microphoneTestStatus) { + case "recording": + return t("recording.microphoneTestRecording"); + case "playing": + return t("recording.microphoneTestPlaying"); + case "error": + switch (microphoneTestError) { + case "permission-denied": + return t("recording.microphoneTestPermissionDenied"); + case "playback-failed": + return t("recording.microphoneTestPlaybackFailed"); + case "capture-failed": + return t("recording.microphoneTestCaptureFailed"); + default: + return t("recording.microphoneTestUnsupported"); + } + default: + return microphoneTestHasPlayback + ? t("recording.microphoneTestReady") + : t("recording.microphoneTestIdle"); + } + }, [microphoneTestSupported, microphoneTestStatus, microphoneTestError, microphoneTestHasPlayback, t]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/LaunchWindow.tsx` around lines 272 - 342, microphoneTestStatusText is recomputed every render via an IIFE; wrap the logic in a useMemo (or extract a pure helper) to avoid unnecessary work and make intent clearer. Move the IIFE into React.useMemo and list its dependencies: microphoneTestSupported, microphoneTestStatus, microphoneTestError, microphoneTestHasPlayback, and t (the translation fn); keep the same switch logic and return values so behavior of microphoneTestStatusText, microphoneTestBusy, canRunMicrophoneTest and the existing effects (which reference microphoneTestStatus) remain unchanged.
🤖 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/LaunchWindow.tsx`:
- Around line 1444-1461: The primary mic test button is disabled based solely on
canRunMicrophoneTest, which can flip false mid-test and prevent the user from
calling stopTest; update the disabled logic on the button (component using
microphoneTestBusy, canRunMicrophoneTest, onClick handlers startTest/stopTest,
and className micTestButton inside micTestActions) so it remains enabled while
microphoneTestBusy is true (e.g., disabled only when !canRunMicrophoneTest AND
!microphoneTestBusy), keeping the current onClick behavior that calls stopTest
when microphoneTestBusy and startTest otherwise.
---
Nitpick comments:
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 272-342: microphoneTestStatusText is recomputed every render via
an IIFE; wrap the logic in a useMemo (or extract a pure helper) to avoid
unnecessary work and make intent clearer. Move the IIFE into React.useMemo and
list its dependencies: microphoneTestSupported, microphoneTestStatus,
microphoneTestError, microphoneTestHasPlayback, and t (the translation fn); keep
the same switch logic and return values so behavior of microphoneTestStatusText,
microphoneTestBusy, canRunMicrophoneTest and the existing effects (which
reference microphoneTestStatus) remain unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e1874967-f048-43da-918c-f9819dc6a15b
📒 Files selected for processing (3)
src/components/launch/LaunchWindow.tsxsrc/hooks/useMicrophoneTest.test.tssrc/hooks/useMicrophoneTest.ts
✅ Files skipped from review due to trivial changes (2)
- src/hooks/useMicrophoneTest.test.ts
- src/hooks/useMicrophoneTest.ts
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.
Description
Adds a collapsible microphone test to the launch microphone menu so users can quickly record and play back a short microphone sample before starting a capture.
Motivation
Users currently only get a live level meter, which makes it hard to tell whether microphone gain, device selection, or monitoring actually sound right before recording. This change adds a Discord-style quick microphone check without changing the main recording flow.
Type of Change
Related Issue(s)
N/A
Testing Guide
Start Mic Test, speak, then stop the test and verify playback starts automaticallyPlay Last Testto verify replay worksValidation run locally:
npx @biomejs/biome check src/hooks/useScreenRecorder.ts src/components/launch/LaunchWindow.tsx src/components/launch/LaunchWindow.module.css src/hooks/useMicrophoneTest.ts src/hooks/microphoneTestMimeType.ts src/hooks/microphoneTestMimeType.test.ts src/i18n/locales/en/launch.json src/i18n/locales/es/launch.json src/i18n/locales/ko/launch.json src/i18n/locales/nl/launch.json src/i18n/locales/zh-CN/launch.jsonnpm run i18n:checknpx tsc --noEmitnpx vitest --run src/hooks/microphoneTestMimeType.test.ts src/hooks/recordingMimeType.test.ts src/hooks/useScreenRecorder.test.tsChecklist
Summary by CodeRabbit
New Features
UI / Styling
Localization
Tests
Chores