Skip to content

Conversation

@Brendonovich
Copy link
Contributor

@Brendonovich Brendonovich commented Nov 4, 2025

Extends macOS support back to macOS 12.7 by adding additional version flags when using Cidre APIs.

  • Don't offer system audio capture on macOS <13

Summary by CodeRabbit

  • New Features

    • Exposed system-audio availability check to the frontend; added reusable System Audio toggle root and new Camera/Microphone base components.
  • Improvements

    • UI refactor: standardized camera/mic selection, pill-component customization, explicit null for missing cameras, and toggle tooltips reflecting support/state.
    • Device discovery and audio capture now respect OS version/support checks.
  • Bug Fixes

    • Prevents attempting system audio capture when unsupported.
  • Chores

    • Public bridge/types and events adjusted (minor surface changes).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

Gates system audio capture and desk‑view camera discovery behind macOS 13.0 runtime checks, exposes an is_system_audio_supported API to the frontend, introduces base selection UI components and SystemAudio toggle wiring, and lowers the cidre workspace feature in Cargo.toml from macos_13_0 to macos_12_7.

Changes

Cohort / File(s) Summary
Dependency config
Cargo.toml
Changed workspace cidre feature from macos_13_0 to macos_12_7.
Screen capture core
crates/scap-screencapturekit/src/lib.rs, crates/scap-screencapturekit/src/config.rs, crates/scap-screencapturekit/src/capture.rs
Added pub fn is_system_audio_supported() -> bool (checks cidre::api::macos_available("13.0")); gated set_captures_audio() and audio-output attachment behind that check and wrapped calls in unsafe where required.
Audio capture binary
crates/audio/src/bin/macos-audio-capture.rs
Wrapped set_captures_audio / set_excludes_current_process_audio behind a macos_available("13.0") runtime check; added api import; calls in an unsafe block.
Camera discovery
crates/camera-avfoundation/src/lib.rs
list_video_devices() now always includes built_in_wide_angle_camera and conditionally appends desk_view_camera only when macOS 13.0+ is available at runtime.
Tauri bridge & platform
apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/platform/mod.rs, apps/desktop/src/utils/tauri.ts
Exposed is_system_audio_capture_supported as a Tauri command and TypeScript bridge method; added platform shim using scap_screencapturekit::is_system_audio_supported() on macOS (returns true on other OSes); removed requestNewScreenshot event mapping; updated exported types (e.g., GifQuality).
Frontend queries & UI wiring
apps/desktop/src/utils/queries.ts, apps/desktop/src/routes/(window-chrome)/(main).tsx, apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
Added isSystemAudioSupported Solid query and integrated it into SystemAudio toggle (disabled state, tooltip, guarded click); introduced SystemAudioToggleRoot and switched SystemAudio to use it; changed cameraID lookup to return null when not found.
Selection base components & pills
apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx, .../MicrophoneSelect.tsx, .../TargetSelectInfoPill.tsx
Introduced CameraSelectBase and MicrophoneSelectBase (new exported base components) and updated TargetSelectInfoPill to accept a PillComponent prop (dynamic pill rendering); added runtime permission gating before showing menus; standardized button props (class/iconClass/type="button").

Sequence Diagram(s)

sequenceDiagram
    participant UI as Frontend UI
    participant Cmd as Tauri Command
    participant Core as scap_screencapturekit
    participant Sys as macOS API

    rect rgb(240,248,255)
    Note over UI,Cmd: Capability check flow
    UI->>Cmd: isSystemAudioCaptureSupported()
    Cmd->>Core: is_system_audio_supported()
    Core->>Sys: cidre::api::macos_available("13.0")
    Sys-->>Core: true / false
    Core-->>Cmd: true / false
    Cmd-->>UI: true / false
    end
Loading
sequenceDiagram
    participant Setup as Capture Setup
    participant Check as macOS 13.0 Check
    participant Audio as Audio Config

    rect rgb(245,255,240)
    Note over Setup,Audio: Guarded audio enablement
    Setup->>Check: macos_available("13.0")?
    alt macOS 13.0+
        Check->>Audio: unsafe set_captures_audio(true)
        Audio-->>Setup: audio enabled
    else macOS < 13.0
        Check-->>Setup: skip audio config
    end
    end
Loading
sequenceDiagram
    participant CameraLib as Camera Library
    participant Check as macOS 13.0 Check
    participant DeviceList as Device List

    CameraLib->>DeviceList: Add built_in_wide_angle_camera
    CameraLib->>Check: macos_available("13.0")?
    alt macOS 13.0+
        Check->>DeviceList: Add desk_view_camera
    else macOS < 13.0
        Check-->>DeviceList: Skip desk_view_camera
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Heterogeneous changes across Rust crates, a binary, Tauri bridge, and frontend UI.
  • Pay extra attention to:
    • Consistent use of the availability string "13.0" and the lowered workspace cidre feature.
    • Safety and justification for new unsafe blocks around captures_audio() calls.
    • Tauri/TypeScript bridge changes (removed event and updated GifQuality) for downstream compile/type issues.
    • New UI base components’ props/types and permission-gating flow (Camera/Microphone/SystemAudio).

Possibly related PRs

Poem

🐰 I hopped through code with gentle cheer,

Audio wakes when thirteen draws near,
Desk‑view waits till versions align,
New pills and toggles tidy the line,
A rabbit’s tweak to keep things clear.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly aligns with the PR's core objective of extending macOS support back to version 12.7 by updating Cidre features from macos_13_0 to macos_12_7.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch macos-12.7-support

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.

@Brendonovich Brendonovich marked this pull request as draft November 4, 2025 04:58
@Brendonovich Brendonovich marked this pull request as ready for review November 4, 2025 06:24
Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
crates/scap-screencapturekit/src/config.rs (1)

34-41: Consider adding debug logging for silent no-op behavior.

The method now silently ignores requests to enable audio capture on unsupported systems. While this is functionally correct, it could be helpful to add a debug log when the no-op path is taken, making it easier to diagnose issues during development.

Example:

 pub fn set_captures_audio(&mut self, captures_audio: bool) {
     if crate::is_system_audio_supported() {
         unsafe {
             self.0.set_captures_audio(captures_audio);
         }
+    } else if captures_audio {
+        // Optionally log when audio capture is requested but unsupported
+        #[cfg(debug_assertions)]
+        eprintln!("System audio capture requested but not supported on this macOS version");
     }
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46eefc6 and 8293c67.

📒 Files selected for processing (9)
  • apps/desktop/src-tauri/src/lib.rs (1 hunks)
  • apps/desktop/src-tauri/src/platform/mod.rs (1 hunks)
  • apps/desktop/src/routes/(window-chrome)/(main).tsx (3 hunks)
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (2 hunks)
  • apps/desktop/src/utils/queries.ts (1 hunks)
  • apps/desktop/src/utils/tauri.ts (4 hunks)
  • crates/scap-screencapturekit/src/capture.rs (1 hunks)
  • crates/scap-screencapturekit/src/config.rs (1 hunks)
  • crates/scap-screencapturekit/src/lib.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Files:

  • apps/desktop/src/utils/queries.ts
  • apps/desktop/src/routes/(window-chrome)/(main).tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
  • apps/desktop/src/utils/tauri.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/utils/queries.ts
  • apps/desktop/src/routes/(window-chrome)/(main).tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
  • apps/desktop/src/utils/tauri.ts
**/queries.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not edit auto-generated files named queries.ts.

Files:

  • apps/desktop/src/utils/queries.ts
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.

Files:

  • apps/desktop/src-tauri/src/lib.rs
  • crates/scap-screencapturekit/src/lib.rs
  • apps/desktop/src-tauri/src/platform/mod.rs
  • crates/scap-screencapturekit/src/config.rs
  • crates/scap-screencapturekit/src/capture.rs
crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

  • crates/scap-screencapturekit/src/lib.rs
  • crates/scap-screencapturekit/src/config.rs
  • crates/scap-screencapturekit/src/capture.rs
**/tauri.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not edit auto-generated files named tauri.ts.

Files:

  • apps/desktop/src/utils/tauri.ts
🧠 Learnings (3)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/queries.ts : Do not edit auto-generated files named `queries.ts`.

Applied to files:

  • apps/desktop/src/utils/queries.ts
📚 Learning: 2025-10-28T08:39:42.230Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.

Applied to files:

  • crates/scap-screencapturekit/src/capture.rs
📚 Learning: 2025-10-17T05:58:22.586Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1219
File: crates/enc-avfoundation/src/mp4.rs:350-373
Timestamp: 2025-10-17T05:58:22.586Z
Learning: In crates/enc-avfoundation/src/mp4.rs, the `finish()` method intentionally skips video extension when `is_paused` is true. This is correct behavior because if recording is paused, the video should not be extended beyond the pause point—the pause is user-initiated, unlike the case where ScreenCaptureKit stops providing frames during static content.

Applied to files:

  • crates/scap-screencapturekit/src/capture.rs
🧬 Code graph analysis (7)
apps/desktop/src/utils/queries.ts (1)
apps/desktop/src/utils/tauri.ts (1)
  • commands (7-292)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/platform/mod.rs (1)
  • is_system_audio_capture_supported (62-74)
apps/desktop/src-tauri/src/platform/mod.rs (1)
crates/scap-screencapturekit/src/lib.rs (1)
  • is_system_audio_supported (13-15)
crates/scap-screencapturekit/src/config.rs (1)
crates/scap-screencapturekit/src/lib.rs (1)
  • is_system_audio_supported (13-15)
apps/desktop/src/routes/(window-chrome)/(main).tsx (2)
apps/desktop/src/utils/queries.ts (1)
  • isSystemAudioSupported (115-119)
apps/desktop/src/components/Tooltip.tsx (1)
  • Tooltip (23-47)
crates/scap-screencapturekit/src/capture.rs (1)
crates/scap-screencapturekit/src/lib.rs (1)
  • is_system_audio_supported (13-15)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (2)
apps/desktop/src/routes/(window-chrome)/OptionsContext.tsx (1)
  • useRecordingOptions (9-16)
apps/desktop/src/utils/queries.ts (2)
  • createCurrentRecordingQuery (163-169)
  • isSystemAudioSupported (115-119)
🔇 Additional comments (13)
crates/scap-screencapturekit/src/lib.rs (1)

11-15: LGTM! Clean runtime capability check.

The implementation is straightforward and well-documented. The function provides a clear API for checking macOS 13.0+ availability, which is correctly used throughout the codebase to gate system audio capture.

crates/scap-screencapturekit/src/capture.rs (1)

196-200: LGTM! Proper runtime gating for system audio.

The condition correctly gates audio output addition behind both the system audio support check and the configuration setting. The short-circuit evaluation ensures the unsafe captures_audio() call is only made when system audio is supported.

apps/desktop/src/utils/queries.ts (1)

115-119: LGTM! Appropriate caching strategy for static capability check.

The query correctly uses Number.POSITIVE_INFINITY for staleTime since the OS version won't change during runtime. The implementation properly delegates to the Tauri command.

apps/desktop/src-tauri/src/lib.rs (1)

1876-1876: LGTM! Command properly exposed.

The new platform command is correctly added to the Tauri command surface, making the system audio capability check available to the frontend.

apps/desktop/src/routes/(window-chrome)/(main).tsx (2)

33-33: LGTM! Import added for system audio support check.


956-995: LGTM! Well-structured conditional UI with proper feedback.

The implementation correctly:

  • Queries system audio support status
  • Disables interaction when recording is active or system audio is unsupported
  • Provides a clear tooltip explaining the macOS 13.0+ requirement when unsupported
  • Prevents onClick action when disabled
  • Conditionally wraps the button with a tooltip only when needed

The user experience is clear and the logic is sound.

apps/desktop/src-tauri/src/platform/mod.rs (1)

56-74: LGTM! Clear platform-specific implementation.

The function is well-documented and correctly:

  • Delegates to the scap-screencapturekit check on macOS (which performs the 13.0+ version check)
  • Returns true for non-macOS platforms with a clear comment noting this can be refined later
  • Includes proper Tauri command attributes for exposure to the frontend
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (2)

1-6: LGTM! Imports updated for system audio capability check.


13-49: LGTM! Consistent implementation with proper user feedback.

The implementation mirrors the pattern used in the main route and correctly:

  • Queries system audio support status
  • Computes disabled state based on recording activity and system audio support
  • Provides clear tooltip messaging for unsupported systems
  • Prevents interaction when disabled
  • Conditionally renders the tooltip wrapper

The consistency across both UI implementations is good for maintainability.

apps/desktop/src/utils/tauri.ts (4)

185-192: Well-documented new API for platform capability checking.

The new isSystemAudioCaptureSupported command is properly documented and aligns with the PR objectives. The JSDoc clearly explains the macOS 13.0+ requirement for system audio capture.


352-356: Cosmetic formatting change only.

This is a formatting adjustment with no semantic impact.


404-412: Verify this change is intentional.

The addition of the fast field to GifQuality appears unrelated to the PR's stated objective of supporting macOS 12.7. This may be an unrelated change that was included when the types were regenerated.

If this change is intentional and related to other work, please ensure it's tested. If it's unintentional, consider reverting the backend changes that introduced it or documenting it in the PR description.


485-489: Cosmetic formatting change only.

This is a formatting adjustment with no semantic impact.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
apps/desktop/src/routes/(window-chrome)/(main).tsx (1)

172-180: Refactor for efficiency and idiomaticity.

The current implementation has two issues:

  1. Unnecessary iteration: When !cameraID (line 175), returning null inside the .find() callback causes iteration through every camera in the array, only to ultimately return undefined. Check !cameraID before calling .find() to short-circuit.

  2. Non-idiomatic .find() usage: Returning the camera object (lines 176, 178) instead of a boolean works but is unconventional. Typically, .find() callbacks return boolean predicates.

Apply this diff to improve efficiency and clarity:

 cameraID: () =>
-  cameras.find((c) => {
-    const { cameraID } = rawOptions;
-    if (!cameraID) return null;
-    if ("ModelID" in cameraID && c.model_id === cameraID.ModelID) return c;
-    if ("DeviceID" in cameraID && c.device_id === cameraID.DeviceID)
-      return c;
-    return null;
-  }),
+  {
+    const { cameraID } = rawOptions;
+    if (!cameraID) return null;
+    return cameras.find((c) => {
+      if ("ModelID" in cameraID) return c.model_id === cameraID.ModelID;
+      if ("DeviceID" in cameraID) return c.device_id === cameraID.DeviceID;
+      return false;
+    }) ?? null;
+  },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8293c67 and c400723.

📒 Files selected for processing (2)
  • apps/desktop/src/routes/(window-chrome)/(main).tsx (3 hunks)
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Files:

  • apps/desktop/src/routes/(window-chrome)/(main).tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/routes/(window-chrome)/(main).tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/(main).tsx (2)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1)
  • SystemAudioToggleRoot (23-73)
apps/desktop/src/routes/(window-chrome)/new-main/InfoPill.tsx (1)
  • InfoPill (4-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Clippy
  • GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src/routes/(window-chrome)/(main).tsx (2)

603-603: LGTM!

The import of SystemAudioToggleRoot is correctly structured and the component is properly utilized in the SystemAudio function below.


954-966: Refactor looks good.

The SystemAudio component correctly delegates to SystemAudioToggleRoot, passing the local InfoPill component and icon. The local InfoPill (defined at lines 1061-1076) maintains this file's existing color scheme (bg-blue-3/text-blue-9) rather than using the InfoPill from new-main/InfoPill.tsx (bg-blue-9/text-white), which appears intentional for styling consistency.

Copy link
Contributor

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c400723 and 06a3796.

📒 Files selected for processing (5)
  • apps/desktop/src/routes/(window-chrome)/(main).tsx (5 hunks)
  • apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx (5 hunks)
  • apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx (5 hunks)
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1 hunks)
  • apps/desktop/src/routes/(window-chrome)/new-main/TargetSelectInfoPill.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/TargetSelectInfoPill.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx
  • apps/desktop/src/routes/(window-chrome)/(main).tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/TargetSelectInfoPill.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx
  • apps/desktop/src/routes/(window-chrome)/(main).tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.

Applied to files:

  • apps/desktop/src/routes/(window-chrome)/(main).tsx
🧬 Code graph analysis (4)
apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx (3)
apps/desktop/src/routes/(window-chrome)/new-main/InfoPill.tsx (1)
  • InfoPill (4-17)
apps/desktop/src/utils/tauri.ts (2)
  • CameraInfo (363-363)
  • requestPermission (125-127)
apps/desktop/src/routes/(window-chrome)/new-main/TargetSelectInfoPill.tsx (1)
  • TargetSelectInfoPill (4-38)
apps/desktop/src/routes/(window-chrome)/(main).tsx (4)
apps/desktop/src/routes/(window-chrome)/new-main/CameraSelect.tsx (1)
  • CameraSelectBase (29-113)
apps/desktop/src/routes/(window-chrome)/new-main/InfoPill.tsx (1)
  • InfoPill (4-17)
apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx (2)
  • MicrophoneSelect (21-36)
  • MicrophoneSelectBase (38-158)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (2)
  • SystemAudio (12-20)
  • SystemAudioToggleRoot (22-72)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (3)
apps/desktop/src/routes/(window-chrome)/new-main/InfoPill.tsx (1)
  • InfoPill (4-17)
apps/desktop/src/routes/(window-chrome)/OptionsContext.tsx (1)
  • useRecordingOptions (9-16)
apps/desktop/src/utils/queries.ts (2)
  • createCurrentRecordingQuery (163-169)
  • isSystemAudioSupported (115-119)
apps/desktop/src/routes/(window-chrome)/new-main/MicrophoneSelect.tsx (4)
apps/desktop/src/routes/(window-chrome)/new-main/InfoPill.tsx (1)
  • InfoPill (4-17)
apps/desktop/src/utils/createEventListener.ts (1)
  • createTauriEventListener (30-44)
apps/desktop/src/utils/tauri.ts (2)
  • events (297-339)
  • requestPermission (125-127)
apps/desktop/src/routes/(window-chrome)/new-main/TargetSelectInfoPill.tsx (1)
  • TargetSelectInfoPill (4-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Clippy
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (rust)

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1)

38-39: Consider handling the loading state.

While the tooltip logic has been correctly fixed from the previous review, isDisabled doesn't account for the loading state when systemAudioSupported.data is undefined. During the brief initial query period, users could toggle system audio before the support check completes. Although the backend should prevent unsupported operations and the window is small (given the infinite staleTime), explicitly handling the loading state would improve UX.

Apply this diff to disable the toggle during loading:

 const isDisabled = () =>
-  !!currentRecording.data || systemAudioSupported.data === false;
+  !!currentRecording.data || 
+  systemAudioSupported.data === false || 
+  systemAudioSupported.data === undefined;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 06a3796 and d74668e.

📒 Files selected for processing (1)
  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (3)
apps/desktop/src/routes/(window-chrome)/new-main/InfoPill.tsx (1)
  • InfoPill (4-17)
apps/desktop/src/routes/(window-chrome)/OptionsContext.tsx (1)
  • useRecordingOptions (9-16)
apps/desktop/src/utils/queries.ts (2)
  • createCurrentRecordingQuery (163-169)
  • isSystemAudioSupported (115-119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Clippy
  • GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/desktop/src/routes/(window-chrome)/new-main/SystemAudio.tsx (3)

1-8: LGTM!

The imports are well-organized and all necessary for the new system audio support checking functionality.


13-20: LGTM!

The wrapper component cleanly delegates to SystemAudioToggleRoot with appropriate props, and the cursor class typo from the previous review has been fixed.


47-71: LGTM!

The JSX rendering correctly uses the Dynamic component for flexible pill rendering, properly handles the disabled state, and includes an appropriate tooltip via the title attribute. The isDisabled() check in the onClick handler (line 53) is redundant since the button is already disabled, but this defensive programming is acceptable.

@Brendonovich Brendonovich merged commit 3af8741 into main Nov 4, 2025
17 checks passed
@Brendonovich Brendonovich deleted the macos-12.7-support branch November 4, 2025 09:28
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.

2 participants