-
Notifications
You must be signed in to change notification settings - Fork 904
Implement list view in the new recording flow #1104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds thumbnail capture/listing for displays and windows (PNG→base64), exposes a TAURI Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Frontend UI (TargetMenu)
participant Q as Queries (queries.ts)
participant Bridge as Tauri Bridge (tauri.ts)
participant Tauri as Tauri Commands
participant Rec as recording.rs
participant OS as OS Capture (scap-*/platform)
User->>UI: open target selection
UI->>Q: request list*WithThumbnails
Q->>Bridge: commands.list*WithThumbnails()
Bridge->>Tauri: invoke("list_*_with_thumbnails")
Tauri->>Rec: list_*_with_thumbnails()
Rec->>OS: capture frame (platform-specific)
OS-->>Rec: pixel buffer / frame
Rec-->>Tauri: [{ id, meta, thumbnail(base64) }, ...]
Tauri-->>Bridge: response
Bridge-->>Q: resolved data
Q-->>UI: render grid/cards with thumbnails
sequenceDiagram
autonumber
actor User
participant UI as Frontend UI
participant Bridge as Tauri Bridge
participant Tauri as Tauri Commands
participant Overlay as target_select_overlay.rs
participant OS as OS APIs
User->>UI: request focus for a window
UI->>Bridge: commands.focusWindow(windowId)
Bridge->>Tauri: invoke("focus_window",{windowId})
Tauri->>Overlay: focus_window(windowId)
Overlay->>OS: activate/restore + bring to foreground
OS-->>Overlay: success / failure
Overlay-->>Tauri: Ok / Err(String)
Tauri-->>Bridge: response
Bridge-->>UI: promise resolved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
58-93
: Keyboard navigation assumes a 2-column grid.The
handleKeyDown
function hardcodescolumns = 2
(line 60), which will break if the grid layout changes to a different column count (e.g., responsive breakpoints or a 3-column layout).Consider one of the following:
- Accept
columns
as a prop and pass it from the parent.- Dynamically calculate columns based on the grid's computed layout.
- Document the 2-column assumption if the layout is fixed.
If the 2-column layout is intentional and fixed, consider adding a data attribute or prop to make this constraint explicit.
apps/desktop/src/utils/queries.ts (1)
52-67
: Consider extracting the shared sorting logic.The sorting logic here (lines 57-60) is identical to the sorting in
listWindows
(lines 33-36). While this duplication is minor, extracting it into a shared helper function would improve maintainability.For example:
const sortCaptureWindows = <T extends { owner_name: string; name: string }>( windows: T[] ): T[] => { windows.sort( (a, b) => a.owner_name.localeCompare(b.owner_name) || a.name.localeCompare(b.name), ); return windows; };Then use:
return sortCaptureWindows(w);
in both query functions.apps/desktop/src-tauri/src/recording.rs (2)
773-846
: Consider parallelizing thumbnail capture for better performance.The collection functions capture thumbnails sequentially, which may introduce noticeable latency when enumerating many windows or displays. Consider using
futures::future::join_all
or similar to capture thumbnails concurrently.Example refactor for
collect_windows_with_thumbnails
:fn collect_windows_with_thumbnails() -> Result<Vec<CaptureWindowWithThumbnail>, String> { tauri::async_runtime::block_on(async move { let windows = screen_capture::list_windows(); debug!(window_count = windows.len(), "Collecting window thumbnails"); - let mut results = Vec::new(); - for (capture_window, window) in windows { - let thumbnail = capture_window_thumbnail(&window).await; + let futures = windows.into_iter().map(|(capture_window, window)| async move { + let thumbnail = capture_window_thumbnail(&window).await; let app_icon = window .app_icon() .and_then(|bytes| { if bytes.is_empty() { None } else { Some( base64::Engine::encode( &base64::engine::general_purpose::STANDARD, bytes, ), ) } }); if thumbnail.is_none() { warn!( window_id = ?capture_window.id, window_name = %capture_window.name, owner_name = %capture_window.owner_name, "Window thumbnail capture returned None", ); } else { debug!( window_id = ?capture_window.id, window_name = %capture_window.name, owner_name = %capture_window.owner_name, "Captured window thumbnail", ); } - results.push(CaptureWindowWithThumbnail { + CaptureWindowWithThumbnail { id: capture_window.id, name: capture_window.name, owner_name: capture_window.owner_name, bounds: capture_window.bounds, refresh_rate: capture_window.refresh_rate, thumbnail, app_icon, - }); - } + } + }); + let results = futures::future::join_all(futures).await; info!(windows = results.len(), "Collected window thumbnail data"); Ok(results) }) }
757-771
: Separate async and blocking contexts
Thecollect_displays_with_thumbnails
andcollect_windows_with_thumbnails
helpers usetauri::async_runtime::block_on(…)
internally, yet the Tauri commands also offload them viatokio::task::spawn_blocking
, resulting in redundant thread blocking. Either
- make the
collect_*
functionsasync fn
and await them directly in the commands (removingspawn_blocking
), or- keep them fully synchronous (dropping the inner
block_on
) and run them insidespawn_blocking
as pure blocking code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (13)
apps/desktop/src-tauri/Cargo.toml
(1 hunks)apps/desktop/src-tauri/src/lib.rs
(2 hunks)apps/desktop/src-tauri/src/recording.rs
(2 hunks)apps/desktop/src-tauri/src/target_select_overlay.rs
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
(9 hunks)apps/desktop/src/utils/queries.ts
(2 hunks)apps/desktop/src/utils/tauri.ts
(3 hunks)crates/scap-targets/src/platform/macos.rs
(2 hunks)packages/ui-solid/src/auto-imports.d.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/ui-solid/src/auto-imports.d.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src-tauri/src/lib.rs
apps/desktop/src/utils/tauri.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src-tauri/src/recording.rs
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src-tauri/src/target_select_overlay.rs
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src/utils/queries.ts
crates/scap-targets/src/platform/macos.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
packages/ui-solid/src/auto-imports.d.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/utils/tauri.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src/utils/queries.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:
packages/ui-solid/src/auto-imports.d.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/utils/tauri.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src/utils/queries.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}
: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/utils/tauri.ts
apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src/utils/queries.ts
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
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
apps/desktop/src-tauri/src/recording.rs
apps/desktop/src-tauri/src/target_select_overlay.rs
crates/scap-targets/src/platform/macos.rs
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file tauri.ts
Do not edit auto-generated files named
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 files named
queries.ts
.
Files:
apps/desktop/src/utils/queries.ts
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
For desktop IPC, use tauri_specta derive/macros on Rust types/events; do not hand-roll bindings
Files:
crates/scap-targets/src/platform/macos.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/
and/or a siblingtests/
directory for each crate insidecrates/*
.
Files:
crates/scap-targets/src/platform/macos.rs
🧬 Code graph analysis (7)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (2)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (2)
TargetCardSkeleton
(199-217)TargetCard
(42-194)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/recording.rs (2)
list_displays_with_thumbnails
(759-763)list_windows_with_thumbnails
(767-771)apps/desktop/src-tauri/src/target_select_overlay.rs (1)
focus_window
(145-181)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)
apps/desktop/src-tauri/src/recording.rs (5)
crates/scap-targets/src/platform/win.rs (10)
std
(967-967)display
(925-932)windows
(254-254)app_icon
(525-614)id
(125-125)id
(418-420)name
(214-251)name
(934-954)name
(946-951)owner_name
(437-477)crates/recording/src/sources/screen_capture/mod.rs (3)
display
(67-73)list_displays
(431-445)list_windows
(447-485)crates/camera/src/macos.rs (2)
sample_buf
(212-214)image_buf
(208-210)crates/scap-targets/src/platform/macos.rs (8)
display
(482-502)windows
(200-200)app_icon
(395-470)id
(49-49)id
(293-295)name
(128-147)name
(341-353)owner_name
(311-323)crates/scap-targets/src/lib.rs (7)
display
(146-148)app_icon
(138-140)id
(28-30)id
(118-120)name
(40-42)name
(150-152)owner_name
(134-136)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (4)
apps/desktop/src/utils/tauri.ts (7)
CaptureWindow
(364-364)CaptureDisplay
(362-362)CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)listDisplaysWithThumbnails
(41-43)listWindowsWithThumbnails
(44-46)commands
(7-278)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
TargetMenuGrid
(33-164)apps/desktop/src/utils/queries.ts (2)
listDisplaysWithThumbnails
(69-74)listWindowsWithThumbnails
(52-67)apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx (1)
TargetDropdownButton
(14-42)
apps/desktop/src-tauri/src/target_select_overlay.rs (3)
apps/desktop/src/utils/tauri.ts (1)
WindowId
(474-474)apps/desktop/src-tauri/src/lib.rs (4)
app
(1130-1131)app
(2237-2237)app
(2274-2274)app
(2280-2280)apps/desktop/src-tauri/src/windows.rs (5)
app
(211-211)app
(340-340)app
(430-430)app
(730-730)app
(919-919)
apps/desktop/src/utils/queries.ts (1)
apps/desktop/src/utils/tauri.ts (2)
listWindowsWithThumbnails
(44-46)listDisplaysWithThumbnails
(41-43)
🪛 GitHub Check: CodeQL
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
[failure] 197-197: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
[failure] 131-131:
Argument of type 'CaptureDisplayWithThumbnail' is not assignable to parameter of type 'CaptureWindowWithThumbnail'.
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
[failure] 17-17:
Type '"disabled"' is not assignable to type '"as" | "expanded" | Exclude<keyof ComponentProps, "as" | "expanded">'.
[failure] 17-17:
Type '"class"' is not assignable to type '"as" | "expanded" | Exclude<keyof ComponentProps, "as" | "expanded">'.
⏰ 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: Vercel Agent Review
- 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 (41)
crates/scap-targets/src/platform/macos.rs (2)
5-10
: LGTM!The
TCFType
import addition is necessary for theas_concrete_TypeRef()
method used in the newowner_pid
implementation.
325-339
: No import forkCGWindowOwnerPID
found incore_graphics
; string literal required
Ripgrep didn’t locate akCGWindowOwnerPID
constant in the codebase, so using the literal"kCGWindowOwnerPID"
here is necessary. If yourcore-graphics
dependency (per yourCargo.toml
) does expose this constant, import and use it for consistency with otherkCGWindow*
keys; otherwise, document this usage.apps/desktop/src-tauri/Cargo.toml (1)
92-93
: LGTM: OS-specific screen capture dependencies added.The addition of
scap-screencapturekit
andscap-direct3d
as local path dependencies aligns with the PR's thumbnail capture infrastructure. The naming follows Rust conventions (kebab-case for crate names).apps/desktop/src-tauri/src/lib.rs (2)
1859-1860
: LGTM: Thumbnail listing commands registered.The new
list_displays_with_thumbnails
andlist_windows_with_thumbnails
commands are properly registered and will be exposed to the frontend via the Tauri IPC boundary.
1927-1927
: LGTM: Window focus command registered.The
focus_window
command is properly registered and provides platform-specific window focusing behavior for macOS and Windows.apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx (1)
14-42
: Component implementation is well-structured.The component properly implements accessibility (aria-expanded), keyboard navigation (focus-visible), and visual states (expanded, disabled). The auto-imported icon follows the project's conventions.
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (8)
11-26
: LGTM! Utility functions are correct.The resolution and refresh rate formatting functions handle edge cases appropriately (zero/undefined values) and produce user-friendly output.
28-40
: LGTM! Type definitions are well-structured.The discriminated union correctly distinguishes between display and window variants, and properly extends button props while excluding
children
.
51-73
: LGTM! Memoized derivations are efficient.The memoized accessors correctly narrow types based on the variant discriminator, and the label/subtitle logic appropriately falls back to
owner_name
whenname
is unavailable.
77-94
: LGTM! Metadata formatting logic is sound.The conditional logic correctly formats resolution and refresh rate based on the variant, with appropriate fallback chaining.
96-106
: LGTM! Base64 data URI construction is correct.The thumbnail and app icon sources are correctly prefixed with the data URI scheme for inline image rendering.
110-129
: LGTM! Highlight logic is well-implemented.The search highlighting correctly splits text on the query pattern, applies styling to matches, and handles case-insensitive matching. The early returns optimize performance.
131-194
: LGTM! Component rendering is well-structured.The component correctly uses splitProps, renders conditional states (thumbnail fallback, app icon overlay), applies accessibility attributes, and handles disabled states. The gradient overlays and borders enhance the visual polish.
199-217
: LGTM! Skeleton component provides good loading UX.The skeleton closely mirrors the structure of the actual card, providing a consistent loading experience with appropriate pulse animation.
apps/desktop/src-tauri/src/target_select_overlay.rs (2)
148-166
: Verify that macOS activation is appropriate.The macOS implementation uses
ActivateIgnoringOtherApps
, which forcibly brings the application to the foreground. This can interrupt the user's current workflow if they've switched away from the Cap application. Ensure this aggressive activation behavior aligns with the intended UX.If the goal is to allow the user to focus a window for recording without interrupting their current task, consider using a less intrusive activation option or deferring activation until the user explicitly requests it.
168-178
: LGTM! Windows implementation is correct.The Windows implementation correctly restores minimized windows with
SW_RESTORE
before callingSetForegroundWindow
, which is the standard pattern for bringing a window to the foreground on Windows.apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (4)
9-31
: LGTM! Type definitions are well-structured.The discriminated union correctly separates display and window grid variants, and the base props cover all necessary configuration options.
34-40
: LGTM! Memoized state is efficient.The memoized items, skeleton count, and empty state detection are correctly implemented and will minimize unnecessary re-renders.
42-56
: LGTM! Ref management pattern is correct.The ref registration and focus management correctly rebuilds the ref array when items change, and the
focusAt
helper safely handles out-of-bounds access.
107-122
: LGTM! State-driven rendering is correct.The Switch correctly prioritizes error, loading, and empty states before rendering the populated grid.
apps/desktop/src/utils/queries.ts (2)
42-42
: Refetch interval disabled for listWindows.This change aligns with the new signature-based refetching strategy introduced in index.tsx (lines 431-471), which triggers updates only when the cheaper non-thumbnail list detects changes. This is a performance optimization to avoid expensive polling.
69-74
: Verify the refetchInterval asymmetry is intentional.
listDisplaysWithThumbnails
polls every 1000ms whilelistWindowsWithThumbnails
(line 66) hasrefetchInterval: false
. This suggests displays need real-time updates (e.g., for monitor connect/disconnect) while windows are managed via signature-based refetching in index.tsx. Confirm this asymmetry aligns with the intended UX behavior.apps/desktop/src/utils/tauri.ts (1)
1-539
: Auto-generated file - no manual review required.This file is auto-generated by tauri-specta (line 2) and should not be manually edited per coding guidelines. The new commands (
listDisplaysWithThumbnails
,listWindowsWithThumbnails
,focusWindow
) and types (CaptureDisplayWithThumbnail
,CaptureWindowWithThumbnail
) are generated from the Rust backend.apps/desktop/src/routes/(window-chrome)/new-main/TargetTypeButton.tsx (2)
1-18
: Strong TypeScript typing and Solid.js patterns.The type definition correctly extends
ComponentProps<"button">
to inherit native button props, and usessplitProps
to separate component-specific props from rest props. This follows Solid.js best practices.
20-42
: Verify text color for unselected state.Line 28 applies
text-gray-12
to both selected and unselected states:local.selected ? "text-gray-12" : "text-gray-12 hover:bg-gray-4"
. This appears unintentional - the unselected state might benefit from a lighter color liketext-gray-11
to provide visual differentiation from the selected state.If differentiation is needed, apply this diff:
- local.selected ? "text-gray-12" : "text-gray-12 hover:bg-gray-4", + local.selected ? "text-gray-12" : "text-gray-11 hover:bg-gray-4 hover:text-gray-12",apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (8)
89-126
: Signature generation utilities for change detection.These helper functions generate deterministic string signatures from window and display lists. They enable the signature-based refetch strategy (lines 454-471) that triggers expensive thumbnail refetches only when the cheaper non-thumbnail lists detect structural changes. The implementation correctly handles undefined inputs and includes all relevant identifying properties.
128-254
: TargetMenuPanel component with search and filtering.The component correctly uses discriminated union props to ensure type safety between display and window variants. The search functionality properly normalizes queries and filters targets. The Escape key handler (lines 215-218) clears search only when there's a value, which is good UX.
301-354
: Lazy query loading for performance optimization.The queries for thumbnail data are conditionally enabled only after the respective menu has been opened for the first time (lines 316, 322). This defers expensive thumbnail generation until needed, improving initial load performance.
366-378
: Verify the 100ms delay after focusWindow is sufficient.Line 376 introduces a 100ms delay after calling
commands.focusWindow()
before settingtargetMode
. This delay likely allows the window to receive focus before the overlay appears. Confirm this timing is adequate across all supported platforms (macOS, Windows) and under various system load conditions.
380-384
: Menu closure during recording.This effect correctly closes both menus when recording starts, preventing user confusion and ensuring a clean recording state.
576-700
: Well-structured UI components with accessibility support.The extracted
BaseControls
andTargetSelectionHome
components improve code organization. Proper ARIA attributes (aria-haspopup
,aria-label
,aria-pressed
) are used throughout, and focus ring styling provides clear visual feedback. The ref assignments (lines 628, 670) enable focus management after menu selection.
822-858
: Transition animation for menu panels.The
Transition
component from solid-transition-group provides smooth enter/exit animations. Thekeyed
prop (line 827) ensures proper transitions when switching between display and window menu variants. The animation classes use Tailwind's built-in utilities for consistency with the design system.Based on learnings: solid-transition-group 0.2.x (used here via ^0.2.3) provides CSS-class-based transitions; ensure the animation classes (
animate-in
,fade-in
, etc.) are available in your Tailwind configuration.
431-471
: Approve signature-based thumbnail refetch strategy. Global search confirmed these status/fetchStatus checks exist only here and the guard logic is comprehensive.apps/desktop/src-tauri/src/recording.rs (8)
187-204
: LGTM!The new thumbnail-enabled structs align with the TypeScript types and correctly use
Option<String>
for base64-encoded data.
292-342
: LGTM!The 32-bit pixel format conversion logic correctly validates bounds, uses
checked_mul
to prevent overflow, and handles BGRA/RGBA/ARGB/ABGR permutations accurately.
352-439
: LGTM!The NV12 to RGBA conversion correctly handles 4:2:0 chroma subsampling, validates all plane bounds, and applies proper indexing for interleaved UV data.
441-462
: LGTM!The YCbCr to RGB conversion correctly applies ITU-R BT.601 coefficients and handles both Video and Full range color spaces with appropriate offsets and scaling.
464-508
: LGTM!The RAII
PixelBufferLock
wrapper correctly ensures the pixel buffer is unlocked on drop, preventing resource leaks.
580-663
: LGTM with note on dimension inconsistency.The Windows display thumbnail capture correctly uses Direct3D, validates frame buffer bounds, and resizes with high-quality filtering. Note the 320-width thumbnails here differ from the 200-width macOS thumbnails.
665-755
: LGTM!The window thumbnail capture correctly isolates the target window (macOS uses
desktop_independent_window
, Windows uses window-specific capture item) and applies appropriate dimensions (200 width for windows vs 320 for displays on Windows).
510-572
: Verify externalcidre
crate for existing Core Video wrappers
I found no local definitions of these FFI methods in this repo; ensurecidre::cv::PixelBuf
doesn’t already exposebase_address
,bytes_per_row
,base_address_of_plane
,bytes_per_row_of_plane
, orheight_of_plane
to avoid duplicating functionality.
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
Show resolved
Hide resolved
// biome-ignore lint: disable | ||
export {} | ||
declare global { | ||
const IconCapArrowLeft: typeof import('~icons/cap/arrow-left.jsx')['default'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the source icon file exists.
The new IconCapArrowLeft
declaration follows the correct pattern for auto-generated icon imports. Ensure that the source icon file at ~icons/cap/arrow-left.jsx
exists or will be generated by unplugin-icons when the build runs.
Run the following script to verify the icon source:
🏁 Script executed:
#!/bin/bash
fd -e svg -e jsx "arrow-left" --type f
Length of output: 45
Missing icon file for IconCapArrowLeft
The generated import ~icons/cap/arrow-left.jsx
isn’t backed by any file—only arrow-left-circle.svg
exists under packages/ui-solid/icons
. Add the missing arrow-left.svg
(or its .jsx
proxy) or update the icon reference to match an existing asset.
🤖 Prompt for AI Agents
In packages/ui-solid/src/auto-imports.d.ts around line 9, the generated type for
IconCapArrowLeft references ~icons/cap/arrow-left.jsx which does not exist (only
arrow-left-circle.svg is present); fix by either adding the missing icon file
(arrow-left.svg) or its JSX proxy under packages/ui-solid/icons (and regenerate
the imports), or update the reference to point to the existing asset (e.g.,
change the import to arrow-left-circle.jsx / rename the identifier to
IconCapArrowLeftCircle) and regenerate the auto-imports so the declaration
matches an actual file.
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…Software/cap into new-recording-flow-list-view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src-tauri/src/recording.rs
(2 hunks)apps/desktop/src-tauri/src/target_select_overlay.rs
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
- apps/desktop/src-tauri/src/target_select_overlay.rs
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}
: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src-tauri/src/recording.rs
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.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/TargetDropdownButton.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
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/recording.rs
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/recording.rs (5)
apps/desktop/src-tauri/src/lib.rs (7)
std
(1438-1456)std
(1480-1502)tauri_specta
(1846-1959)new
(348-350)new
(767-771)new
(1400-1408)None
(2489-2489)crates/scap-targets/src/platform/win.rs (10)
std
(967-967)windows
(254-254)display
(925-932)app_icon
(525-614)id
(125-125)id
(418-420)name
(214-251)name
(934-954)name
(946-951)owner_name
(437-477)crates/recording/src/sources/screen_capture/mod.rs (5)
info
(422-424)config
(418-420)display
(67-73)list_displays
(431-445)list_windows
(447-485)apps/desktop/src/utils/tauri.ts (5)
CaptureDisplayWithThumbnail
(363-363)DisplayId
(379-379)CaptureWindowWithThumbnail
(365-365)WindowId
(474-474)LogicalBounds
(408-408)crates/scap-targets/src/platform/macos.rs (8)
windows
(200-200)display
(482-502)app_icon
(395-470)id
(49-49)id
(293-295)name
(128-147)name
(341-353)owner_name
(311-323)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (2)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (2)
TargetCardSkeleton
(199-217)TargetCard
(42-194)
⏰ 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: Vercel Agent Review
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
Outdated
Show resolved
Hide resolved
…Software/Cap into new-recording-flow-list-view
…Software/cap into new-recording-flow-list-view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
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/editor/ui.tsx (1)
149-159
: Swap class order incx()
to allow user overrides.Placing
props.class
before the base classes prevents users from overriding default styles. Thecx
utility applies classes in order, and later classes take precedence in CSS specificity conflicts.Apply this diff to fix the class order:
export function Input(props: ComponentProps<"input">) { return ( <TextInput {...props} class={cx( - props.class, "rounded-[0.5rem] bg-gray-2 hover:ring-1 py-[18px] hover:ring-gray-5 h-[2rem] font-normal placeholder:text-black-transparent-40 text-xs caret-gray-500 transition-shadow duration-200 focus:ring-offset-1 focus:bg-gray-3 focus:ring-offset-gray-100 focus:ring-1 focus:ring-gray-10 px-[0.5rem] w-full text-[0.875rem] outline-none text-gray-12", + props.class, )} /> ); }
♻️ Duplicate comments (3)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (2)
135-146
: Blocker:onSelect
type not narrowed inside variant branches (TS error).TypeScript still sees
props.onSelect
as the union version; passingitem
causes the failure flagged by the checker. Narrowprops
per branch before invokingonSelect
.Apply one of these minimal fixes (A keeps current structure):
@@ - <TargetCard + {(() => { + const p = props as DisplayGridProps; + return ( + <TargetCard variant="display" target={item} - onClick={() => props.onSelect?.(item)} + onClick={() => p.onSelect?.(item)} disabled={props.disabled} ref={registerRef(index())} onKeyDown={(event) => handleKeyDown(event, index())} role="option" class="w-full" highlightQuery={props.highlightQuery} - /> + /> + ); + })()} @@ - <TargetCard + {(() => { + const p = props as WindowGridProps; + return ( + <TargetCard variant="window" target={item} - onClick={() => props.onSelect?.(item)} + onClick={() => p.onSelect?.(item)} disabled={props.disabled} ref={registerRef(index())} onKeyDown={(event) => handleKeyDown(event, index())} role="option" class="w-full" highlightQuery={props.highlightQuery} - /> + /> + ); + })()}Alternatively, split into
<DisplayGrid>
/<WindowGrid>
subcomponents to avoid casts.Also applies to: 159-170
45-48
: Keyboard nav breaks after list updates:cardRefs
reset loses refs.Clearing
cardRefs
on everyitems()
change can leave it empty when DOM nodes are reused; arrows then stop working.Quick fix: stop clearing the array.
createEffect(() => { - items(); - cardRefs = []; + items(); // do not clear cardRefs; keep existing refs });Recommended fix (more robust): derive buttons on demand from the container instead of storing refs (see next comment).
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
371-381
: Handle tauri command failures to avoid unhandled promise rejections.Wrap
focusWindow
/showWindow
calls in try/catch so clicks don’t produce unhandled rejections when windows are unavailable.Patches:
@@ - await commands.focusWindow(target.id); + try { + await commands.focusWindow(target.id); + } catch (error) { + console.warn("Failed to focus window:", error); + }@@ - await commands.showWindow({ Settings: { page: "general" } }); - getCurrentWindow().hide(); + try { + await commands.showWindow({ Settings: { page: "general" } }); + getCurrentWindow().hide(); + } catch (error) { + console.error("Failed to show settings window:", error); + }@@ - await commands.showWindow({ Settings: { page: "recordings" } }); - getCurrentWindow().hide(); + try { + await commands.showWindow({ Settings: { page: "recordings" } }); + getCurrentWindow().hide(); + } catch (error) { + console.error("Failed to show recordings window:", error); + }@@ - if (license.data?.type !== "pro") { - await commands.showWindow("Upgrade"); - } + if (license.data?.type !== "pro") { + try { + await commands.showWindow("Upgrade"); + } catch (error) { + console.error("Failed to show upgrade window:", error); + } + }Also applies to: 748-750, 760-764, 789-797
🧹 Nitpick comments (9)
.claude/settings.local.json (2)
3-9
: Tighten command allowlist; move heavy builds to ask, add tests to allow.Allowing
Bash(pnpm build:*)
with a wildcard is broad and can trigger heavy or release-adjacent tasks. Keep fast, safe checks in allow; gate builds behind ask. Also add tests to encourage pre-merge runs.Proposed diff:
"permissions": { "allow": [ "Bash(pnpm typecheck:*)", - "Bash(pnpm lint:*)", - "Bash(pnpm build:*)" + "Bash(pnpm lint:*)", + "Bash(pnpm test:*)" ], - "deny": [], - "ask": [] + "deny": [], + "ask": [ + "Bash(pnpm build:*)" + ] }Please confirm the exact script names used in this repo (e.g.,
typecheck:web
,lint:all
,test:unit
) so the patterns match your workspace.
1-11
: Is a “local” settings file meant to be committed?Filename suggests developer-local config. If this is advisory, consider committing a
.claude/settings.template.json
and git‑ignoringsettings.local.json
. If it’s intentionally shared, add a brief README note so teammates understand scope.apps/desktop/package.json (1)
62-62
: Approve solid-motionone addition; consider consolidating animation libraries and version alignment
- solid-motionone ^1.0.4 is in active use (e.g. new-main/index.tsx, TargetMenuGrid.tsx) alongside solid-presence and solid-transition-group.
- If Motion One covers your entry/exit patterns, plan to migrate off solid-presence (in in-progress-recording.tsx) and solid-transition-group (in various routes/components) to reduce bundle/runtime surface.
- Lockfile shows two solid-presence versions (0.1.8 & 0.2.0); bump to ^0.2.0 in package.json to dedupe.
- If retaining TransitionGroup flows, check for solid-transition-group v0.3.x for improved Solid 1.9 compatibility and upgrade as needed.
apps/desktop/src/routes/target-select-overlay.tsx (3)
99-107
: Icon query: re-valid after accessor fix; consider stabilizing key.After fixing windowToShow(), this is correct. Optional: narrow the key to a stable scalar to avoid churn (e.g., window id only).
67-89
: Query key breadth and work: prefer stable scalars in key.Using the whole captureTarget object may over‑invalidate. Key by variant/id instead.
- const selectedWindow = createQuery(() => ({ - queryKey: ["selectedWindow", rawOptions.captureTarget], + const selectedWindow = createQuery(() => ({ + queryKey: [ + "selectedWindow", + rawOptions.captureTarget.variant === "window" + ? rawOptions.captureTarget.id + : null, + rawOptions.targetMode, + ],
357-363
: Remove inline comments per repo guidelines.Inline comments were added on changed lines; code should be self‑explanatory here.
As per coding guidelines- x: moveEvent.clientX - downEvent.clientX, // Remove Math.max constraint - y: moveEvent.clientY - downEvent.clientY, // Remove Math.max constraint + x: moveEvent.clientX - downEvent.clientX, + y: moveEvent.clientY - downEvent.clientY,apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
131-142
: Optional a11y: add aria-label to the button.Use the computed label for screen readers.
- <button + <button + aria-label={label() ?? (local.variant === "display" ? "Display" : "Window")}apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
1-1
: Filename casing: switch to kebab-case for modules.Per guidelines, TS/TSX module filenames should be kebab-case. Consider renaming this file to
target-menu-grid.tsx
and updating imports.apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
392-394
: Avoidany
onwindow
; add a typed global instead.Replace
(window as any).__CAP__
with a typed declaration to keep strict TS.Add a global type (new file):
// apps/desktop/src/types/global.d.ts declare global { interface Window { __CAP__: { initialTargetMode: "display" | "window" | "area" | null; }; } } export {};Then here:
- setOptions({ targetMode: (window as any).__CAP__.initialTargetMode }); + setOptions({ targetMode: window.__CAP__.initialTargetMode });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.claude/settings.local.json
(1 hunks)apps/desktop/package.json
(1 hunks)apps/desktop/src-tauri/src/windows.rs
(1 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
(9 hunks)apps/desktop/src/routes/editor/ui.tsx
(1 hunks)apps/desktop/src/routes/target-select-overlay.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/windows.rs
apps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/(main).tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/editor/ui.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
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/windows.rs
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}
: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/(main).tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/editor/ui.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
apps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/(main).tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/editor/ui.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.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/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/(main).tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/editor/ui.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
🧬 Code graph analysis (4)
apps/desktop/src/routes/target-select-overlay.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
commands
(7-278)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (2)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (2)
TargetCardSkeleton
(199-217)TargetCard
(42-194)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (5)
apps/desktop/src/utils/tauri.ts (7)
CaptureWindow
(364-364)CaptureDisplay
(362-362)CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)listDisplaysWithThumbnails
(41-43)listWindowsWithThumbnails
(44-46)commands
(7-278)apps/desktop/src/routes/editor/ui.tsx (1)
Input
(149-159)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
TargetMenuGrid
(34-179)apps/desktop/src/utils/queries.ts (2)
listDisplaysWithThumbnails
(69-74)listWindowsWithThumbnails
(52-67)apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx (1)
TargetDropdownButton
(16-44)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
[failure] 138-138:
Argument of type 'CaptureDisplayWithThumbnail' is not assignable to parameter of type 'CaptureWindowWithThumbnail'.
⏰ 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: Vercel Agent Review
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
.claude/settings.local.json (1)
1-11
: JSON looks valid and minimal.Structure and values parse fine; no secrets or env-specific paths present. Good baseline.
apps/desktop/src-tauri/src/windows.rs (1)
260-260
: Document and confirm disabling content protection for Main window.Only the Main window initialization at line 260 uses
.content_protected(false)
, while all other windows remain protected (true
on lines 293, 520, 544, 601, 631). Please confirm this exception is intentional, ensure the Main window contains no sensitive data, and add a comment or feature flag if it’s limited to dev/demo builds.apps/desktop/src/routes/target-select-overlay.tsx (1)
222-222
: Show binding relies on windowToShow() returning an object.This is fine once the accessor fix lands; please verify no undefined passes through.
apps/desktop/src/routes/(window-chrome)/(main).tsx (1)
348-348
: LGTM: minor spacing tweak only.
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (2)
8-9
: Remove manual icon imports to follow desktop app convention.Per the desktop guidelines, icons must be auto-imported instead of pulled in manually. Please drop these explicit imports and rely on the auto-import pipeline so future icon updates keep working.
196-198
: Hyphen in the escape regex is creating a range.Placing
-
where it is turns[\-^
into a range from\
to^
, so characters like]
slip through unescaped. Move the hyphen to the end (or escape it) to keep the character class literal, e.g.:- return value.replace(/[\\-^$*+?.()|[\]{}]/g, "\\$&"); + return value.replace(/[\\^$*+?.()|[\]{}-]/g, "\\$&");apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
104-176
: Drop the improvised listbox roles.The container is declared
role="listbox"
while each option is still a native<button>
, which is invalid ARIA (buttons can’t takerole="option"
and the keyboard model is different). Either build the full listbox pattern or keep native semantics: remove the listbox/option roles and let the grid act as a plain button group (role="group"
if you need one).apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
57-61
: Remove manual icon imports; rely on auto-imported icons.These manual
~icons/
imports violate the desktop app's coding guidelines.As per coding guidelines.
🧹 Nitpick comments (5)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (5)
196-204
: Use button element for clickable back control.The back control uses a
div
withonClick
, which is not keyboard-accessible by default and lacks semantic meaning.Apply this diff:
- <div + <button + type="button" onClick={() => props.onBack()} class="flex gap-1 items-center rounded-md px-1.5 text-xs text-gray-11 transition-opacity hover:opacity-70 hover:text-gray-12 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-9 focus-visible:ring-offset-2 focus-visible:ring-offset-gray-1" > <IconLucideArrowLeft class="size-3 text-gray-11" /> <span class="font-medium text-gray-12">Back</span> - </div> + </button>
751-754
: Add error handling for async showWindow command.The
showWindow
command can fail (e.g., if window creation fails), leading to unhandled promise rejections.Apply this diff:
<button type="button" onClick={async () => { - await commands.showWindow({ Settings: { page: "general" } }); - getCurrentWindow().hide(); + try { + await commands.showWindow({ Settings: { page: "general" } }); + getCurrentWindow().hide(); + } catch (error) { + console.error("Failed to show settings window:", error); + } }} class="flex items-center justify-center size-5 -ml-[1.5px]" >
763-767
: Add error handling for async showWindow command.The
showWindow
command can fail (e.g., if window creation fails), leading to unhandled promise rejections.Apply this diff:
<button type="button" onClick={async () => { - await commands.showWindow({ - Settings: { page: "recordings" }, - }); - getCurrentWindow().hide(); + try { + await commands.showWindow({ + Settings: { page: "recordings" }, + }); + getCurrentWindow().hide(); + } catch (error) { + console.error("Failed to show recordings window:", error); + } }} class="flex justify-center items-center size-5" >
793-796
: Add error handling for async showWindow command.The
showWindow
command can fail (e.g., if window creation fails), leading to unhandled promise rejections.Apply this diff:
<span onClick={async () => { if (license.data?.type !== "pro") { - await commands.showWindow("Upgrade"); + try { + await commands.showWindow("Upgrade"); + } catch (error) { + console.error("Failed to show upgrade window:", error); + } } }} class={cx(
31-31
: Remove unused Transition import.The
Transition
import fromsolid-transition-group
is not used in this file.Apply this diff:
import { Motion, Presence } from "solid-motionone"; -import { Transition } from "solid-transition-group"; import Tooltip from "~/components/Tooltip";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src-tauri/src/recording.rs
(2 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
(9 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}
: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src-tauri/src/recording.rs
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.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/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
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/recording.rs
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:17:47.407Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.407Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Do not manually import icons in the desktop app; rely on auto-imported icons
Applied to files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
🧬 Code graph analysis (4)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (2)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (2)
TargetCardSkeleton
(199-217)TargetCard
(42-194)
apps/desktop/src-tauri/src/recording.rs (5)
crates/scap-targets/src/platform/win.rs (11)
std
(967-967)windows
(254-254)windows
(1031-1031)display
(925-932)app_icon
(525-614)id
(125-125)id
(418-420)name
(214-251)name
(934-954)name
(946-951)owner_name
(437-477)crates/recording/src/sources/screen_capture/mod.rs (3)
display
(67-73)list_displays
(431-445)list_windows
(447-485)apps/desktop/src/utils/tauri.ts (5)
CaptureDisplayWithThumbnail
(363-363)DisplayId
(379-379)CaptureWindowWithThumbnail
(365-365)WindowId
(474-474)LogicalBounds
(408-408)crates/scap-targets/src/platform/macos.rs (8)
windows
(200-200)display
(482-502)app_icon
(395-470)id
(49-49)id
(293-295)name
(128-147)name
(341-353)owner_name
(311-323)crates/scap-targets/src/lib.rs (7)
display
(146-148)app_icon
(138-140)id
(28-30)id
(118-120)name
(40-42)name
(150-152)owner_name
(134-136)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (5)
apps/desktop/src/utils/tauri.ts (7)
CaptureWindow
(364-364)CaptureDisplay
(362-362)CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)listDisplaysWithThumbnails
(41-43)listWindowsWithThumbnails
(44-46)commands
(7-278)apps/desktop/src/routes/editor/ui.tsx (1)
Input
(149-159)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
TargetMenuGrid
(34-183)apps/desktop/src/utils/queries.ts (2)
listDisplaysWithThumbnails
(69-74)listWindowsWithThumbnails
(52-67)apps/desktop/src/routes/(window-chrome)/new-main/TargetDropdownButton.tsx (1)
TargetDropdownButton
(16-44)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
[failure] 142-142:
Argument of type 'CaptureDisplayWithThumbnail' is not assignable to parameter of type 'CaptureWindowWithThumbnail'.
⏰ 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: Vercel Agent Review
- GitHub Check: Analyze (rust)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
Outdated
Show resolved
Hide resolved
…Software/cap into new-recording-flow-list-view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/desktop/src/routes/target-select-overlay.tsx (2)
90-101
: Fix accessor misuse and redundant fallback.
selectedWindow.data
is an accessor (must be called), and thetypeof
check causes a TypeScript error. The final fallback tohoveredWindow
is redundant since it's already returned on line 92.Apply this diff:
const windowToShow = () => { - const hoveredWindow = targetUnderCursor.window; - if (hoveredWindow) return hoveredWindow; + if (targetUnderCursor.window) return targetUnderCursor.window; if (rawOptions.captureTarget.variant === "window") { - const selected = - typeof selectedWindow.data === "function" - ? selectedWindow.data() - : selectedWindow.data; - if (selected) return selected; + const data = selectedWindow.data?.(); + if (data) return data; } - return hoveredWindow; + return null; };
220-224
: Fix condition to call the accessor.The condition checks
selectedWindow.data
(the function) instead of calling it to get the value. This will always evaluate totrue
even when the data isnull
.Apply this diff:
when={ rawOptions.targetMode === "window" && (targetUnderCursor.display_id === params.displayId || (rawOptions.captureTarget.variant === "window" && - selectedWindow.data)) + !!selectedWindow.data?.())) }apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
8-9
: Remove manual icon imports from desktop routes.Desktop targets rely on the auto-import pipeline for icons; bringing them in manually breaks the agreed tooling flow. Drop these imports and keep using the components so auto-import resolves them. As per coding guidelines
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (1)
143-167
: Narrowprops.variant
before invokingonSelect
.Typecheck is still red because
props.onSelect
is a union; inside these branches it hasn’t been narrowed, so we end up calling the window overload with a display payload (and vice versa). Guard the calls with variant-aware helpers so TypeScript sees the correct signature.export default function TargetMenuGrid(props: TargetMenuGridProps) { + const handleDisplaySelect = (target: CaptureDisplayWithThumbnail) => { + if (props.variant !== "display") return; + props.onSelect?.(target); + }; + + const handleWindowSelect = (target: CaptureWindowWithThumbnail) => { + if (props.variant !== "window") return; + props.onSelect?.(target); + }; @@ <TargetCard variant="display" target={item} - onClick={() => props.onSelect?.(item)} + onClick={() => handleDisplaySelect(item)} @@ <TargetCard variant="window" target={item} - onClick={() => props.onSelect?.(item)} + onClick={() => handleWindowSelect(item)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
(1 hunks)apps/desktop/src/routes/target-select-overlay.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}
: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
apps/desktop/src/routes/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.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/target-select-overlay.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:17:47.407Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.407Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Do not manually import icons in the desktop app; rely on auto-imported icons
Applied to files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
🧬 Code graph analysis (3)
apps/desktop/src/routes/target-select-overlay.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
commands
(7-278)
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx (2)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (2)
TargetCardSkeleton
(199-217)TargetCard
(42-194)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/target-select-overlay.tsx
[failure] 96-96:
Property 'data' does not exist on type 'never'.
apps/desktop/src/routes/(window-chrome)/new-main/TargetMenuGrid.tsx
[failure] 143-143:
Argument of type 'CaptureDisplayWithThumbnail' is not assignable to parameter of type 'CaptureWindowWithThumbnail'.
⏰ 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). (2)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/desktop/src/routes/target-select-overlay.tsx (3)
67-88
: LGTM!The
selectedWindow
query is well-structured: it fetches window details only whencaptureTarget.variant
is"window"
andtargetMode
is"window"
, maps to a simplified object, and caches for 5 seconds.
103-112
: LGTM, oncewindowToShow
is fixed.The
windowIcon
query correctly keys onwindowToShow()?.id
, fetches the icon when that ID exists, and caches for 5 minutes. This will work as intended once thewindowToShow
accessor issue is resolved.
227-289
: LGTM, oncewindowToShow
is fixed.The
Show
block correctly callswindowToShow()
and uses the keyed result to render window details (icon, app_name, bounds) and pass the target toRecordingControls
. This will work as intended once thewindowToShow
accessor issue is resolved.
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
Outdated
Show resolved
Hide resolved
} | ||
|
||
function escapeRegExp(value: string) { | ||
return value.replace(/[\^$*+?.()|[\]{}-]/g, "\\$&"); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 18 days ago
To correctly escape all regexp metacharacters, including the backslash, we need to include backslash (\
) in the character set of the regular expression within escapeRegExp
. This is done by prepending backslash to the character set. The character set will be [\^$*+?.()|[\]{}\\-]
, where \\
escapes a backslash inside a character class. Locate the function definition in file apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
, at lines 196-198, and update line 197 accordingly. No other lines or dependencies are required.
-
Copy modified line R197
@@ -194,7 +194,7 @@ | ||
} | ||
|
||
function escapeRegExp(value: string) { | ||
return value.replace(/[\^$*+?.()|[\]{}-]/g, "\\$&"); | ||
return value.replace(/[\^$*+?.()|[\]{}\\-]/g, "\\$&"); | ||
} | ||
|
||
export function TargetCardSkeleton(props: { class?: string }) { |
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
8-9
: Remove manual icon imports; rely on auto-imported icons.These explicit imports violate the coding guideline for the desktop app. The icons should be auto-imported at build time.
Apply this diff:
-import IconLucideAppWindowMac from "~icons/lucide/app-window-mac"; -import IconMdiMonitor from "~icons/mdi/monitor";The JSX usages of
IconMdiMonitor
andIconLucideAppWindowMac
(lines 63, 65) will resolve via the auto-import system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
(1 hunks)apps/desktop/src/routes/target-select-overlay.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/routes/target-select-overlay.tsx
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}
: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.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/TargetCard.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:17:47.407Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.407Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Do not manually import icons in the desktop app; rely on auto-imported icons
Applied to files:
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx (1)
apps/desktop/src/utils/tauri.ts (2)
CaptureDisplayWithThumbnail
(363-363)CaptureWindowWithThumbnail
(365-365)
🪛 GitHub Check: CodeQL
apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx
[failure] 197-197: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
⏰ 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: Typecheck
- GitHub Check: Analyze (rust)
function escapeRegExp(value: string) { | ||
return value.replace(/[\^$*+?.()|[\]{}-]/g, "\\$&"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix escapeRegExp
: backslash itself is not escaped.
CodeQL correctly identifies that the character class [\^$*+?.()|[\]{}-]
does not include the backslash character. If the input contains a backslash (e.g., "C:\Users"), it won't be escaped and will be interpreted as an escape sequence in the RegExp
constructor, causing either a syntax error or unintended regex behavior.
Apply this diff to include backslash in the character class:
function escapeRegExp(value: string) {
- return value.replace(/[\^$*+?.()|[\]{}-]/g, "\\$&");
+ return value.replace(/[\\^$*+?.()|[\]{}-]/g, "\\$&");
}
Note: The first two backslashes (\\
) in the character class escape a literal backslash; the remaining metacharacters follow.
📝 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.
function escapeRegExp(value: string) { | |
return value.replace(/[\^$*+?.()|[\]{}-]/g, "\\$&"); | |
} | |
function escapeRegExp(value: string) { | |
return value.replace(/[\\^$*+?.()|[\]{}-]/g, "\\$&"); | |
} |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 197-197: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
🤖 Prompt for AI Agents
In apps/desktop/src/routes/(window-chrome)/new-main/TargetCard.tsx around lines
196 to 198, the escapeRegExp character class is missing the backslash so inputs
like "C:\Users" won't have backslashes escaped; update the character class to
include a literal backslash (escaped as "\\\\" in the string literal or `\\` in
a regex literal) so the backslash itself is escaped, e.g., add `\\` inside the
class and keep other metacharacters escaped as before, ensuring the function
returns a string safe for RegExp constructors.
Demo:
Example.Video.mov
Summary by CodeRabbit
New Features
Improvements
Chores