improvement: update new recording flow with mode info #970
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughIntroduces a standalone ModeSelect overlay and optional close mechanism; updates ModeSelect API and styling; wires overlay into target-select overlay flow with a “What is Mode?” trigger; adjusts RecordingControls props and UI; minor heading text change in mode-select route. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant T as TargetSelectOverlay
participant RC as RecordingControls
participant MS as ModeSelect (standalone)
U->>RC: Click "What is <Mode> Mode?"
RC->>T: setToggleModeSelect(true)
T->>T: Render overlay backdrop
T->>MS: Render <ModeSelect standalone onClose=...>
rect rgba(200,230,255,0.2)
note over MS: Standalone overlay UI
U-->>MS: View modes / optional close
alt User clicks Close
MS-->>T: onClose()
T->>T: setToggleModeSelect(false)
else Clicks outside overlay
U-->>T: Backdrop click
T->>T: setToggleModeSelect(false)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
apps/desktop/src/components/ModeSelect.tsx (2)
10-14: Remove unused prop from ModeOptionProps.
standalone?isn’t used byModeOption. Drop it to avoid API drift.interface ModeOptionProps { mode: RecordingMode; title: string; description: string; - standalone?: boolean; icon: (props: { class: string; style?: JSX.CSSProperties }) => JSX.Element; isSelected: boolean; onSelect: (mode: RecordingMode) => void; }
81-84: Add an id for aria-labelledby target.
Ensures the dialog announces its title.- <h2 class="text-[24px] col-span-2 font-medium text-center text-gray-12"> + <h2 id="mode-select-title" class="text-[24px] col-span-2 font-medium text-center text-gray-12"> Recording Modes </h2>apps/desktop/src/routes/target-select-overlay.tsx (4)
35-37: Consider movingcapitalizeto a shared util.
It’s used in multiple places; avoid duplication.
786-793: Use a button for the mode menu trigger.
Improves a11y; stopPropagation remains.- <div + <button + type="button" class="pl-2.5 group-hover:bg-blue-10 transition-colors pr-3 py-1.5 flex items-center" onClick={(e) => { e.stopPropagation(); menuModes().then((menu) => menu.popup()); }} > <IconCapCaretDown class="focus:rotate-90" /> - </div> + </button>
796-803: Use a button for the gear menu trigger.
Same a11y rationale.- <div + <button + type="button" onClick={(e) => { e.stopPropagation(); preRecordingMenu().then((menu) => menu.popup()); }} class="flex justify-center items-center rounded-full border transition-opacity bg-gray-6 text-gray-12 size-9 hover:opacity-80" > <IconCapGear class="will-change-transform size-5" /> - </div> + </button>
756-767: Handle startRecording errors.
Surface failures and avoid double-activating when sign-in needed.- commands.startRecording({ - capture_target: props.target, - mode: rawOptions.mode, - capture_system_audio: rawOptions.captureSystemAudio, - }); + try { + await commands.startRecording({ + capture_target: props.target, + mode: rawOptions.mode, + capture_system_audio: rawOptions.captureSystemAudio, + }); + } catch (err) { + console.error("Failed to start recording:", err); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/desktop/src/components/ModeSelect.tsx(4 hunks)apps/desktop/src/routes/mode-select.tsx(1 hunks)apps/desktop/src/routes/target-select-overlay.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/mode-select.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/ModeSelect.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/mode-select.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/ModeSelect.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/desktop/src/routes/mode-select.tsxapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/ModeSelect.tsx
🧠 Learnings (1)
📚 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/routes/target-select-overlay.tsx
🧬 Code graph analysis (2)
apps/desktop/src/routes/target-select-overlay.tsx (2)
apps/desktop/src/utils/tauri.ts (3)
DisplayId(362-362)ScreenCaptureTarget(430-430)commands(7-266)apps/desktop/src/utils/queries.ts (1)
createOptionsQuery(87-118)
apps/desktop/src/components/ModeSelect.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
RecordingMode(419-419)
⏰ 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 (5)
apps/desktop/src/routes/mode-select.tsx (1)
26-27: Heading change aligns with overlay copy; good for consistency.
No functional impact; matches the new standalone overlay title.apps/desktop/src/routes/target-select-overlay.tsx (4)
165-167: Prop threading looks good.
setToggleModeSelectpassed down so children can open the overlay.
214-219: Prop threading on window path looks good.
Consistent with the display path.
222-222: Button variant change acknowledged.
No functional concerns; verify contrast in both themes.
681-684: RecordingControls signature change is safe.
Optional prop addition; upstream call sites updated in this file.
Comment on lines
20
to
27
| class={cx( | ||
| `p-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`, | ||
| { | ||
| "ring-2 ring-offset-2 hover:bg-gray-2 cursor-default ring-blue-9 ring-offset-gray-100": | ||
| props.isSelected, | ||
| "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer": | ||
| !props.isSelected, | ||
| }, | ||
| `p-5 space-y-3 rounded-lg border transition-all duration-200 border-gray-4 dark:border-gray-3 h-fit bg-gray-3 dark:bg-gray-2`, | ||
| )} | ||
| > | ||
| <div class="flex flex-col items-center mb-2 text-center"> | ||
| {props.icon({ | ||
| class: cx( | ||
| "size-12 mb-3", | ||
| props.isSelected ? "opacity-100" : "opacity-30", | ||
| ), | ||
| style: { | ||
| filter: props.isSelected | ||
| ? "drop-shadow(0 0 0.5rem rgba(255, 255, 255, 0.5))" | ||
| : "none", | ||
| }, | ||
| class: "size-12 mb-5 invert dark:invert-0", | ||
| })} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Selected state no longer reflected; either style it or remove isSelected.
UX regression: current card styles don’t indicate selection. Add a selected style or drop the prop.
- class={cx(
- `p-5 space-y-3 rounded-lg border transition-all duration-200 border-gray-4 dark:border-gray-3 h-fit bg-gray-3 dark:bg-gray-2`,
- )}
+ data-selected={props.isSelected ? "true" : "false"}
+ class={cx(
+ "p-5 space-y-3 rounded-lg border transition-all duration-200 h-fit bg-gray-3 dark:bg-gray-2",
+ "border-gray-4 dark:border-gray-3 ring-1 ring-transparent",
+ // highlight when selected
+ props.isSelected && "border-blue-8 ring-blue-7"
+ )}📝 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.
Suggested change
| class={cx( | |
| `p-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`, | |
| { | |
| "ring-2 ring-offset-2 hover:bg-gray-2 cursor-default ring-blue-9 ring-offset-gray-100": | |
| props.isSelected, | |
| "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer": | |
| !props.isSelected, | |
| }, | |
| `p-5 space-y-3 rounded-lg border transition-all duration-200 border-gray-4 dark:border-gray-3 h-fit bg-gray-3 dark:bg-gray-2`, | |
| )} | |
| > | |
| <div class="flex flex-col items-center mb-2 text-center"> | |
| {props.icon({ | |
| class: cx( | |
| "size-12 mb-3", | |
| props.isSelected ? "opacity-100" : "opacity-30", | |
| ), | |
| style: { | |
| filter: props.isSelected | |
| ? "drop-shadow(0 0 0.5rem rgba(255, 255, 255, 0.5))" | |
| : "none", | |
| }, | |
| class: "size-12 mb-5 invert dark:invert-0", | |
| })} | |
| <div | |
| // reflect selection state for styling/testing | |
| data-selected={props.isSelected ? "true" : "false"} | |
| class={cx( | |
| "p-5 space-y-3 rounded-lg border transition-all duration-200 h-fit bg-gray-3 dark:bg-gray-2", | |
| "border-gray-4 dark:border-gray-3 ring-1 ring-transparent", | |
| // highlight when selected | |
| props.isSelected && "border-blue-8 ring-blue-7" | |
| )} | |
| > | |
| <div class="flex flex-col items-center mb-2 text-center"> | |
| {props.icon({ | |
| class: "size-12 mb-5 invert dark:invert-0", | |
| })} |
🤖 Prompt for AI Agents
In apps/desktop/src/components/ModeSelect.tsx around lines 20 to 27, the card no
longer visually reflects the selection state; either apply a selected style when
props.isSelected is true or remove the isSelected prop usage. Update the
className construction (using cx) to conditionally include a "selected" set of
classes (e.g., stronger border color, background highlight, ring or shadow) when
props.isSelected is true, ensuring you reference props.isSelected and keep
existing classes otherwise; if selection state is truly unused, remove the
isSelected prop and any conditional logic that expects it. Ensure the
conditional uses the same cx helper pattern as the surrounding code and that
styles work in both light and dark modes.
Comment on lines
+63
to
+71
| <div | ||
| class={cx( | ||
| "grid grid-cols-2 gap-8 items-center text-center bg-gray-1", | ||
| props.standalone | ||
| ? "absolute z-10 border border-gray-3 p-16 rounded-xl" | ||
| : "", | ||
| )} | ||
| onClick={(e) => e.stopPropagation()} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Center and label the overlay (dialog semantics).
Absolute without offsets may not center; also add a11y attributes.
- <div
- class={cx(
- "grid grid-cols-2 gap-8 items-center text-center bg-gray-1",
- props.standalone
- ? "absolute z-10 border border-gray-3 p-16 rounded-xl"
- : "",
- )}
- onClick={(e) => e.stopPropagation()}
- >
+ <div
+ role={props.standalone ? "dialog" : undefined}
+ aria-modal={props.standalone ? "true" : undefined}
+ aria-labelledby={props.standalone ? "mode-select-title" : undefined}
+ class={cx(
+ "grid grid-cols-2 gap-8 items-center text-center bg-gray-1",
+ props.standalone
+ ? "fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 z-20 border border-gray-3 p-16 rounded-xl shadow-2xl"
+ : ""
+ )}
+ onClick={(e) => e.stopPropagation()}
+ >📝 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.
Suggested change
| <div | |
| class={cx( | |
| "grid grid-cols-2 gap-8 items-center text-center bg-gray-1", | |
| props.standalone | |
| ? "absolute z-10 border border-gray-3 p-16 rounded-xl" | |
| : "", | |
| )} | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| <div | |
| role={props.standalone ? "dialog" : undefined} | |
| aria-modal={props.standalone ? "true" : undefined} | |
| aria-labelledby={props.standalone ? "mode-select-title" : undefined} | |
| class={cx( | |
| "grid grid-cols-2 gap-8 items-center text-center bg-gray-1", | |
| props.standalone | |
| ? "fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 z-20 border border-gray-3 p-16 rounded-xl shadow-2xl" | |
| : "" | |
| )} | |
| onClick={(e) => e.stopPropagation()} | |
| > |
🤖 Prompt for AI Agents
In apps/desktop/src/components/ModeSelect.tsx around lines 63 to 71, the overlay
uses absolute positioning without offsets and lacks dialog semantics; update the
standalone class to center the overlay (e.g., apply utility offsets/transform
such as top-1/2 left-1/2 and -translate-x-1/2 -translate-y-1/2 or equivalent
centering classes) and add proper accessibility attributes by giving the
container role="dialog", aria-modal="true" and either aria-labelledby pointing
to a visible heading id inside the overlay or an aria-label describing the
dialog; keep the existing onClick stopPropagation behavior.
Comment on lines
+72
to
+79
| <Show when={props.onClose}> | ||
| <div | ||
| onClick={() => props.onClose?.()} | ||
| class="absolute -top-2.5 -right-2.5 p-2 rounded-full border duration-200 bg-gray-2 border-gray-3 hover:bg-gray-3 transition-duration" | ||
| > | ||
| <IconCapX class="invert-1 size-2 dark:invert" /> | ||
| </div> | ||
| </Show> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a button for the close control and fix Tailwind classes.
Divs aren’t keyboard-accessible; also transition-duration isn’t a Tailwind class and invert-1 is invalid.
- <Show when={props.onClose}>
- <div
- onClick={() => props.onClose?.()}
- class="absolute -top-2.5 -right-2.5 p-2 rounded-full border duration-200 bg-gray-2 border-gray-3 hover:bg-gray-3 transition-duration"
- >
- <IconCapX class="invert-1 size-2 dark:invert" />
- </div>
- </Show>
+ <Show when={props.onClose}>
+ <button
+ type="button"
+ aria-label="Close"
+ onClick={() => props.onClose?.()}
+ class="absolute -top-2.5 -right-2.5 p-2 rounded-full border bg-gray-2 border-gray-3 hover:bg-gray-3 transition-colors duration-200"
+ >
+ <IconCapX class="invert size-2 dark:invert-0" />
+ </button>
+ </Show>📝 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.
Suggested change
| <Show when={props.onClose}> | |
| <div | |
| onClick={() => props.onClose?.()} | |
| class="absolute -top-2.5 -right-2.5 p-2 rounded-full border duration-200 bg-gray-2 border-gray-3 hover:bg-gray-3 transition-duration" | |
| > | |
| <IconCapX class="invert-1 size-2 dark:invert" /> | |
| </div> | |
| </Show> | |
| <Show when={props.onClose}> | |
| <button | |
| type="button" | |
| aria-label="Close" | |
| onClick={() => props.onClose?.()} | |
| class="absolute -top-2.5 -right-2.5 p-2 rounded-full border bg-gray-2 border-gray-3 hover:bg-gray-3 transition-colors duration-200" | |
| > | |
| <IconCapX class="invert size-2 dark:invert-0" /> | |
| </button> | |
| </Show> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/ModeSelect.tsx around lines 72 to 79, replace the
interactive div with a semantic button to make the close control
keyboard-accessible (use type="button"), add an aria-label (e.g., "Close"), and
keep calling props.onClose() on click; remove any custom key handlers since
button handles Enter/Space. Fix the Tailwind utility class names by replacing
transition-duration with transition and invert-1 with invert (or remove invert
if not needed), preserve layout classes (absolute -top-2.5 -right-2.5 p-2
rounded-full border bg-gray-2 border-gray-3 hover:bg-gray-3) and add
focus-visible ring/focus:outline-none utilities so the button shows focus when
tabbed. Ensure IconCapX class usage remains but update any invalid size class if
necessary.
Comment on lines
+152
to
+163
| <Show when={toggleModeSelect()}> | ||
| {/* Transparent overlay to capture outside clicks */} | ||
| <div | ||
| class="absolute inset-0 z-10" | ||
| onClick={() => setToggleModeSelect(false)} | ||
| /> | ||
| <ModeSelect | ||
| standalone | ||
| onClose={() => setToggleModeSelect(false)} | ||
| /> | ||
| </Show> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Overlay layering and dismissal UX.
- Ensure the dialog is above the click-catcher; bump dialog z-index if needed.
- Add Escape-to-close handling while overlay is open.
<Show when={toggleModeSelect()}>
{/* Transparent overlay to capture outside clicks */}
<div
- class="absolute inset-0 z-10"
+ class="absolute inset-0 z-10"
onClick={() => setToggleModeSelect(false)}
/>
<ModeSelect
standalone
onClose={() => setToggleModeSelect(false)}
/>
</Show>Add this listener near your other listeners to close on Escape (outside the changed hunk, included here for completeness):
createEventListener(document, "keydown", (e) => {
if (e.key === "Escape" && toggleModeSelect()) setToggleModeSelect(false);
});🤖 Prompt for AI Agents
In apps/desktop/src/routes/target-select-overlay.tsx around lines 152 to 163,
the transparent click-catcher div may be layered above the ModeSelect dialog and
there is no Escape-to-close handling; bump the ModeSelect/dialog z-index so it
is rendered above the click-catcher (ensure ModeSelect receives a higher
z-index/class or pass a prop to set z-index > 10) and add a document keydown
listener alongside your other listeners that closes the overlay when Escape is
pressed (check toggleModeSelect() before calling setToggleModeSelect(false) to
avoid unnecessary state changes).
Comment on lines
+744
to
+751
| <> | ||
| <div class="flex gap-2.5 items-center p-2.5 my-2.5 rounded-xl border min-w-fit w-fit bg-gray-2 border-gray-4"> | ||
| <div | ||
| onClick={() => setOptions("targetMode", null)} | ||
| class="flex justify-center items-center rounded-full transition-opacity bg-gray-12 size-9 hover:opacity-80" | ||
| > | ||
| <IconCapX class="invert will-change-transform size-3 dark:invert-0" /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a button for the close control.
Keyboard/a11y and semantics; consistent with the dialog close.
- <div class="flex gap-2.5 items-center p-2.5 my-2.5 rounded-xl border min-w-fit w-fit bg-gray-2 border-gray-4">
- <div
- onClick={() => setOptions("targetMode", null)}
- class="flex justify-center items-center rounded-full transition-opacity bg-gray-12 size-9 hover:opacity-80"
- >
- <IconCapX class="invert will-change-transform size-3 dark:invert-0" />
- </div>
+ <div class="flex gap-2.5 items-center p-2.5 my-2.5 rounded-xl border min-w-fit w-fit bg-gray-2 border-gray-4">
+ <button
+ type="button"
+ aria-label="Close target selection"
+ onClick={() => setOptions("targetMode", null)}
+ class="flex justify-center items-center rounded-full transition-opacity bg-gray-12 size-9 hover:opacity-80"
+ >
+ <IconCapX class="invert will-change-transform size-3 dark:invert-0" />
+ </button>📝 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.
Suggested change
| <> | |
| <div class="flex gap-2.5 items-center p-2.5 my-2.5 rounded-xl border min-w-fit w-fit bg-gray-2 border-gray-4"> | |
| <div | |
| onClick={() => setOptions("targetMode", null)} | |
| class="flex justify-center items-center rounded-full transition-opacity bg-gray-12 size-9 hover:opacity-80" | |
| > | |
| <IconCapX class="invert will-change-transform size-3 dark:invert-0" /> | |
| </div> | |
| <> | |
| <div class="flex gap-2.5 items-center p-2.5 my-2.5 rounded-xl border min-w-fit w-fit bg-gray-2 border-gray-4"> | |
| <button | |
| type="button" | |
| aria-label="Close target selection" | |
| onClick={() => setOptions("targetMode", null)} | |
| class="flex justify-center items-center rounded-full transition-opacity bg-gray-12 size-9 hover:opacity-80" | |
| > | |
| <IconCapX class="invert will-change-transform size-3 dark:invert-0" /> | |
| </button> |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/target-select-overlay.tsx around lines 744 to 751,
the close control is implemented as a clickable div which is non-semantic and
not keyboard-accessible; replace the div with a <button> element, give it
type="button", preserve the existing class names, move the onClick handler to
the button, and add an accessible label (e.g. aria-label="Close" or visually
hidden text) so it behaves like the dialog close control and supports
keyboard/a11y semantics.
Merged
6 tasks
This was referenced Sep 26, 2025
This was referenced Oct 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Style