Improved Start Recording keybinds #1025
Conversation
WalkthroughAdds persistent recording settings and target modes, expands recording start and picker events and hotkey actions, changes ShowCapWindow::Main to accept an initialTargetMode passed to window init script, wires frontend to consume it, adds a desktop permission, and includes small logging and UI wiring updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant OS as Global Shortcut / UI
participant Backend as Tauri Backend
participant Win as Main Window (Web)
participant Store as RecordingSettingsStore
rect rgba(230,240,255,0.25)
note over User,Backend: Open recording picker via hotkey
User->>OS: press picker hotkey
OS->>Backend: hotkeys::handle_hotkey(OpenRecordingPicker*)
Backend->>Backend: emit RequestOpenRecordingPicker{ target_mode }
Backend->>Win: ShowCapWindow::Main{ init_target_mode }
Backend->>Win: init script sets window.__CAP__.initialTargetMode
Win->>Win: onMount reads initialTargetMode and opens picker if set
end
rect rgba(240,255,230,0.25)
note over User,Store: Start recording with mode preference
User->>OS: press Start (Studio/Instant) or UI triggers event
OS/Win->>Backend: emit RequestStartRecording{ mode }
Backend->>Store: read RecordingSettingsStore (mic/camera/target/mode/system_audio)
Backend->>Backend: set_mic_input / set_camera_input
Backend->>Backend: start_recording(..., capture_target, mode)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
apps/desktop/src-tauri/src/camera.rs (1)
123-128: No-op semicolon is fine; consider a clearer log message.The added semicolon doesn’t change behavior. The extra log helps, but “DONE” is vague—prefer something like “camera preview thread exited” for grep-ability.
Apply this diff to improve the log:
- info!("DONE"); + info!("Camera preview thread exited");apps/desktop/src/routes/(window-chrome)/(main).tsx (1)
249-251: Debounce start on event to avoid double-trigger.Tiny race: two events before currentRecording updates could enqueue two starts. Guard with isPending.
Apply this diff:
-createTauriEventListener(events.requestStartRecording, () => { - if (!isRecording()) toggleRecording.mutate(); -}); +createTauriEventListener(events.requestStartRecording, () => { + if (!isRecording() && !toggleRecording.isPending) toggleRecording.mutate(); +});apps/desktop/src-tauri/src/recording.rs (1)
637-642: Preserve last target when reopening the main window.Instead of None, pass an init_target_mode derived from the just-deleted recording’s capture_target for a smoother UX.
Apply this diff to use a previously captured variable:
- let _ = ShowCapWindow::Main { - init_target_mode: None, - } + let _ = ShowCapWindow::Main { + init_target_mode, + } .show(&app) .await;Add before cancelling the recording (near Line 611), to compute init_target_mode:
// derive target mode before we drop/cancel `recording` use crate::recording_settings::RecordingTargetMode; let init_target_mode = Some(match recording.inputs().capture_target { ScreenCaptureTarget::Window { .. } => RecordingTargetMode::Window, ScreenCaptureTarget::Area { .. } => RecordingTargetMode::Area, ScreenCaptureTarget::Display { .. } => RecordingTargetMode::Display, });apps/desktop/src-tauri/capabilities/default.json (1)
44-45: Scope core:window:allow-set-ignore-cursor-events to overlay windows (least privilege)core:window:allow-set-ignore-cursor-events is the correct permission and capabilities can be scoped to specific windows via the capability's "windows" field. Remove it from the default capability (apps/desktop/src-tauri/capabilities/default.json — currently windows: ["*"]) and add a dedicated capability, e.g.:
{
"identifier": "overlay-ignore-cursor",
"permissions": [
"core:window:allow-set-ignore-cursor-events"
],
"windows": [
"target-select-overlay*",
"window-capture-occluder"
]
}Assign that capability to the overlay/occluder windows where needed.
apps/desktop/src/utils/queries.ts (2)
113-121: Coalesce systemAudio to a boolean; avoid undefined writes to the store.
RecordingSettingsStore.systemAudioisboolean(see apps/desktop/src/utils/tauri.ts), but_state.captureSystemAudiois optional. Writingundefinedcan desync types and persistence.Apply this diff:
- recordingSettingsStore.set({ + recordingSettingsStore.set({ target: _state.captureTarget, micName: _state.micName, cameraId: _state.cameraID, mode: _state.mode, - systemAudio: _state.captureSystemAudio, + systemAudio: _state.captureSystemAudio ?? false, });
123-125: Persisted state order is fine; consider removing legacy storage event mirroring.With a dedicated recordingSettingsStore, continuing to react to
window.storagechanges can fight with the new persistence path or cause oscillations across windows.If cross-window sync is still required, migrate to the same tauri store and drop the
storagelistener for consistency. I can provide a small helper to mirror via the store.apps/desktop/src-tauri/src/windows.rs (2)
241-270: Initialization script for initialTargetMode is good; minor robustness nits.
- Using
serde_json::to_string(init_target_mode)is correct (yields"display"/null). Consider adding a trailing semicolon to the assignment to guard against concatenation.- Consider handling permission denial without extra heap allocation:
return Self::Setup.show(app).await;(Box::pin is unnecessary here).- window.__CAP__.initialTargetMode = {} + window.__CAP__.initialTargetMode = {};
272-275: Use named window levels instead of magic numbers (macOS).
50isn’t self-documenting. Prefer a constant (e.g.,const RECORDING_MAIN_LEVEL: NSWindowLevel = 50;) or map to a known Cocoa level for clarity.apps/desktop/src-tauri/src/recording_settings.rs (1)
27-38: Fix copy-paste in error message.Error refers to “general settings store” instead of “recording settings store”.
- Err(e) => Err(format!("Failed to deserialize general settings store: {e}")), + Err(e) => Err(format!("Failed to deserialize recording settings store: {e}")),apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (1)
80-95: Dynamic actions list is fine; small optimization optional.You could wrap this in
createMemoto avoid recomputation during unrelated reactive updates.apps/desktop/src-tauri/src/hotkeys.rs (1)
132-176: Persist hotkey changes from set_hotkey for durability.Hotkeys are loaded from the plugin store at startup, but
set_hotkeydoesn’t write back. If the UI fails to sync the store for any reason, changes won’t survive restarts. Persisting here hardens the flow.Here’s a minimal write-back you can add at the end of
set_hotkey(outside the selected range; adapt names to avoid shadowing):// after updating `store.hotkeys` and (un)registering with global_shortcut: if let Ok(plugin_store) = app.store("store") { // Serialize current in-memory store and persist if let Ok(value) = serde_json::to_value(&*store) { plugin_store.set("hotkeys", value); let _ = plugin_store.save(); } }apps/desktop/src-tauri/src/lib.rs (1)
2143-2171: Start-recording handler: remove dbg!, preserve load errors, and add an active-recording guard
- Drop dbg!(RecordingSettingsStore::get(&app)) — it prints in production and the current .ok().flatten().unwrap_or_default() swallows load/deserialization errors; replace with a match that logs Err and falls back to default. Location: apps/desktop/src-tauri/src/lib.rs (around the start-recording handler).
- Confirmed: recording::StartRecordingInputs.capture_system_audio is a bool (apps/desktop/src-tauri/src/recording.rs, ~line 188) — the call passes a bool, so types match.
- Optional: early-return if a recording is already active/pending (check App state) to avoid races.
Apply the suggested change (still valid):
- let settings = dbg!(RecordingSettingsStore::get(&app)) - .ok() - .flatten() - .unwrap_or_default(); + let settings = match RecordingSettingsStore::get(&app) { + Ok(Some(s)) => s, + Ok(None) => RecordingSettingsStore::default(), + Err(e) => { + tracing::warn!("Failed to load RecordingSettingsStore: {e}"); + RecordingSettingsStore::default() + } + }; + + // Optional: avoid starting if already active/pending + // if app.state::<ArcLock<App>>().read().await.is_recording_active_or_pending() { + // return; + // }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src-tauri/src/camera.rs(1 hunks)apps/desktop/src-tauri/src/hotkeys.rs(3 hunks)apps/desktop/src-tauri/src/lib.rs(12 hunks)apps/desktop/src-tauri/src/recording.rs(1 hunks)apps/desktop/src-tauri/src/recording_settings.rs(1 hunks)apps/desktop/src-tauri/src/tray.rs(1 hunks)apps/desktop/src-tauri/src/windows.rs(5 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx(6 hunks)apps/desktop/src/store.ts(2 hunks)apps/desktop/src/utils/queries.ts(2 hunks)apps/desktop/src/utils/tauri.ts(5 hunks)crates/recording/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/store.tsapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsxapps/desktop/src/utils/queries.ts
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/store.tsapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsxapps/desktop/src/utils/queries.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/store.tsapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsxapps/desktop/src/utils/queries.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/camera.rsapps/desktop/src-tauri/src/tray.rsapps/desktop/src-tauri/src/recording_settings.rsapps/desktop/src-tauri/src/hotkeys.rsapps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/windows.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs: Rust code must be formatted with rustfmt and adhere to workspace clippy lints
Rust module files should use snake_case names
Files:
apps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/camera.rscrates/recording/src/lib.rsapps/desktop/src-tauri/src/tray.rsapps/desktop/src-tauri/src/recording_settings.rsapps/desktop/src-tauri/src/hotkeys.rsapps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/windows.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/{src,tests}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust tests should live in src (inline/unit) or tests (integration) directories
Files:
apps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/camera.rscrates/recording/src/lib.rsapps/desktop/src-tauri/src/tray.rsapps/desktop/src-tauri/src/recording_settings.rsapps/desktop/src-tauri/src/hotkeys.rsapps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/windows.rs
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file: tauri.ts
Do not edit auto-generated file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
**/queries.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated query bindings file: queries.ts
Do not edit auto-generated file: queries.ts
Files:
apps/desktop/src/utils/queries.ts
🧠 Learnings (2)
📚 Learning: 2025-08-25T10:58:06.142Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:58:06.142Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Applied to files:
apps/desktop/src/routes/(window-chrome)/(main).tsx
📚 Learning: 2025-08-25T10:58:06.142Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:58:06.142Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use tanstack/solid-query for server state in the desktop app
Applied to files:
apps/desktop/src/utils/queries.ts
🧬 Code graph analysis (11)
apps/desktop/src/routes/(window-chrome)/(main).tsx (2)
apps/desktop/src/utils/createEventListener.ts (1)
createTauriEventListener(30-41)apps/desktop/src/utils/tauri.ts (1)
events(271-315)
apps/desktop/src-tauri/src/recording.rs (1)
apps/desktop/src/utils/tauri.ts (1)
ShowCapWindow(443-443)
apps/desktop/src/store.ts (1)
apps/desktop/src/utils/tauri.ts (1)
RecordingSettingsStore(426-426)
crates/recording/src/lib.rs (1)
apps/desktop/src/utils/tauri.ts (1)
RecordingMode(424-424)
apps/desktop/src-tauri/src/tray.rs (1)
apps/desktop/src/utils/tauri.ts (1)
ShowCapWindow(443-443)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (2)
apps/desktop/src/utils/tauri.ts (1)
HotkeyAction(391-391)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)
apps/desktop/src-tauri/src/recording_settings.rs (1)
apps/desktop/src/utils/tauri.ts (5)
RecordingMode(424-424)DeviceOrModelID(366-366)ScreenCaptureTarget(438-438)RecordingTargetMode(429-429)RecordingSettingsStore(426-426)
apps/desktop/src-tauri/src/hotkeys.rs (1)
apps/desktop/src/utils/tauri.ts (6)
RequestOpenRecordingPicker(432-432)RequestStartRecording(434-434)RecordingTargetMode(429-429)ShowCapWindow(443-443)HotkeyAction(391-391)RecordingMode(424-424)
apps/desktop/src-tauri/src/lib.rs (3)
apps/desktop/src/utils/tauri.ts (11)
DisplayId(367-367)WindowId(462-462)LogicalBounds(396-396)RecordingSettingsStore(426-426)RecordingTargetMode(429-429)RequestStartRecording(434-434)RecordingMode(424-424)RequestOpenRecordingPicker(432-432)ShowCapWindow(443-443)StartRecordingInputs(445-445)ScreenCaptureTarget(438-438)apps/desktop/src-tauri/src/recording_settings.rs (1)
get(27-38)crates/scap-targets/src/lib.rs (3)
primary(20-22)id(28-30)id(118-120)
apps/desktop/src/utils/queries.ts (1)
apps/desktop/src/store.ts (1)
recordingSettingsStore(63-64)
apps/desktop/src-tauri/src/windows.rs (5)
apps/desktop/src/utils/tauri.ts (4)
RecordingTargetMode(429-429)DisplayId(367-367)Camera(339-339)ShowCapWindow(443-443)apps/desktop/src-tauri/src/permissions.rs (1)
do_permissions_check(147-190)apps/desktop/src-tauri/src/recording_settings.rs (1)
get(27-38)apps/desktop/src-tauri/src/general_settings.rs (1)
get(176-187)apps/desktop/src-tauri/src/platform/macos/mod.rs (1)
set_window_level(15-24)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
[failure] 123-123:
Property 'CAP' does not exist on type 'Window & typeof globalThis'.
⏰ 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)
🔇 Additional comments (27)
crates/recording/src/lib.rs (1)
22-26: Default RecordingMode → Studio: LGTM.This aligns with TS string union ("studio" | "instant") and serde rename policy.
apps/desktop/src-tauri/src/tray.rs (1)
105-109: Struct-like Main variant usage: LGTM.Matches the updated ShowCapWindow::Main { init_target_mode } shape.
apps/desktop/src/store.ts (1)
5-11: Type-only import + new recordingSettingsStore: LGTM.Good move to type-only import; store declaration fits existing pattern. Ensure any initial defaults/migration are handled when the key is absent.
Do we want to pre-populate recording_settings with sane defaults on first run to avoid null fan‑out?
Also applies to: 63-64
apps/desktop/src-tauri/src/windows.rs (2)
181-187: API change acknowledged: ShowCapWindow::Main now carries init_target_mode.TS bindings appear aligned (utils/tauri.ts exposes
{ Main: { init_target_mode: RecordingTargetMode | null } }). Ensure all call sites were updated.
727-728: ID mapping for Main variant updated correctly.No issues.
apps/desktop/src-tauri/src/recording_settings.rs (1)
40-50: Store update flow looks solid.
updatereads-modifies-writes atomically via plugin store; good defensive design.apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (2)
23-34: ACTION_TEXT additions look correct and type-safe.Keys align with
HotkeyActionviasatisfies. Good.
47-47: Correct: gating via generalSettingsStore query.This defers action list to the feature flag.
apps/desktop/src-tauri/src/hotkeys.rs (2)
50-62: Enums updated for new flow; serde(other) keeps backward compat.Good approach for deserializing deprecated actions.
132-176: Event emissions for picker/recording modes look correct.Emits
RequestStartRecording/RequestOpenRecordingPickerwith the right payloads; non-blocking and matches TS bindings.apps/desktop/src-tauri/src/lib.rs (13)
20-20: New recording_settings module wired in — good.
59-59: Importing Display to derive primary display default is appropriate.
88-93: Correct crate-scoped imports for settings/preview/start_recording.
252-252: Cloning camera_feed under a short-lived read lock is fine.
313-317: New RequestOpenRecordingPicker event looks good.
1903-1926: Events list updated to include request-open-recording-picker and on-escape-press — OK.
1932-1933: Exposing RecordingSettingsStore via specta is correct for typed IPC.
1985-1991: On second-instance invoke, showing Main with init_target_mode: None is consistent with the new flow.
2130-2134: Post-permissions startup shows Main with init_target_mode: None — consistent.
2172-2178: Picker handler correctly forwards target_mode into Main init — good.
2206-2213: Closing TargetSelectOverlay windows when Main is destroyed prevents leaks/strays — nice.
2339-2344: Reopen flow using new Main variant — OK.
305-309: RequestStartRecording payload aligned — no action required.
- Rust: apps/desktop/src-tauri/src/lib.rs defines RequestStartRecording { mode: Option }.
- TS: apps/desktop/src/utils/tauri.ts defines export type RequestStartRecording = { mode: RecordingMode | null }.
- Producers: apps/desktop/src-tauri/src/hotkeys.rs emits RequestStartRecording with Some(RecordingMode::…).
- Consumers: apps/desktop/src/routes/(window-chrome)/(main).tsx listens for the event (handler ignores payload).
- No tests or other emitters found assuming a different shape.
apps/desktop/src/utils/tauri.ts (4)
288-315: Events map includes requestOpenRecordingPicker — matches Rust side.
426-435: Approve — autogenerated file and types verified.
Header in apps/desktop/src/utils/tauri.ts shows it was generated by tauri-specta; RecordingSettingsStore, RecordingTargetMode, and RequestOpenRecordingPicker look consistent.
443-443: ShowCapWindow.Main now struct-like — update all callers.
Replace any showWindow('Main') / showWindow("Main") calls with the object variant: showWindow({ Main: { init_target_mode: RecordingTargetMode | null } }).
Automated sandbox search couldn't scan repo files; run a repo-wide search for showWindow('Main') locally and update all call sites.
445-446: StartRecordingInputs.mode is required — resolved. All direct commands.startRecording calls include mode (apps/desktop/src/routes/(window-chrome)/(main).tsx, apps/desktop/src/routes/target-select-overlay.tsx).
| onMount(async () => { | ||
| // We don't want the target select overlay on launch | ||
| setOptions({ targetMode: null }); | ||
| setOptions({ targetMode: window.__CAP__.initialTargetMode }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
❓ Verification inconclusive
Fix typecheck/runtime guard for window.CAP.initialTargetMode.
Static analysis is right: Window has no CAP. Guard with optional chaining and avoid any.
Apply this diff:
- // We don't want the target select overlay on launch
- setOptions({ targetMode: window.__CAP__.initialTargetMode });
+ // We don't want the target select overlay on launch
+ const initialTargetMode =
+ (window as typeof window & {
+ __CAP__?: { initialTargetMode?: "display" | "window" | "area" | null };
+ }).__CAP__?.initialTargetMode ?? null;
+ setOptions({ targetMode: initialTargetMode });Optionally, add a global ambient type to eliminate inline casting:
// apps/desktop/src/types/cap-bootstrap.d.ts
import type { RecordingTargetMode } from "~/utils/tauri";
declare global {
interface Window {
__CAP__?: { initialTargetMode?: RecordingTargetMode | null };
}
}
export {};This keeps strict TS without any and resolves the CI type error.
After applying, please re-run typecheck to ensure the error is gone.
Guard window.CAP access and add proper ambient type
Static analysis is correct — Window has no CAP. Use optional chaining and avoid any; apply the diff below and (optionally) add the ambient declaration. Re-run typecheck.
- // We don't want the target select overlay on launch
- setOptions({ targetMode: window.__CAP__.initialTargetMode });
+ // We don't want the target select overlay on launch
+ const initialTargetMode =
+ (window as typeof window & {
+ __CAP__?: { initialTargetMode?: "display" | "window" | "area" | null };
+ }).__CAP__?.initialTargetMode ?? null;
+ setOptions({ targetMode: initialTargetMode });Optional ambient type file to remove inline casting:
// apps/desktop/src/types/cap-bootstrap.d.ts
import type { RecordingTargetMode } from "~/utils/tauri";
declare global {
interface Window {
__CAP__?: { initialTargetMode?: RecordingTargetMode | null };
}
}
export {};🧰 Tools
🪛 GitHub Check: Typecheck
[failure] 123-123:
Property 'CAP' does not exist on type 'Window & typeof globalThis'.
🤖 Prompt for AI Agents
In apps/desktop/src/routes/(window-chrome)/new-main/index.tsx around lines 121
to 124, replace the direct access to window.__CAP__ with a guarded access using
optional chaining (e.g., window.__CAP__?.initialTargetMode) and remove any
inline casts to any; ensure setOptions receives the possibly undefined/null
value handled by your existing setter. Additionally, add the suggested ambient
type file (apps/desktop/src/types/cap-bootstrap.d.ts) declaring Window.__CAP__
as optional with initialTargetMode typed to RecordingTargetMode | null to avoid
inline casting, then re-run the typechecker.
| {idx !== actions().length - 1 && ( | ||
| <div class="w-full h-px bg-gray-3" /> |
There was a problem hiding this comment.
Bug: Index accessor must be called.
idx is an accessor; comparison should use idx(). As written, the separator renders between all rows.
Apply this diff:
- {idx !== actions().length - 1 && (
+ {idx() !== actions().length - 1 && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {idx !== actions().length - 1 && ( | |
| <div class="w-full h-px bg-gray-3" /> | |
| {idx() !== actions().length - 1 && ( | |
| <div class="w-full h-px bg-gray-3" /> |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx around lines
200-201, the code compares the accessor `idx` directly which always truthy;
change the comparison to call the accessor `idx()` (i.e., use `idx() !==
actions().length - 1`) so the separator only renders when the current index is
not the last item.
| } from "@tanstack/solid-query"; | ||
| import { getCurrentWindow } from "@tauri-apps/api/window"; | ||
| import { createMemo } from "solid-js"; | ||
| import { createEffect, createMemo } from "solid-js"; |
There was a problem hiding this comment.
❓ Verification inconclusive
Do not modify queries.ts per repo guidelines.
Per coding guidelines, files matching **/queries.ts are auto-generated and shouldn’t be edited. Please move these changes (store sync, effects, imports) into a non-generated module (e.g., a dedicated hook or utils file) and import from there.
If this particular queries.ts is not generated in this repo, please confirm and we can downgrade this.
Also applies to: 13-17
Do not modify auto-generated apps/desktop/src/utils/queries.ts — move logic to a non-generated module
apps/desktop/src/utils/queries.ts is auto-generated in this repo; edits will be overwritten and violate repo guidelines. Revert changes in that file and move store sync / createEffect / createMemo and any new imports into a dedicated non-generated module (e.g., apps/desktop/src/utils/query-hooks.ts) and import/use that from consumers.
🤖 Prompt for AI Agents
In apps/desktop/src/utils/queries.ts around line 10, the file is auto-generated
and must not be edited; the createEffect/createMemo/store sync logic and any new
imports you added should be reverted from this generated file and moved into a
new non-generated module (for example apps/desktop/src/utils/query-hooks.ts).
Create the new module, add the imports (createEffect, createMemo, store sync
logic) and implement the reactive hooks or exported helper functions there,
export the same API needed by consumers, then update consumer modules to import
from the new query-hooks module; leave apps/desktop/src/utils/queries.ts
unchanged in the repo so generated files remain unmodified.
| export type HapticPerformanceTime = "Default" | "Now" | "DrawCompleted" | ||
| export type Hotkey = { code: string; meta: boolean; ctrl: boolean; alt: boolean; shift: boolean } | ||
| export type HotkeyAction = "startRecording" | "stopRecording" | "restartRecording" | ||
| export type HotkeyAction = "startStudioRecording" | "startInstantRecording" | "stopRecording" | "restartRecording" | "openRecordingPicker" | "openRecordingPickerDisplay" | "openRecordingPickerWindow" | "openRecordingPickerArea" | "other" |
There was a problem hiding this comment.
🧩 Analysis chain
HotkeyAction union expanded — verify defaults/migrations.
Ensure HotkeysStore initializes new keys to sane defaults to avoid undefined accesses.
🏁 Script executed:
#!/bin/bash
rg -nP 'HotkeysStore|defaultHotkeys|hotkeys:\s*{\s*$' apps packages -C3Length of output: 5704
HotkeyAction union expanded — ensure sensible defaults & migrations.
- Rust: HotkeysStore::default() is derived (empty HashMap). Implement Default to populate all HotkeyAction keys with sane Hotkey values and/or add a migration that inserts any missing keys on load. (apps/desktop/src-tauri/src/hotkeys.rs)
- TS: Align types/handling — either make HotkeysStore.hotkeys a partial mapping or add runtime guards where keys are read so missing entries don't produce undefined accesses. (apps/desktop/src/utils/tauri.ts, apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx)
🤖 Prompt for AI Agents
apps/desktop/src/utils/tauri.ts around line 391: the HotkeyAction union was
expanded but there is no guarantee all keys exist at runtime; update handling so
TS and Rust stay in sync by (1) in Rust (apps/desktop/src-tauri/src/hotkeys.rs)
implement Default for HotkeysStore to populate every HotkeyAction key with
sensible defaults and/or add a migration step on load that inserts any missing
keys into the persisted map, and (2) in TypeScript either change
HotkeysStore.hotkeys to a Partial<Record<HotkeyAction, YourHotkeyType>> or add
runtime guards where hotkeys are read (fall back to default hotkey values or
noop) to avoid undefined accesses; ensure the same canonical set of keys is used
in both places so missing entries get seeded on load.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
121-124: Guard window.CAP and removeanyto prevent mount-time crash.Directly reading
(window as any).__CAP__.initialTargetModewill throw if__CAP__is absent and violates strict TS. Guard with optional chaining and keep types explicit.Apply this diff:
onMount(async () => { // We don't want the target select overlay on launch - setOptions({ targetMode: (window as any).__CAP__.initialTargetMode }); + const initialTargetMode = + (window as typeof window & { + __CAP__?: { initialTargetMode?: "display" | "window" | "area" | null }; + }).__CAP__?.initialTargetMode ?? null; + setOptions({ targetMode: initialTargetMode });Optionally add an ambient type to avoid inline casting and prefer shared types:
// apps/desktop/src/types/cap-bootstrap.d.ts import type { RecordingTargetMode } from "~/utils/tauri"; declare global { interface Window { __CAP__?: { initialTargetMode?: RecordingTargetMode | null }; } } export {};
🧹 Nitpick comments (1)
apps/desktop/src/routes/(window-chrome)/setup.tsx (1)
84-89: Await showWindow and handle errors; close only on success.Make
handleContinueasync for consistency with the Windows path and to avoid closing if showWindow fails.Apply this diff:
-const handleContinue = () => { - // Just proceed to the main window without saving mode to store - commands.showWindow({ Main: { init_target_mode: null } }).then(() => { - getCurrentWindow().close(); - }); -}; +const handleContinue = async () => { + // Just proceed to the main window without saving mode to store + try { + await commands.showWindow({ Main: { init_target_mode: null } }); + getCurrentWindow().close(); + } catch (err) { + console.error("Failed to open Main window:", err); + } +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/lib.rs(12 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/setup.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/setup.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/setup.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/setup.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/(window-chrome)/setup.tsx
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src/routes/(window-chrome)/setup.tsx (2)
92-178: UI flow and copy look good.Markup and state wiring for Permissions → Mode selection are coherent and match the new flow. No issues spotted.
506-508: Good: using typed ShowCapWindow payload.Switch to
commands.showWindow({ Main: { init_target_mode: null } })aligns with the generated Tauri types.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/utils/tauri.ts (1)
391-394: HotkeyAction union expanded — seed defaults/migrate store to avoid undefined accesses.Same concern as earlier review: widening the HotkeyAction set while HotkeysStore.hotkeys requires all keys risks missing entries at runtime.
Action:
- Rust: Implement Default for HotkeysStore to populate all actions and/or add a migration on load to insert missing keys.
- TS: If persistence can be partial, consider Partial<Record<HotkeyAction, Hotkey>> or guard reads with sensible fallbacks.
This mirrors the prior feedback.
🧹 Nitpick comments (8)
apps/desktop/src-tauri/src/lib.rs (3)
248-276: Avoid popping the Camera preview window when starting from a hotkey.Calling set_camera_input opens the Camera window (lines 262–267). When invoked by RequestStartRecording (lines 2153–2155), this can flash a preview UI right before capture, which is jarring and may interfere with the new quick‑start flow.
Suggestion: set the camera input directly on the actor in the hotkey path (no preview), keeping set_camera_input for interactive UI flows.
Apply this minimal change in the RequestStartRecording handler:
- let _ = set_mic_input(app.state(), settings.mic_name).await; - let _ = set_camera_input(app.clone(), app.state(), settings.camera_id).await; + let _ = set_mic_input(app.state(), settings.mic_name).await; + // Set camera without opening the preview window + if let Some(id) = settings.camera_id { + let camera_feed = app + .state::<ArcLock<App>>() + .read() + .await + .camera_feed + .clone(); + if let Ok(rx) = camera_feed.ask(feeds::camera::SetInput { id }).await { + let _ = rx.await; + } + } else { + let camera_feed = app + .state::<ArcLock<App>>() + .read() + .await + .camera_feed + .clone(); + let _ = camera_feed.ask(feeds::camera::RemoveInput).await; + }Also applies to: 2143-2171
2143-2171: Hotkey start is a no‑op if Main is open — confirm UX or respect “main window start behaviour.”Early return when Main exists (lines 2144–2146) means pressing StartStudio/Instant while the Main window is visible does nothing. If intentional, fine; otherwise consider:
- honoring GeneralSettingsStore.mainWindowRecordingStartBehaviour (minimise/close) then proceed to start, or
- starting anyway and hiding Main before capture.
This aligns hotkey semantics regardless of current window state.
2160-2167: Default capture target fallback is sane; add safety for stale/invalid targets.If a persisted window/area target is stale (closed window, unplugged display), start_recording may error. Consider validating settings.target before use and falling back to primary display when invalid.
apps/web/components/pages/HomePage/Header.tsx (5)
11-11: Remove unused ref (and its import).proArtRef isn’t used; clean it up to satisfy strict TS/Biome.
Apply:
-import { useRef, useState } from "react"; +import { useState } from "react"; @@ -import type { ProArtRef } from "./Pricing/ProArt"; @@ - const proArtRef = useRef<ProArtRef>(null); + // removed unused proArtRefAlso applies to: 21-21, 69-69
191-203: Make the Play control a keyboard‑accessible button.Clickable div is not keyboard/a11y friendly. Use motion.button with an aria‑label and focus styles.
Apply:
- <motion.div + <motion.button + type="button" + aria-label="Play demo video" whileTap={{ scale: 0.95 }} whileHover={{ scale: 1.05 }} onClick={() => setVideoToggled(true)} - className="size-[100px] md:size-[150px] inset-x-0 mx-auto top-[35vw] xs:top-[180px] sm:top-[35vw] xl:top-[350px] 2xl:top-[400px] xl:left-[-120px] relative cursor-pointer z-10 - shadow-[0px_60px_40px_3px_rgba(0,0,0,0.4)] flex items-center justify-center rounded-full bg-blue-500" + className="size-[100px] md:size-[150px] inset-x-0 mx-auto top-[35vw] xs:top-[180px] sm:top-[35vw] xl:top-[350px] 2xl:top-[400px] xl:left-[-120px] relative cursor-pointer z-10 shadow-[0px_60px_40px_3px_rgba(0,0,0,0.4)] flex items-center justify-center rounded-full bg-blue-500 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white/80" > <FontAwesomeIcon icon={faPlay} - className="text-white size-8 md:size-12" + aria-hidden="true" + className="text-white size-8 md:size-12" /> - </motion.div> + </motion.button>
90-94: Remove nonstandard prop on FontAwesomeIcon; add aria-hidden.fontWeight isn’t a valid prop here and “light” isn’t a valid CSS font-weight value. It’s decorative; hide from SR.
Apply:
- <FontAwesomeIcon - fontWeight="light" - className="w-1.5 text-gray-12 group-hover:translate-x-0.5 transition-transform" - icon={faAngleRight} - /> + <FontAwesomeIcon + className="w-1.5 text-gray-12 group-hover:translate-x-0.5 transition-transform" + icon={faAngleRight} + aria-hidden="true" + />
204-210: Clarify image alt text (or mark decorative).“App” is vague. Either describe the screenshot or mark decorative with empty alt.
Option A (descriptive):
- alt="App" + alt="Screenshot of the Cap app interface"Option B (decorative):
- alt="App" + alt=""
1-1: File naming guideline: kebab‑case.Project guideline prefers kebab‑case for TSX files. Consider renaming to header.tsx when convenient (follow‑up PR is fine).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src-tauri/src/hotkeys.rs(3 hunks)apps/desktop/src-tauri/src/lib.rs(12 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(4 hunks)apps/desktop/src/utils/tauri.ts(5 hunks)apps/web/components/pages/HomePage/Header.tsx(1 hunks)packages/ui-solid/src/auto-imports.d.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ui-solid/src/auto-imports.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/routes/(window-chrome)/(main).tsx
- apps/desktop/src-tauri/src/hotkeys.rs
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/components/pages/HomePage/Header.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/components/pages/HomePage/Header.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/web/components/pages/HomePage/Header.tsxapps/desktop/src/utils/tauri.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/web/components/pages/HomePage/Header.tsx
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file: tauri.ts
Do not edit auto-generated file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/utils/tauri.ts
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/utils/tauri.ts
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs: Rust code must be formatted with rustfmt and adhere to workspace clippy lints
Rust module files should use snake_case names
Files:
apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/{src,tests}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust tests should live in src (inline/unit) or tests (integration) directories
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (5)
apps/desktop/src/utils/tauri.ts (10)
DisplayId(367-367)WindowId(462-462)RecordingSettingsStore(426-426)RecordingTargetMode(429-429)RequestStartRecording(434-434)RecordingMode(424-424)RequestOpenRecordingPicker(432-432)ShowCapWindow(443-443)StartRecordingInputs(445-445)ScreenCaptureTarget(438-438)apps/desktop/src-tauri/src/recording.rs (1)
start_recording(207-529)apps/desktop/src-tauri/src/windows.rs (9)
app(211-211)app(340-340)app(430-430)app(730-730)app(919-919)get(145-148)get(918-920)id(724-750)from_str(54-86)apps/desktop/src-tauri/src/recording_settings.rs (1)
get(27-38)crates/scap-targets/src/lib.rs (5)
primary(20-22)id(28-30)id(118-120)from_str(73-75)from_str(217-219)
⏰ 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 (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/desktop/src-tauri/src/lib.rs (4)
20-20: Specta exports and type wiring look consistent; confirm serde field casing for settings.
- Good: module split (recording_settings), scap_targets imports, crate re-exports, and specta .typ::() ensure TS bindings are generated.
- Please confirm RecordingSettingsStore fields use serde rename_all = "camelCase" (or explicit renames) to match the generated TS type keys (micName, cameraId, systemAudio, target, mode). Mismatch will silently break persisted store reads/writes across Rust/TS.
Also applies to: 59-59, 88-93, 1903-1934
2172-2178: Recording picker wiring and Main init_target_mode: LGTM.RequestOpenRecordingPicker handler, startup, and macOS reopen flows cleanly pass init_target_mode (None or provided).
Also applies to: 2130-2134, 2339-2343
1913-1925: Event surface updates: LGTM; ensure frontend listens to on-escape-press if used for overlays.New events (request-open-recording-picker, on-escape-press) are exported and mapped. Verify UI bindings exist where expected.
2206-2213: Closing TargetSelectOverlay windows when Main closes: LGTM.Prevents orphaned overlay windows.
apps/desktop/src/utils/tauri.ts (2)
288-315: Event mapping includes requestOpenRecordingPicker: LGTM.Auto‑generated mapping correctly exposes "request-open-recording-picker".
426-435: New types (RecordingSettingsStore, RecordingTargetMode, ShowCapWindow.Main payload): LGTM; verify callers updated.
- Types align with Rust exports.
- Ensure all showWindow({ Main: { init_target_mode } }) call sites migrated from the old "Main" string variant; TS should catch most, but double‑check route/tray code.
Also applies to: 443-443
apps/web/components/pages/HomePage/Header.tsx (1)
21-21: Type-only import is correct.ProArtRef is exported as an interface in apps/web/components/pages/HomePage/Pricing/ProArt.tsx and no non-type (value) imports of ProArtRef were found.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/routes/(window-chrome)/settings/recordings.tsx (1)
177-183: Row click handler is defined but unused — the item isn’t clickable.props.onClick is passed by the parent but not wired to the row. Add click and keyboard handlers for accessibility.
- return ( - <li class="flex flex-row justify-between [&:not(:last-child)]:border-b [&:not(:last-child)]:pb-5 [&:not(:last-child)]:border-gray-3 items-center w-full transition-colors duration-200 hover:bg-gray-2"> + return ( + <li + class="flex flex-row justify-between [&:not(:last-child)]:border-b [&:not(:last-child)]:pb-5 [&:not(:last-child)]:border-gray-3 items-center w-full transition-colors duration-200 hover:bg-gray-2" + onClick={props.onClick} + role="button" + tabIndex={0} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + props.onClick(); + } + }} + >Also applies to: 192-193
♻️ Duplicate comments (1)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (1)
203-204: Bug: call theidxaccessor; otherwise the separator renders after every row.
idxis an accessor in Solid’s<Index>; useidx()in the comparison.- {idx !== actions().length - 1 && ( + {idx() !== actions().length - 1 && (
🧹 Nitpick comments (5)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (2)
80-95: Use createMemo for actions to avoid array churn and keep length checks stable.Recreating the array on every read can cause unnecessary reactivity work and inconsistent identity for
Index. Memoize it.Apply this diff:
- const actions = () => - [ + const actions = createMemo(() => + [ ...(generalSettings.data?.enableNewRecordingFlow ? (["openRecordingPicker"] as const) : (["startStudioRecording", "startInstantRecording"] as const)), "stopRecording", "restartRecording", ...(generalSettings.data?.enableNewRecordingFlow ? ([ "openRecordingPickerDisplay", "openRecordingPickerWindow", "openRecordingPickerArea", ] as const) : []), - ] satisfies Array<keyof typeof ACTION_TEXT>; + ] satisfies Array<keyof typeof ACTION_TEXT> + );Also add
createMemoto the Solid import:-import { - batch, - createEffect, - createResource, - createSignal, - For, - Index, - Match, - Show, - Switch, -} from "solid-js"; +import { + batch, + createEffect, + createMemo, + createResource, + createSignal, + For, + Index, + Match, + Show, + Switch, +} from "solid-js";
99-105: Clarify the user‑facing note.“Closed for shortcuts to work” is ambiguous; suggest “Restart the app for shortcuts to take effect.”
- <span class="font-medium text-gray-12"> - Note: for now the app needs to be closed for shortcuts to work. - </span> + <span class="font-medium text-gray-12"> + Note: Restart the app for shortcuts to take effect. + </span>apps/desktop/src/routes/(window-chrome)/settings/recordings.tsx (3)
150-156: Inline empty-state is a good UX addition; small polish for label and a11y.Use the tab’s display label (Instant/Studio) instead of the raw id and add basic a11y. Also prevent the overlay from intercepting clicks.
- <Show when={filteredRecordings().length === 0}> - <p class="text-center text-[--text-tertiary] absolute flex items-center justify-center w-full h-full"> - No {activeTab()} recordings - </p> - </Show> + <Show when={filteredRecordings().length === 0}> + <p + class="text-center text-[--text-tertiary] absolute flex items-center justify-center w-full h-full pointer-events-none" + role="status" + aria-live="polite" + > + No {Tabs.find((t) => t.id === activeTab())?.label ?? activeTab()} recordings + </p> + </Show>
289-294: refetchQueries called with QueryOptions object — pass filters or invalidate instead.queryClient.refetchQueries expects filters (e.g., { queryKey }) or a QueryKey, not a QueryOptions object. This can fail type-checking and/or behave unexpectedly.
- queryClient.refetchQueries(recordingsQuery); + await queryClient.invalidateQueries({ queryKey: recordingsQuery.queryKey });
184-186: Avoid reloading thumbnails on every reactive re-render.Using Date.now() in the src busts cache each render. Make it stable per mount.
- const [imageExists, setImageExists] = createSignal(true); + const [imageExists, setImageExists] = createSignal(true); + const [thumbKey] = createSignal(Date.now());- src={`${convertFileSrc( - props.recording.thumbnailPath, - )}?t=${Date.now()}`} + src={`${convertFileSrc(props.recording.thumbnailPath)}?t=${thumbKey()}`}Also applies to: 201-204
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx(5 hunks)apps/desktop/src/routes/(window-chrome)/settings/recordings.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/settings/recordings.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/(window-chrome)/settings/recordings.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/desktop/src/routes/(window-chrome)/settings/recordings.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/(window-chrome)/settings/recordings.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (2)
apps/desktop/src/utils/tauri.ts (1)
HotkeyAction(391-391)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (4)
14-14: LGTM: correct store import for feature‑flagged flow.Importing
generalSettingsStorealongsidehotkeysStoreis appropriate for gating the new recording shortcuts.
23-34: LGTM: action text map is well‑typed.Good use of
satisfies { [K in HotkeyAction]?: string }; covers new picker variants and keeps keys aligned withHotkeyAction.
47-47: LGTM: settings query wiring.Using
generalSettingsStore.createQuery()to drive the action list matches the feature‑flag intent.
108-108: LGTM:Indexiterates the computed actions list.This ties neatly to the feature‑flag logic. If you adopt
createMemo, keep usingactions()here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (1)
200-201: Bug: Index accessor must be called.
idxis an accessor; compare usingidx(). This otherwise renders separators after every row.- {idx !== actions().length - 1 && ( + {idx() !== actions().length - 1 && (
🧹 Nitpick comments (5)
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (1)
79-84: Use a valid scroll behavior; 'instant' is non-standard.The ScrollToOptions spec supports 'auto' and 'smooth' only. 'instant' will be ignored; use 'auto' (or remove the option).
- setTimeout( - () => window.scrollTo({ top: 0, behavior: "instant" }), - 5, - ); + requestAnimationFrame(() => window.scrollTo({ top: 0, behavior: "auto" }));Also applies to: 93-97
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
489-506: Harden URL parsing; new URL() will throw for invalid inputs.Wrap in try/catch and surface a user-friendly error; otherwise an invalid entry (e.g., without scheme) crashes the handler.
- onChange={async (v) => { - const url = new URL(v); - const origin = url.origin; + onChange={async (v) => { + let origin: string; + try { + origin = new URL(v).origin; + } catch { + await commands.globalMessageDialog("Invalid URL. Please include http(s)://"); + return; + } if (!(await confirm( `Are you sure you want to change the server URL to '${origin}'? You will need to sign in again.`, ))) return; await authStore.set(undefined); await commands.setServerUrl(origin); handleChange("serverUrl", origin); }}
421-478: Avoidanyin renderRecordingSelect; make it generic over the union.Calls here pass strictly typed unions; the helper uses
anyfor value/onChange, weakening type-safety.Proposed helper signature (apply outside this hunk where renderRecordingSelect is defined):
function renderRecordingSelect<T extends MainWindowRecordingStartBehaviour | PostStudioRecordingBehaviour | PostDeletionBehaviour | number>( label: string, description: string, getValue: () => T, onChange: (value: T) => void, options: { text: string; value: T }[], ) { /* ... */ }
429-431: Spelling consistency: “Minimise” vs “Minimize”.UI elsewhere uses US English; consider “Minimize” for consistency.
apps/desktop/src-tauri/src/lib.rs (1)
2143-2166: Don’t ignore fallible device and start calls; log or surface errors.Failures in set_mic_input, set_camera_input, or start_recording are silently dropped, making hotkey-driven starts hard to debug.
- let _ = set_mic_input(app.state(), settings.mic_name).await; - let _ = set_camera_input(app.clone(), app.state(), settings.camera_id).await; + if let Err(e) = set_mic_input(app.state(), settings.mic_name).await { + tracing::error!("set_mic_input failed: {e}"); + } + if let Err(e) = set_camera_input(app.clone(), app.state(), settings.camera_id).await { + tracing::error!("set_camera_input failed: {e}"); + } - let _ = start_recording(app.clone(), app.state(), recording::StartRecordingInputs { /* ... */ }).await; + if let Err(e) = start_recording(app.clone(), app.state(), recording::StartRecordingInputs { /* ... */ }).await { + tracing::error!("start_recording failed: {e}"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src-tauri/src/hotkeys.rs(3 hunks)apps/desktop/src-tauri/src/lib.rs(12 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(4 hunks)apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/routes/(window-chrome)/(main).tsx
- apps/desktop/src-tauri/src/hotkeys.rs
🧰 Additional context used
📓 Path-based instructions (7)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs: Rust code must be formatted with rustfmt and adhere to workspace clippy lints
Rust module files should use snake_case names
Files:
apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/{src,tests}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust tests should live in src (inline/unit) or tests (integration) directories
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (4)
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (1)
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (1)
ToggleSetting(24-40)
apps/desktop/src-tauri/src/lib.rs (4)
apps/desktop/src/utils/tauri.ts (9)
DisplayId(367-367)WindowId(462-462)RecordingSettingsStore(426-426)RecordingTargetMode(429-429)RequestStartRecording(434-434)RecordingMode(424-424)RequestOpenRecordingPicker(432-432)ShowCapWindow(443-443)ScreenCaptureTarget(438-438)apps/desktop/src-tauri/src/windows.rs (9)
app(211-211)app(340-340)app(430-430)app(730-730)app(919-919)get(145-148)get(918-920)id(724-750)from_str(54-86)apps/desktop/src-tauri/src/recording_settings.rs (1)
get(27-38)crates/scap-targets/src/lib.rs (5)
primary(20-22)id(28-30)id(118-120)from_str(73-75)from_str(217-219)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (2)
apps/desktop/src/store.ts (2)
generalSettingsStore(61-62)authStore(59-59)apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (1)
ToggleSetting(24-40)
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (2)
apps/desktop/src/utils/tauri.ts (1)
HotkeyAction(391-391)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (1)
43-45: Verify scroll container move doesn’t introduce nested/dual scrollbars.Shifting custom-scroll to the outer container can change overflow behavior vs. other settings routes. Please sanity-check mouse/trackpad scrolling and focus trapping across Settings.
apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (1)
80-95: LGTM: feature‑flagged action list.The dynamic actions() list cleanly switches between legacy and new flows and keeps the type surface aligned with ACTION_TEXT.
apps/desktop/src-tauri/src/lib.rs (2)
1986-1991: LGTM: migrated to struct-like ShowCapWindow::Main with init_target_mode.Consistent with windows.rs variant change; call sites updated correctly.
2202-2209: LGTM: cleanup of TargetSelectOverlay windows on Main destroy.Prevents orphan overlays; good resiliency improvement.
Current recording flow:
New recording flow
Summary by CodeRabbit
New Features
Improvements
Chores