feat: LaunchWindow refactoring, SourceSelector performance#431
Merged
webadderall merged 37 commits intowebadderallorg:mainfrom May 7, 2026
Merged
Conversation
fix: SourceSelection performances add: seperator.tsx for "divider" component, instead of a non-generic component. can be reused elsewhere!
add: button.tsx "iconSize" variants for different icons size, replacing effectively iconButton. and can be reused accross the whole app. fix: RecordingControls with those same ui component !
… still weirdly behaving but it's better
…everything right but it's getting there
…opening up select... add: launchTheme.css for consistent styling accross the whole components spectrum related to LaunchWindow add: correct styles accross the popovers! with unstyled parameters in Popover.tsx because it was needed to make sure its the same
…dow to render the overlay, otherwise it wouldn't stop jittering.) add: launchTheme.css to have everything under the same variables.
add: caching for 1.2s for a better user experience, less refreshing, it's actually instant now.
LaunchWindow could still be further reduced, but honestly from 1600 lines to 650 lines is a win
fix: Icons that were deprecated (mostly added Icon at the end of each ones.) I also removed "as" Microphone as Mic everywhere... Let's use normal icon names
…gging (avoiding re-renders) add: SourceSelector marquee (scrolling text when too big)
Contributor
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors the launch HUD and renderer handshake, adds a short-lived sources cache, extracts multiple launch hooks, introduces a popover coordinator and scaffold, adds UI theme variables and primitives, and updates launch locales. ChangesElectron HUD Overlay & IPC
React Hook Extraction
Popover System & Components
LaunchWindow Refactoring & UI
Styling & UI primitives
Internationalization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Contributor
|
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the HUD/LaunchWindow UI into smaller components, improves drag performance (RAF-based updates), and optimizes desktop source enumeration to reduce jitter/lag in the source selector workflow.
Changes:
- Split LaunchWindow into dedicated popover components + hooks (mic/webcam/countdown/more/source), and moved recording controls into a reusable component.
- Reworked HUD overlay behavior (renderer-ready show signal, full-work-area sizing) and added CSS/theming to unify popover styling.
- Added short-TTL caching for
get-sourcesIPC and updated the SourceSelector UI to use thumbnails with smoother behavior.
Reviewed changes
Copilot reviewed 36 out of 41 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/launch/LaunchWindow.tsx | Refactors the HUD overlay UI and popover structure; updates drag/interaction wiring. |
| src/components/launch/RecordingControls.tsx | Extracts recording-mode controls into a separate component. |
| src/components/launch/SourceSelector.tsx | Replaces the previous full-window source selector with a popover-based selector + marquee behavior. |
| src/components/launch/popovers/* | New popover components and coordinator to manage mutually exclusive popovers. |
| src/components/launch/hooks/* | New hooks for HUD drag, webcam preview overlay, system state, and timer logic. |
| src/components/launch/LaunchWindow.module.css | Updates popover/menu styling to use shared theme variables. |
| src/components/launch/launchTheme.css | Introduces shared CSS variables for HUD popover theming. |
| src/components/launch/SourceSelector.css | Adds dedicated styling for the new source selector popover/list UI. |
| src/components/ui/button.tsx | Adds iconSize variant to standardize icon-only button sizing. |
| src/components/ui/popover.tsx | Adds usePortal + unstyled options for HUD-styled popovers. |
| src/components/ui/separator.tsx | Introduces a reusable separator/divider component. |
| src/index.css | Hides overflow for the HUD overlay window root elements. |
| src/App.tsx | Adds data-window-type and HUD overlay window classes for targeted styling. |
| electron/windows.ts | Changes HUD overlay window bounds logic and show timing (renderer-ready + fallback). |
| electron/preload.ts / electron/electron-env.d.ts | Replaces old HUD sizing IPC with hudOverlayRendererReady. |
| electron/ipc/register/sources.ts | Adds TTL caching and optional window icon fetching for get-sources. |
| package-lock.json | Updates lockfile metadata/version values. |
| electron/native/bin/win32-x64/helpers-manifest.json | Updates helper binary fingerprints and timestamps. |
Comment on lines
+191
to
+221
| export const SourceSelector = React.memo(function SourceSelector({ | ||
| screenSources = [], | ||
| windowSources = [], | ||
| selectedSource = "Screen", | ||
| loading = false, | ||
| onSourceSelect = () => {}, | ||
| onFetchSources = async () => {}, | ||
| open = false, | ||
| onOpenChange = () => {}, | ||
| children, | ||
| }: SourceSelectorProps) { | ||
| const hasPrefetchedRef = useRef(false); | ||
|
|
||
| const handleSourceSelect = (source: DesktopSource) => setSelectedSource(source); | ||
| const handleSourceKeyDown = ( | ||
| event: ReactKeyboardEvent<HTMLDivElement>, | ||
| source: DesktopSource, | ||
| ) => { | ||
| if (event.key !== "Enter" && event.key !== " ") { | ||
| const prefetchSources = React.useCallback(() => { | ||
| if (hasPrefetchedRef.current) { | ||
| return; | ||
| } | ||
| hasPrefetchedRef.current = true; | ||
| void onFetchSources(); | ||
| }, [onFetchSources]); | ||
|
|
||
| event.preventDefault(); | ||
| handleSourceSelect(source); | ||
| }; | ||
|
|
||
| const handleShare = async () => { | ||
| if (selectedSource) await window.electronAPI.selectSource(selectedSource); | ||
| }; | ||
|
|
||
| if (loading) { | ||
| return ( | ||
| <div | ||
| className={`h-full flex items-center justify-center ${styles.glassContainer}`} | ||
| style={{ minHeight: "100vh" }} | ||
| > | ||
| <div className="text-center"> | ||
| <div className="animate-spin rounded-full h-6 w-6 border-b-2 border-zinc-600 mx-auto mb-2" /> | ||
| <p className="text-xs text-zinc-300">{t("sourceSelector.loadingSources")}</p> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| // Fetch sources when popover opens | ||
| useEffect(() => { | ||
| if (open) { | ||
| void onFetchSources(); | ||
| } | ||
| }, [open, onFetchSources]); | ||
|
|
||
| return ( | ||
| <div | ||
| className={`min-h-screen flex flex-col items-center justify-center ${styles.glassContainer}`} | ||
| > | ||
| <div className="flex-1 flex flex-col w-full max-w-xl" style={{ padding: 0 }}> | ||
| <Tabs | ||
| value={activeTab} | ||
| onValueChange={(value) => setActiveTab(value as "screens" | "windows")} | ||
| > | ||
| <TabsList className="grid grid-cols-2 mb-3 bg-zinc-900/40 rounded-full"> | ||
| <TabsTrigger | ||
| value="screens" | ||
| className="data-[state=active]:bg-[#2563EB] data-[state=active]:text-white text-zinc-200 rounded-full text-xs py-1" | ||
| <Popover open={open} onOpenChange={onOpenChange}> | ||
| <PopoverTrigger asChild> |
Comment on lines
+220
to
+248
| <Popover open={open} onOpenChange={onOpenChange}> | ||
| <PopoverTrigger asChild> | ||
| <div onPointerEnter={prefetchSources} onFocusCapture={prefetchSources}> | ||
| {children || ( | ||
| <Button | ||
| variant="outline" | ||
| size="lg" | ||
| className={cn( | ||
| "group gap-2 px-3 min-w-0 max-w-[180px] rounded-[11px] font-medium text-[12px] [ -webkit-app-region:no-drag ] shrink-0", | ||
| "border-[#2a2a34] bg-[#1a1a22] text-[#eeeef2] hover:border-[#3e3e4c] hover:bg-[#20202a] transition-all", | ||
| "data-[state=open]:border-[#3e3e4c] data-[state=open]:bg-[#20202a]", | ||
| )} | ||
| title={selectedSource} | ||
| > | ||
| {t("sourceSelector.screens")} ({screenSources.length}) | ||
| </TabsTrigger> | ||
| <TabsTrigger | ||
| value="windows" | ||
| className="data-[state=active]:bg-[#2563EB] data-[state=active]:text-white text-zinc-200 rounded-full text-xs py-1" | ||
| > | ||
| {t("sourceSelector.windows")} ({windowSources.length}) | ||
| </TabsTrigger> | ||
| </TabsList> | ||
| <div className="h-72 flex flex-col justify-stretch"> | ||
| <TabsContent value="screens" className="h-full"> | ||
| <div | ||
| className={`grid grid-cols-2 gap-2 h-full overflow-y-auto pr-1 relative ${styles.sourceGridScroll}`} | ||
| > | ||
| {screenSources.length === 0 && ( | ||
| <div className="col-span-2 text-center text-xs text-zinc-500 py-8"> | ||
| {t("sourceSelector.noScreensAvailable")} | ||
| </div> | ||
| )} | ||
| {screenSources.map((source) => { | ||
| const isSelected = selectedSource?.id === source.id; | ||
|
|
||
| return ( | ||
| <Card | ||
| key={source.id} | ||
| className={`${styles.sourceCard} ${isSelected ? styles.selected : ""} cursor-pointer h-fit p-2 scale-95 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#2563EB] focus-visible:ring-offset-2 focus-visible:ring-offset-transparent`} | ||
| style={{ margin: 8, width: "90%", maxWidth: 220 }} | ||
| onClick={() => handleSourceSelect(source)} | ||
| onKeyDown={(event) => | ||
| handleSourceKeyDown(event, source) | ||
| } | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-pressed={isSelected} | ||
| > | ||
| <div className="p-1"> | ||
| <div className="relative mb-1"> | ||
| <img | ||
| src={source.thumbnail || ""} | ||
| alt={source.name} | ||
| className="w-full aspect-video object-cover rounded border border-zinc-800" | ||
| /> | ||
| {isSelected && ( | ||
| <div className="absolute -top-1 -right-1"> | ||
| <div className="w-4 h-4 bg-[#2563EB] rounded-full flex items-center justify-center shadow-md"> | ||
| <MdCheck className={styles.icon} /> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| <div className={styles.name + " truncate"}> | ||
| {source.name} | ||
| </div> | ||
| </div> | ||
| </Card> | ||
| ); | ||
| })} | ||
| <Monitor size={16} className="shrink-0" /> | ||
| <div className="flex-1 min-w-0"> | ||
| <MarqueeText text={selectedSource} /> | ||
| </div> | ||
| </TabsContent> | ||
| <TabsContent value="windows" className="h-full"> | ||
| <p className="text-[10px] text-zinc-500 mb-1 px-1"> | ||
| {t("sourceSelector.windowsNote")} | ||
| </p> | ||
| <div | ||
| className={`grid grid-cols-2 gap-2 h-full overflow-y-auto pr-1 relative ${styles.sourceGridScroll}`} | ||
| > | ||
| {windowSources.length === 0 && ( | ||
| <div className="col-span-2 text-center text-xs text-zinc-500 py-8"> | ||
| {t("sourceSelector.noWindowsAvailable")} | ||
| </div> | ||
| <ChevronUp | ||
| size={10} | ||
| className={cn( | ||
| "text-[#6b6b78] ml-0.5 shrink-0 transition-transform duration-200", | ||
| open ? "" : "rotate-180", | ||
| )} | ||
| {windowSources.map((source) => { | ||
| const isSelected = selectedSource?.id === source.id; | ||
|
|
||
| return ( | ||
| <Card | ||
| key={source.id} | ||
| className={`${styles.sourceCard} ${isSelected ? styles.selected : ""} cursor-pointer h-fit p-2 scale-95 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#2563EB] focus-visible:ring-offset-2 focus-visible:ring-offset-transparent`} | ||
| style={{ margin: 8, width: "90%", maxWidth: 220 }} | ||
| onClick={() => handleSourceSelect(source)} | ||
| onKeyDown={(event) => | ||
| handleSourceKeyDown(event, source) | ||
| } | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-pressed={isSelected} | ||
| > | ||
| <div className="p-1"> | ||
| <div className="relative mb-1"> | ||
| {source.thumbnail ? ( | ||
| <img | ||
| src={source.thumbnail} | ||
| alt={source.name} | ||
| className="w-full aspect-video object-cover rounded border border-gray-700" | ||
| /> | ||
| ) : ( | ||
| <div className="w-full aspect-video rounded border border-gray-700 bg-zinc-900/80 flex flex-col items-center justify-center text-zinc-400 gap-2"> | ||
| {source.appIcon ? ( | ||
| <img | ||
| src={source.appIcon} | ||
| alt="App icon" | ||
| className="w-8 h-8 rounded-md" | ||
| /> | ||
| ) : ( | ||
| <div className="w-8 h-8 rounded-md bg-zinc-800 border border-zinc-700" /> | ||
| )} | ||
| <div className="text-[10px] uppercase tracking-[0.2em] text-zinc-500"> | ||
| {t( | ||
| "sourceSelector.windowPlaceholder", | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} | ||
| {isSelected && ( | ||
| <div className="absolute -top-1 -right-1"> | ||
| <div className="w-4 h-4 bg-blue-600 rounded-full flex items-center justify-center shadow-md"> | ||
| <MdCheck className={styles.icon} /> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| <div className="flex items-center gap-1"> | ||
| {source.appIcon && ( | ||
| <img | ||
| src={source.appIcon} | ||
| alt="App icon" | ||
| className={ | ||
| styles.icon + " flex-shrink-0" | ||
| } | ||
| /> | ||
| )} | ||
| <div className={styles.name + " truncate"}> | ||
| {source.name} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </Card> | ||
| ); | ||
| })} | ||
| </div> | ||
| </TabsContent> | ||
| </div> | ||
| </Tabs> | ||
| </div> | ||
| <div className="border-t border-zinc-800 p-2 w-full max-w-xl"> | ||
| <div className="flex justify-center gap-2"> | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => window.close()} | ||
| className="px-4 py-1 text-xs bg-zinc-800 border-zinc-700 text-zinc-200 hover:bg-zinc-700" | ||
| > | ||
| {t("sourceSelector.cancel")} | ||
| </Button> | ||
| <Button | ||
| onClick={handleShare} | ||
| disabled={!selectedSource} | ||
| className="px-4 py-1 text-xs bg-[#2563EB] text-white hover:bg-[#2563EB]/80 disabled:opacity-50 disabled:bg-zinc-700" | ||
| > | ||
| {t("sourceSelector.share")} | ||
| </Button> | ||
| /> | ||
| </Button> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </PopoverTrigger> |
Comment on lines
+151
to
+160
| <div className="px-3 py-2 text-[10px] font-semibold uppercase tracking-[0.08em] source-selector-label flex items-center gap-2"> | ||
| {t("recording.screens")} | ||
| <span | ||
| className={cn( | ||
| "normal-case tracking-normal text-[10px] source-selector-muted transition-opacity duration-150", | ||
| loading ? "opacity-100" : "opacity-0", | ||
| )} | ||
| > | ||
| {t("common.loading", "Refreshing...")} | ||
| </span> |
Comment on lines
448
to
453
| <motion.div | ||
| ref={hudBarRef} | ||
| layout={!showRecordingWebcamPreview} | ||
| layout={!showRecordingWebcamPreview && !isHudDragging} | ||
| transition={hudStateTransition} | ||
| className={`${styles.bar} mb-2`} | ||
| > |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/components/launch/SourceSelector.css (1)
114-115: ⚡ Quick winAdd reduced-motion fallback for marquee animation.
The hover-triggered marquee should respect
prefers-reduced-motionto avoid unnecessary motion for sensitive users.Proposed change
`@keyframes` source-selector-marquee { from { transform: translateX(0); } to { transform: translateX(-50%); } } + +@media (prefers-reduced-motion: reduce) { + .source-selector-marquee-track { + animation: none; + } + + .group:hover .source-selector-marquee[data-overflowing="true"] .source-selector-marquee-static { + opacity: 1; + } + + .group:hover .source-selector-marquee[data-overflowing="true"] .source-selector-marquee-animated { + opacity: 0; + } +}Also applies to: 126-136, 138-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/SourceSelector.css` around lines 114 - 115, The marquee animation defined by animation: source-selector-marquee (and related rules around the hover-triggered marquee) doesn't respect users' prefers-reduced-motion setting; add a prefers-reduced-motion media query that disables the animation (e.g., set animation: none and animation-play-state: paused or remove transform/animation properties) for the selector(s) using source-selector-marquee so the marquee and hover-started animation are paused when prefers-reduced-motion: reduce is set.src/App.tsx (1)
17-48: ⚡ Quick winAdd cleanup for global class/style mutations in the mount effect.
This effect mutates
documentElement,body, and#rootbut never reverts. A cleanup avoids stale global UI state during remount/HMR/test scenarios.Proposed change
useEffect(() => { + const root = document.getElementById("root"); const params = new URLSearchParams(window.location.search); const type = params.get("windowType") || ""; const isMacOS = /mac/i.test(navigator.platform); setWindowType(type); document.documentElement.dataset.windowType = type; @@ - document.getElementById("root")?.style.setProperty("background", "transparent"); + root?.style.setProperty("background", "transparent"); } @@ - document.getElementById("root")?.classList.add("hud-overlay-window"); + root?.classList.add("hud-overlay-window"); } else if (type === "update-toast") { document.documentElement.style.overflow = "visible"; document.body.style.overflow = "visible"; - document.getElementById("root")?.style.setProperty("overflow", "visible"); + root?.style.setProperty("overflow", "visible"); } @@ loadAllCustomFonts().catch((error) => { console.error("Failed to load custom fonts:", error); }); + + return () => { + document.documentElement.classList.remove("hud-overlay-window"); + document.body.classList.remove("hud-overlay-window"); + root?.classList.remove("hud-overlay-window"); + delete document.documentElement.dataset.windowType; + }; }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 17 - 48, The effect in useEffect (which calls setWindowType and loadAllCustomFonts) mutates document.documentElement.dataset.windowType, documentElement/body styles and classes and `#root` styles but never reverts them; update the useEffect to capture previous values (dataset.windowType, documentElement/body/background and overflow, and root element styles and class presence) and return a cleanup function that restores those previous values, removes the "hud-overlay-window" class if added, and clears dataset.windowType (or restores the prior value); also ensure any ongoing loadAllCustomFonts promise is handled (e.g., track a cancelled flag) to avoid applying font side-effects after unmount.src/components/launch/hooks/useRecordingTimer.ts (1)
10-10: ⚡ Quick winUse renderer-safe timer typing here.
At Line 10,
NodeJS.Timeoutis semantically incorrect for browser-only code. Since this React hook uses only DOM APIs (setInterval/clearInterval), preferReturnType<typeof setInterval>which correctly resolves tonumberin the DOM context, ensuring proper cross-environment type safety.Proposed change
- let timer: NodeJS.Timeout | null = null; + let timer: ReturnType<typeof setInterval> | null = null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/hooks/useRecordingTimer.ts` at line 10, The timer variable in useRecordingTimer.ts is typed as NodeJS.Timeout | null which is incorrect for browser-only React code; change the declaration of timer (used with setInterval/clearInterval inside the hook) to use ReturnType<typeof setInterval> | null so it resolves to the DOM number type and is renderer-safe, then ensure usages in start/stop logic keep the same null checks and pass the timer value to clearInterval correctly.src/components/launch/popovers/WebcamPopover.tsx (1)
104-123: 💤 Low valueConsider extracting the duplicated selection condition.
The device selection check
webcamDeviceId === device.deviceId || selectedVideoDeviceId === device.deviceIdis repeated for bothiconandselectedprops. Extracting this to a variable would improve readability.♻️ Suggested refactor
{videoDevices.map((device) => ( - <DropdownItem + const isDeviceSelected = webcamEnabled && + (webcamDeviceId === device.deviceId || selectedVideoDeviceId === device.deviceId); + return ( + <DropdownItem key={device.deviceId} - icon={ - webcamEnabled && - (webcamDeviceId === device.deviceId || selectedVideoDeviceId === device.deviceId) ? ( - <Video size={16} /> - ) : ( - <VideoOff size={16} /> - ) - } - selected={ - webcamEnabled && - (webcamDeviceId === device.deviceId || selectedVideoDeviceId === device.deviceId) - } + icon={isDeviceSelected ? <Video size={16} /> : <VideoOff size={16} />} + selected={isDeviceSelected} onClick={() => onSelectVideoDevice(device.deviceId)} > {device.label} - </DropdownItem> + </DropdownItem>); ))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/popovers/WebcamPopover.tsx` around lines 104 - 123, The repeated selection expression inside the videoDevices.map for DropdownItem should be extracted to a local boolean to improve readability: compute a const (e.g., isSelected) right before rendering each <DropdownItem> using the existing symbols webcamEnabled, webcamDeviceId, selectedVideoDeviceId and device.deviceId, then use that variable for both the icon conditional and the selected prop, leaving onSelectVideoDevice unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/launch/hooks/useHudBarDrag.ts`:
- Around line 122-132: When ending the drag, first apply any final pointer
position before clearing state: if hudDragPendingPointerRef.current (or the
received event.clientX/clientY) exists, compute the final delta from
hudDragStartRef.current to that pointer, update recordingHudOffsetRef.current
with that delta and call setRecordingHudOffset({
...recordingHudOffsetRef.current }) so the HUD snaps to the last position, then
proceed to cancel hudDragMoveRafRef, clear hudDragPendingPointerRef, set
hudDragStartRef.current = null, set isHudDraggingRef.current = false and call
setIsHudDragging(false). Use the same math/logic as your pointer-move handler to
compute the offset so the final frame isn't dropped.
- Around line 92-103: The clamp currently uses window.screen dimensions which
can exceed the overlay's visible work area; in useHudBarDrag replace the
window.screen width/height usage when computing viewportWidth/viewportHeight
with the actual overlay viewport bounds used by the HUD (for example the
overlay's width/height or bounding rect available in latestDragState or the
overlayRect passed into the hook), and then use those overlay dimensions in the
clampedLeft/clampedTop calculations so the HUD cannot be dragged outside the
overlay's visible area.
In `@src/components/launch/hooks/useLaunchWindowSystemState.ts`:
- Around line 98-102: The chooseRecordingsDirectory callback can reject and
cause unhandled promise rejections when invoked with void (e.g., from
MorePopover); wrap the await call in a try/catch inside useCallback (function
chooseRecordingsDirectory) and handle/log errors locally (e.g., processLogger or
console.error) and return early on error so callers do not need to await the
promise to avoid unhandled rejections; ensure you still respect result.canceled
and result.success/result.path logic inside the try block.
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 209-219: The UI exposes a live mic toggle that only flips local UI
state (microphoneEnabled via setMicrophoneEnabled) while audio capture
continues; remove or disable that control during active recording to avoid
misleading users. Update the RecordingControls usage so the mic toggle is either
hidden or passed a disabled prop when a recording is in progress (use the
existing paused/recording state—e.g., when paused === false and recording is
active), and ensure setMicrophoneEnabled is not called during active recording;
keep the control enabled only when the recorder can actually detach/reattach mic
input mid-session.
In `@src/components/launch/popovers/PopoverScaffold.tsx`:
- Around line 69-79: The prop type for trigger is too permissive for use with
PopoverTrigger asChild; change the trigger prop in the PopoverScaffold component
signature from ReactNode to a single ReactElement (e.g., ReactElement or
JSX.Element) and import ReactElement from React, making it non-nullable so only
a single element can be passed; update any callers if necessary to wrap plain
strings/fragments in an element to satisfy the new type.
In `@src/components/launch/popovers/SourcePopover.tsx`:
- Around line 80-83: The popover is closing immediately because
onSourceSelect(source) is invoked with void, discarding rejections; change the
handler in SourcePopover so it awaits the promise returned by
onSourceSelect(source) and only calls requestClose(POPOVER_ID) after that await
resolves successfully (and do not close on rejection); reference the
onSourceSelect callback and requestClose(POPOVER_ID) in the event handler to
ensure the popover stays open if the source switch fails.
In `@src/components/ui/separator.tsx`:
- Around line 19-25: The divider currently renders a plain <div> and lacks
accessibility semantics; update the separator component (the element created
where className uses cn and the orientation prop is checked) to include
role="separator" and set aria-orientation to the current orientation value so
screen readers understand its purpose (use the existing orientation prop value
for aria-orientation); ensure any other props like className are preserved and
that orientation remains used for visual classes as before.
---
Nitpick comments:
In `@src/App.tsx`:
- Around line 17-48: The effect in useEffect (which calls setWindowType and
loadAllCustomFonts) mutates document.documentElement.dataset.windowType,
documentElement/body styles and classes and `#root` styles but never reverts them;
update the useEffect to capture previous values (dataset.windowType,
documentElement/body/background and overflow, and root element styles and class
presence) and return a cleanup function that restores those previous values,
removes the "hud-overlay-window" class if added, and clears dataset.windowType
(or restores the prior value); also ensure any ongoing loadAllCustomFonts
promise is handled (e.g., track a cancelled flag) to avoid applying font
side-effects after unmount.
In `@src/components/launch/hooks/useRecordingTimer.ts`:
- Line 10: The timer variable in useRecordingTimer.ts is typed as NodeJS.Timeout
| null which is incorrect for browser-only React code; change the declaration of
timer (used with setInterval/clearInterval inside the hook) to use
ReturnType<typeof setInterval> | null so it resolves to the DOM number type and
is renderer-safe, then ensure usages in start/stop logic keep the same null
checks and pass the timer value to clearInterval correctly.
In `@src/components/launch/popovers/WebcamPopover.tsx`:
- Around line 104-123: The repeated selection expression inside the
videoDevices.map for DropdownItem should be extracted to a local boolean to
improve readability: compute a const (e.g., isSelected) right before rendering
each <DropdownItem> using the existing symbols webcamEnabled, webcamDeviceId,
selectedVideoDeviceId and device.deviceId, then use that variable for both the
icon conditional and the selected prop, leaving onSelectVideoDevice unchanged.
In `@src/components/launch/SourceSelector.css`:
- Around line 114-115: The marquee animation defined by animation:
source-selector-marquee (and related rules around the hover-triggered marquee)
doesn't respect users' prefers-reduced-motion setting; add a
prefers-reduced-motion media query that disables the animation (e.g., set
animation: none and animation-play-state: paused or remove transform/animation
properties) for the selector(s) using source-selector-marquee so the marquee and
hover-started animation are paused when prefers-reduced-motion: reduce is set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a1cc1a3f-7397-4715-bf92-3a79c07446e9
⛔ Files ignored due to path filters (3)
electron/native/bin/win32-x64/cursor-monitor.exeis excluded by!**/*.exeelectron/native/bin/win32-x64/wgc-capture.exeis excluded by!**/*.exepackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
electron/electron-env.d.tselectron/ipc/register/sources.tselectron/native/bin/win32-x64/helpers-manifest.jsonelectron/preload.tselectron/windows.tssrc/App.tsxsrc/components/launch/LaunchWindow.module.csssrc/components/launch/LaunchWindow.tsxsrc/components/launch/RecordingControls.tsxsrc/components/launch/SourceSelector.csssrc/components/launch/SourceSelector.tsxsrc/components/launch/hooks/useHudBarDrag.tssrc/components/launch/hooks/useLaunchHudInteractionState.tssrc/components/launch/hooks/useLaunchWindowActions.tssrc/components/launch/hooks/useLaunchWindowSystemState.tssrc/components/launch/hooks/useRecordingTimer.tssrc/components/launch/hooks/useWebcamPreviewOverlay.tssrc/components/launch/launchTheme.csssrc/components/launch/popovers/CountdownPopover.tsxsrc/components/launch/popovers/LaunchPopoverCoordinator.tsxsrc/components/launch/popovers/MicPopover.tsxsrc/components/launch/popovers/MorePopover.tsxsrc/components/launch/popovers/PopoverScaffold.tsxsrc/components/launch/popovers/SourcePopover.tsxsrc/components/launch/popovers/WebcamPopover.tsxsrc/components/launch/popovers/launchPopoverTypes.tssrc/components/ui/button.tsxsrc/components/ui/popover.tsxsrc/components/ui/separator.tsxsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/pt-BR/launch.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-TW/launch.jsonsrc/index.css
Comment on lines
+209
to
+219
| <RecordingControls | ||
| paused={paused} | ||
| microphoneEnabled={microphoneEnabled} | ||
| elapsed={elapsed} | ||
| onToggleMicrophone={() => setMicrophoneEnabled(!microphoneEnabled)} | ||
| onPauseResume={paused ? resumeRecording : pauseRecording} | ||
| onStopRecording={toggleRecording} | ||
| onHideHud={() => window.electronAPI?.hudOverlayHide?.()} | ||
| onCancelRecording={cancelRecording} | ||
| formatTime={formatTime} | ||
| /> |
Contributor
There was a problem hiding this comment.
Do not expose a live mic toggle until it affects the recording pipeline.
The PR already calls out that this only flips UI state during recording while audio capture continues. That is misleading in the recording HUD and can create a privacy expectation mismatch. Until the recorder can actually detach/reattach mic input mid-session, this control should be disabled or removed in recording mode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/LaunchWindow.tsx` around lines 209 - 219, The UI
exposes a live mic toggle that only flips local UI state (microphoneEnabled via
setMicrophoneEnabled) while audio capture continues; remove or disable that
control during active recording to avoid misleading users. Update the
RecordingControls usage so the mic toggle is either hidden or passed a disabled
prop when a recording is in progress (use the existing paused/recording
state—e.g., when paused === false and recording is active), and ensure
setMicrophoneEnabled is not called during active recording; keep the control
enabled only when the recorder can actually detach/reattach mic input
mid-session.
Collaborator
|
Quick triage pass on the current head: I would not treat this as ready for maintainer review yet. The main blockers I see are:
Suggested next step: split or narrow this before another review pass. A good first slice would be either the source selector performance work, or the LaunchWindow extraction without binary/package-lock churn, plus targeted runtime smoke notes for screen/window source selection, recording start/stop/pause, mic/system toggles before recording, HUD dragging, and language switching. |
add: launch.json translation for that tooltip
… address the comments
Contributor
Author
|
Hello, I have made all the edits regarding the revert of the package.lock and manifest etc.. Standalone source selector is fixed too. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/components/launch/hooks/useLaunchWindowSystemState.ts (1)
18-24: 💤 Low valueMissing cancellation flag for recordings directory load.
This effect is inconsistent with other async effects in this file (lines 26-40, 42-58, 64-78, 80-96) which all use a
cancelledflag to prevent state updates after unmount. While not critical, aligning the pattern improves maintainability.♻️ Suggested fix to align with existing pattern
useEffect(() => { + let cancelled = false; const load = async () => { - const result = await window.electronAPI.getRecordingsDirectory(); - if (result.success) setRecordingsDirectory(result.path); + try { + const result = await window.electronAPI.getRecordingsDirectory(); + if (!cancelled && result.success) setRecordingsDirectory(result.path); + } catch (error) { + console.error("Failed to load recordings directory:", error); + } }; void load(); + return () => { + cancelled = true; + }; }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/hooks/useLaunchWindowSystemState.ts` around lines 18 - 24, The effect that calls window.electronAPI.getRecordingsDirectory in useEffect currently omits the cancelled flag pattern used elsewhere; modify this effect to declare a let cancelled = false, check cancelled before calling setRecordingsDirectory, and set cancelled = true in the cleanup function so load (the async function) does not update state after unmount (refer to the existing cancelled usage in the other effects for the exact pattern).src/components/launch/popovers/SourcePopover.tsx (1)
34-55: ⚡ Quick winDuplicated source mapping logic with
SourceSelector.tsx.The mapping logic here (lines 34-55) is nearly identical to
mapSourceinSourceSelector.tsx(lines 216-238). Extract this into a shared utility to ensure consistent behavior and avoid maintenance burden.💡 Example extraction
Create a shared utility, e.g., in
launchPopoverTypes.ts:export function mapRawSource(s: RawDesktopSource): DesktopSource { const isWindow = s.id.startsWith("window:"); const type = s.sourceType ?? (isWindow ? "window" : "screen"); let displayName = s.name; let appName = s.appName; if (isWindow && !appName && s.name.includes(" — ")) { const parts = s.name.split(" — "); appName = parts[0]?.trim(); displayName = parts.slice(1).join(" — ").trim() || s.name; } else if (isWindow && s.windowTitle) { displayName = s.windowTitle; } return { id: s.id, name: displayName, thumbnail: s.thumbnail, display_id: s.display_id, appIcon: s.appIcon, sourceType: type, appName, windowTitle: s.windowTitle ?? displayName, }; }Then import and use in both files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/popovers/SourcePopover.tsx` around lines 34 - 55, The mapping logic duplicated between SourcePopover.tsx and the mapSource function in SourceSelector.tsx should be extracted to a shared utility; create a function (e.g., mapRawSource or mapSourceRaw) that accepts a RawDesktopSource and returns a DesktopSource using the existing logic (isWindow detection, sourceType fallback, appName/displayName/windowTitle normalization, and copying thumbnail/appIcon/display_id/id), export it from a new shared module (e.g., launchPopoverTypes or a utils file) and replace the inline mapping in SourcePopover.tsx and the mapSource implementation in SourceSelector.tsx with imports that call the new shared function so both components use the same canonical mapping.src/components/launch/SourceSelector.tsx (2)
216-238: 💤 Low valueConsider adding a type for the raw Electron source response.
Using
anyhere loses type safety for the incoming Electron API response. A minimal interface for the raw source shape would catch mismatches early.💡 Suggested type
+interface RawDesktopSource { + id: string; + name: string; + thumbnail: string | null; + display_id: string; + appIcon?: string | null; + sourceType?: "screen" | "window"; + appName?: string; + windowTitle?: string; +} + -const mapSource = useCallback((s: any): DesktopSource => { +const mapSource = useCallback((s: RawDesktopSource): DesktopSource => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/SourceSelector.tsx` around lines 216 - 238, The mapSource callback currently types its parameter as any which loses safety; define a minimal interface like RawDesktopSource capturing the expected properties (id, sourceType?, name?, appName?, windowTitle?, thumbnail?, display_id?, appIcon?) and replace the any with RawDesktopSource in the useCallback signature and any local usages; ensure optional fields are marked optional to match Electron responses and update imports/exports if you place the interface in a shared types file, leaving the rest of mapSource logic unchanged.
293-308: 💤 Low valueMinor: First open may trigger redundant fetch after prefetch.
If the user hovers (triggering prefetch) and then immediately opens, both
prefetchSourcesand theuseEffecton line 304-308 callonFetchSources. Consider adding a short debounce or tracking fetch-in-progress to avoid the duplicate call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/SourceSelector.tsx` around lines 293 - 308, The prefetch flow can call onFetchSources twice if prefetchSources sets hasPrefetchedRef.current then the open effect also calls onFetchSources; update the logic to avoid duplicate calls by tracking fetch-in-progress or checking the hasPrefetchedRef flag before invoking onFetchSources in the open effect: add a fetchingRef (or reuse hasPrefetchedRef) that is set when starting onFetchSources in prefetchSources and have the useEffect that runs when open becomes true skip calling onFetchSources if the fetch is already in progress or hasPrefetchedRef.current is true; ensure both prefetchSources and the useEffect update/consult the same ref(s) so only one fetch request runs (references: hasPrefetchedRef, prefetchSources, onFetchSources, useEffect, open).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/launch/RecordingControls.tsx`:
- Around line 61-69: The disabled microphone Button in RecordingControls still
passes the onToggleMicrophone handler (button element with props
microphoneEnabled, disabled, onToggleMicrophone); remove the onToggleMicrophone
prop from that always-disabled Button so clicks cannot trigger the UI-only mic
toggle path during recording, leaving the variant, size, iconSize, className and
disabled props intact.
- Around line 61-124: Add explicit localized aria-label props to the icon-only
Button components so screen readers get a reliable name; for the microphone
toggle Button (uses microphoneEnabled and onToggleMicrophone) add
aria-label={t("recording.microphoneToggle")} (or similar localized key), for the
pause/resume Button (uses paused and onPauseResume) add aria-label based on
paused like aria-label={paused ? t("recording.resume") : t("recording.pause")},
and for the stop, hide HUD, and cancel Buttons (onStopRecording, onHideHud,
onCancelRecording) add aria-labels using their respective localized strings
(e.g. t("recording.stop"), t("recording.hideHud"), t("recording.cancel")); keep
titles as-is but ensure each Button has the matching aria-label prop.
In `@src/components/launch/SourceSelector.tsx`:
- Around line 311-315: Effect re-opens the popover whenever internalOpen changes
in autonomous mode; change the logic in SourceSelector so the effect only sets
internalOpen when autonomous mode first becomes active (or on mount) instead of
on every internalOpen change. Specifically, update the useEffect that references
isAutonomous/internalOpen to remove internalOpen from the dependency list and/or
gate the setInternalOpen call with a one-time flag (e.g., hasInitializedRef) so
that setInternalOpen(true) runs only when isAutonomous transitions true (or on
initial mount) and not when the user closes the popover via onOpenChange; keep
references to SourceSelector, isAutonomous, internalOpen, setInternalOpen, and
onOpenChange to locate the code to modify.
---
Nitpick comments:
In `@src/components/launch/hooks/useLaunchWindowSystemState.ts`:
- Around line 18-24: The effect that calls
window.electronAPI.getRecordingsDirectory in useEffect currently omits the
cancelled flag pattern used elsewhere; modify this effect to declare a let
cancelled = false, check cancelled before calling setRecordingsDirectory, and
set cancelled = true in the cleanup function so load (the async function) does
not update state after unmount (refer to the existing cancelled usage in the
other effects for the exact pattern).
In `@src/components/launch/popovers/SourcePopover.tsx`:
- Around line 34-55: The mapping logic duplicated between SourcePopover.tsx and
the mapSource function in SourceSelector.tsx should be extracted to a shared
utility; create a function (e.g., mapRawSource or mapSourceRaw) that accepts a
RawDesktopSource and returns a DesktopSource using the existing logic (isWindow
detection, sourceType fallback, appName/displayName/windowTitle normalization,
and copying thumbnail/appIcon/display_id/id), export it from a new shared module
(e.g., launchPopoverTypes or a utils file) and replace the inline mapping in
SourcePopover.tsx and the mapSource implementation in SourceSelector.tsx with
imports that call the new shared function so both components use the same
canonical mapping.
In `@src/components/launch/SourceSelector.tsx`:
- Around line 216-238: The mapSource callback currently types its parameter as
any which loses safety; define a minimal interface like RawDesktopSource
capturing the expected properties (id, sourceType?, name?, appName?,
windowTitle?, thumbnail?, display_id?, appIcon?) and replace the any with
RawDesktopSource in the useCallback signature and any local usages; ensure
optional fields are marked optional to match Electron responses and update
imports/exports if you place the interface in a shared types file, leaving the
rest of mapSource logic unchanged.
- Around line 293-308: The prefetch flow can call onFetchSources twice if
prefetchSources sets hasPrefetchedRef.current then the open effect also calls
onFetchSources; update the logic to avoid duplicate calls by tracking
fetch-in-progress or checking the hasPrefetchedRef flag before invoking
onFetchSources in the open effect: add a fetchingRef (or reuse hasPrefetchedRef)
that is set when starting onFetchSources in prefetchSources and have the
useEffect that runs when open becomes true skip calling onFetchSources if the
fetch is already in progress or hasPrefetchedRef.current is true; ensure both
prefetchSources and the useEffect update/consult the same ref(s) so only one
fetch request runs (references: hasPrefetchedRef, prefetchSources,
onFetchSources, useEffect, open).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f9e7556-afbd-4f2f-a020-1675c9b2c5f0
📒 Files selected for processing (15)
src/components/launch/RecordingControls.tsxsrc/components/launch/SourceSelector.tsxsrc/components/launch/hooks/useHudBarDrag.tssrc/components/launch/hooks/useLaunchWindowSystemState.tssrc/components/launch/popovers/PopoverScaffold.tsxsrc/components/launch/popovers/SourcePopover.tsxsrc/components/ui/separator.tsxsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/pt-BR/launch.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-TW/launch.json
✅ Files skipped from review due to trivial changes (10)
- src/i18n/locales/en/launch.json
- src/i18n/locales/nl/launch.json
- src/i18n/locales/ko/launch.json
- src/i18n/locales/pt-BR/launch.json
- src/i18n/locales/zh-CN/launch.json
- src/components/ui/separator.tsx
- src/i18n/locales/es/launch.json
- src/components/launch/popovers/PopoverScaffold.tsx
- src/i18n/locales/fr/launch.json
- src/components/launch/hooks/useHudBarDrag.ts
Comment on lines
+61
to
+69
| <span title={t("recording.micToggleDisabledTip")}> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| iconSize="lg" | ||
| className={microphoneEnabled ? styles.ibActive : ""} | ||
| disabled | ||
| onClick={onToggleMicrophone} | ||
| > |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Remove onToggleMicrophone from the always-disabled mic button.
Line 67 hard-disables this control, but Line 68 still wires the click handler. Removing the handler avoids accidentally reintroducing the UI-only mic toggle path in recording mode.
Suggested diff
<span title={t("recording.micToggleDisabledTip")}>
<Button
variant="ghost"
size="icon"
iconSize="lg"
className={microphoneEnabled ? styles.ibActive : ""}
disabled
- onClick={onToggleMicrophone}
+ aria-label={t("recording.microphone")}
>📝 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
| <span title={t("recording.micToggleDisabledTip")}> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| iconSize="lg" | |
| className={microphoneEnabled ? styles.ibActive : ""} | |
| disabled | |
| onClick={onToggleMicrophone} | |
| > | |
| <span title={t("recording.micToggleDisabledTip")}> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| iconSize="lg" | |
| className={microphoneEnabled ? styles.ibActive : ""} | |
| disabled | |
| aria-label={t("recording.microphone")} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/RecordingControls.tsx` around lines 61 - 69, The
disabled microphone Button in RecordingControls still passes the
onToggleMicrophone handler (button element with props microphoneEnabled,
disabled, onToggleMicrophone); remove the onToggleMicrophone prop from that
always-disabled Button so clicks cannot trigger the UI-only mic toggle path
during recording, leaving the variant, size, iconSize, className and disabled
props intact.
Contributor
Author
|
Edited to conform to CodeRabbits suggestions |
Contributor
Author
|
I also fixed the nitpick comments which were right :) |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/launch/SourceSelector.tsx`:
- Around line 27-28: The component is currently tracking selection by the
presentational label which mapSource() can collapse; change the selection to use
the stable source.id instead: update the selectedSource prop/state type and
initializers in SourceSelector to hold the source.id (string) instead of the
display name, update any handlers like the click/select event and functions that
call setSelectedSource to pass source.id, and update render logic that compares
selectedSource to each row to compare against source.id (keeping source.name
purely for display). Also audit and adjust any props/consumers (e.g., functions
that read selectedSource or compute highlighted rows) to accept and compare the
id rather than the mapped name so duplicates no longer conflict.
- Line 85: The default label "Screen" is hardcoded in SourceSelector (the
selectedSource default and the other fallback) so the trigger shows English;
replace both hardcoded "Screen" values with the translated launch string (e.g.,
use the component's translation helper like t('launch.screen') or
launchStrings.screen) so the selectedSource default and the fallback display the
localized label instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3bec3f68-ea71-4c41-b27e-cd6c60cbd62e
📒 Files selected for processing (2)
src/components/launch/RecordingControls.tsxsrc/components/launch/SourceSelector.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/launch/RecordingControls.tsx
Comment on lines
+27
to
+28
| /** Currently selected source name */ | ||
| selectedSource?: string; |
Contributor
There was a problem hiding this comment.
Track selection by source.id, not the display label.
mapSource() intentionally collapses some window names to a friendlier label, so two different windows can end up sharing the same name. Because the selected state and row highlight are keyed off that label, duplicate titles can render as simultaneously selected and a refresh can re-bind the highlight to the wrong source. Store a stable identifier (source.id) for selection and keep name purely presentational.
Also applies to: 90-91, 221-224, 271-271
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/SourceSelector.tsx` around lines 27 - 28, The component
is currently tracking selection by the presentational label which mapSource()
can collapse; change the selection to use the stable source.id instead: update
the selectedSource prop/state type and initializers in SourceSelector to hold
the source.id (string) instead of the display name, update any handlers like the
click/select event and functions that call setSelectedSource to pass source.id,
and update render logic that compares selectedSource to each row to compare
against source.id (keeping source.name purely for display). Also audit and
adjust any props/consumers (e.g., functions that read selectedSource or compute
highlighted rows) to accept and compare the id rather than the mapped name so
duplicates no longer conflict.
| export const SourceSelectorContent = ({ | ||
| screenSources = [], | ||
| windowSources = [], | ||
| selectedSource = "Screen", |
Contributor
There was a problem hiding this comment.
Localize the default selected-source label.
Both fallbacks hardcode "Screen", so autonomous mode shows English text in the trigger before the user selects anything. Reuse the translated launch string here as well.
Also applies to: 206-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/SourceSelector.tsx` at line 85, The default label
"Screen" is hardcoded in SourceSelector (the selectedSource default and the
other fallback) so the trigger shows English; replace both hardcoded "Screen"
values with the translated launch string (e.g., use the component's translation
helper like t('launch.screen') or launchStrings.screen) so the selectedSource
default and the fallback display the localized label instead.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/launch/popovers/launchPopoverTypes.ts (1)
1-21: 💤 Low value
DesktopSourceandRawDesktopSourceare structurally identical.If these types are meant to diverge in the future, consider adding a comment explaining the distinction. Otherwise, a single type or type alias would reduce maintenance burden.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/popovers/launchPopoverTypes.ts` around lines 1 - 21, DesktopSource and RawDesktopSource are identical; consolidate them to avoid duplication by removing one and using a single shared type (e.g., keep DesktopSource and make RawDesktopSource a type alias to DesktopSource or delete RawDesktopSource and replace its usages with DesktopSource), or if you intend to keep both, add a clarifying comment above the DesktopSource/RawDesktopSource declaration explaining why they must remain distinct and what future differences are expected; update all references to use the chosen single symbol (DesktopSource or RawDesktopSource alias).src/components/launch/popovers/SourcePopover.tsx (1)
41-48: 💤 Low valueConsider consolidating source-type filtering logic.
The same
sourceType === "screen"/id.startsWith("screen:")filter pattern is duplicated inSourceSelector.tsx(lines 246-252). If the heuristic changes, both locations must be updated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/popovers/SourcePopover.tsx` around lines 41 - 48, Duplicate filtering logic for screen/window sources should be extracted into shared predicate helpers (e.g., isScreenSource and isWindowSource). Add these functions to a common util or a shared component module and replace the inline filters in SourcePopover (screenSources/windowSources) and the corresponding filter in SourceSelector (the screen/window filter block around lines 246-252) to call the new predicates so future heuristic changes are made in one place.src/components/launch/SourceSelector.tsx (1)
303-311: 💤 Low valuePrefer a narrower type than
anyfor cloned element props.The cast to
React.ReactElement<any>suppresses type checking. A minimal fix is to use a more constrained type.♻️ Suggested type refinement
- React.cloneElement(children as React.ReactElement<any>, { + React.cloneElement(children as React.ReactElement<{ onPointerEnter?: () => void; onFocusCapture?: () => void }>, { onPointerEnter: prefetchSources, onFocusCapture: prefetchSources, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/SourceSelector.tsx` around lines 303 - 311, The cloned element is cast to React.ReactElement<any>, which disables prop type checking; change the cast to a narrower prop type that includes the event handlers you set (e.g., React.ReactElement<React.DOMAttributes<any>> or React.ReactElement<Record<string, unknown>>), so replace the React.cloneElement(children as React.ReactElement<any>, {...}) with React.cloneElement(children as React.ReactElement<React.DOMAttributes<any>>, { onPointerEnter: prefetchSources, onFocusCapture: prefetchSources }) (or another appropriate DOM/event props type) to preserve type safety for trigger/children and the prefetchSources handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/launch/popovers/launchPopoverTypes.ts`:
- Around line 28-34: When determining displayName in the launch popover logic,
s.windowTitle is currently ignored whenever s.name contains " — " because the
isWindow && s.name.includes(" — ") branch runs first; change the condition order
so that the isWindow && s.windowTitle check runs before parsing s.name. In
practice, update the logic around isWindow, s.windowTitle, s.name, appName and
displayName to first set displayName = s.windowTitle if isWindow and
s.windowTitle exist, otherwise fall back to splitting s.name on " — " and
extracting appName/displayName as currently implemented.
---
Nitpick comments:
In `@src/components/launch/popovers/launchPopoverTypes.ts`:
- Around line 1-21: DesktopSource and RawDesktopSource are identical;
consolidate them to avoid duplication by removing one and using a single shared
type (e.g., keep DesktopSource and make RawDesktopSource a type alias to
DesktopSource or delete RawDesktopSource and replace its usages with
DesktopSource), or if you intend to keep both, add a clarifying comment above
the DesktopSource/RawDesktopSource declaration explaining why they must remain
distinct and what future differences are expected; update all references to use
the chosen single symbol (DesktopSource or RawDesktopSource alias).
In `@src/components/launch/popovers/SourcePopover.tsx`:
- Around line 41-48: Duplicate filtering logic for screen/window sources should
be extracted into shared predicate helpers (e.g., isScreenSource and
isWindowSource). Add these functions to a common util or a shared component
module and replace the inline filters in SourcePopover
(screenSources/windowSources) and the corresponding filter in SourceSelector
(the screen/window filter block around lines 246-252) to call the new predicates
so future heuristic changes are made in one place.
In `@src/components/launch/SourceSelector.tsx`:
- Around line 303-311: The cloned element is cast to React.ReactElement<any>,
which disables prop type checking; change the cast to a narrower prop type that
includes the event handlers you set (e.g.,
React.ReactElement<React.DOMAttributes<any>> or
React.ReactElement<Record<string, unknown>>), so replace the
React.cloneElement(children as React.ReactElement<any>, {...}) with
React.cloneElement(children as React.ReactElement<React.DOMAttributes<any>>, {
onPointerEnter: prefetchSources, onFocusCapture: prefetchSources }) (or another
appropriate DOM/event props type) to preserve type safety for trigger/children
and the prefetchSources handlers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ed80553-792d-422f-99b8-45329dc85637
📒 Files selected for processing (4)
src/components/launch/SourceSelector.tsxsrc/components/launch/hooks/useLaunchWindowSystemState.tssrc/components/launch/popovers/SourcePopover.tsxsrc/components/launch/popovers/launchPopoverTypes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/launch/hooks/useLaunchWindowSystemState.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/launch/SourceSelector.tsx (1)
22-23:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftStore a stable source id, and derive the label separately.
mapRawSource()can collapse multiple windows to the same visible title, so usingsource.nameas the selected value makes duplicate rows highlight together and lets refreshes re-bind the selection to the wrong item. It also forces the autonomous fallback to persist the English"Screen"sentinel as state. Keepsource.idin props/state, compare selection againstsource.id, and compute the trigger label from the selected source or the translated screen fallback instead.Suggested direction
-interface SourceSelectorProps { - /** Currently selected source name */ - selectedSource?: string; +interface SourceSelectorProps { + /** Currently selected source id */ + selectedSourceId?: string;-const isSelected = selectedSource === source.name; +const isSelected = selectedSourceId === source.id;-const [internalSelectedSource, setInternalSelectedSource] = useState("Screen"); +const [internalSelectedSourceId, setInternalSelectedSourceId] = useState<string | undefined>(); ... -if (result) { - setInternalSelectedSource(source.name); -} +if (result) { + setInternalSelectedSourceId(source.id); +}Then derive the visible label from the selected source object, with
t("recording.screen")as the untranslated fallback.Also applies to: 80-86, 201-208, 239-242, 328-332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/SourceSelector.tsx` around lines 22 - 23, The component currently stores and compares selectedSource by visible name which causes duplicate titles to collide; change selectedSource to store the stable source.id (update prop/interface and state where selectedSource?: string is declared), update all selection comparisons to compare against source.id (references: mapRawSource, the selection handlers and render rows), and compute the visible trigger label by finding the selected source object and using its name or falling back to t("recording.screen") (untranslated sentinel) when not found; ensure any places that assumed the name (e.g., the trigger button label and autonomous fallback logic) derive the label from the source object instead of persisting the English "Screen" string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/launch/SourceSelector.tsx`:
- Around line 81-83: The empty-block default callback in the SourceSelector
component signature should be replaced: in the parameter destructuring for
SourceSelector change the onSourceSelect default from () => {} to an
expression-body noop like () => undefined, or remove the default and make
onSourceSelect optional in the Pick type and guard calls where onSourceSelect is
invoked; update the Pick<SourceSelectorProps, "...onSourceSelect"> usage and any
call sites inside the SourceSelector implementation to handle a possibly
undefined onSourceSelect.
---
Duplicate comments:
In `@src/components/launch/SourceSelector.tsx`:
- Around line 22-23: The component currently stores and compares selectedSource
by visible name which causes duplicate titles to collide; change selectedSource
to store the stable source.id (update prop/interface and state where
selectedSource?: string is declared), update all selection comparisons to
compare against source.id (references: mapRawSource, the selection handlers and
render rows), and compute the visible trigger label by finding the selected
source object and using its name or falling back to t("recording.screen")
(untranslated sentinel) when not found; ensure any places that assumed the name
(e.g., the trigger button label and autonomous fallback logic) derive the label
from the source object instead of persisting the English "Screen" string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a5dc4a6-f3f3-472b-81c0-9ac314f6b761
📒 Files selected for processing (3)
src/components/launch/SourceSelector.tsxsrc/components/launch/popovers/SourcePopover.tsxsrc/components/launch/popovers/launchPopoverTypes.ts
Comment on lines
+81
to
+83
| loading = false, | ||
| onSourceSelect = () => {}, | ||
| }: Pick<SourceSelectorProps, "screenSources" | "windowSources" | "selectedSource" | "loading" | "onSourceSelect">) => { |
Contributor
There was a problem hiding this comment.
Avoid the empty default callback here.
Biome flags this initializer, so the file will fail lint as written. Use an expression-body noop or make the call site optional instead.
Minimal fix
- onSourceSelect = () => {},
+ onSourceSelect = () => undefined,📝 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
| loading = false, | |
| onSourceSelect = () => {}, | |
| }: Pick<SourceSelectorProps, "screenSources" | "windowSources" | "selectedSource" | "loading" | "onSourceSelect">) => { | |
| loading = false, | |
| onSourceSelect = () => undefined, | |
| }: Pick<SourceSelectorProps, "screenSources" | "windowSources" | "selectedSource" | "loading" | "onSourceSelect">) => { |
🧰 Tools
🪛 Biome (2.4.14)
[error] 82-82: Unexpected empty block.
(lint/suspicious/noEmptyBlockStatements)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/SourceSelector.tsx` around lines 81 - 83, The
empty-block default callback in the SourceSelector component signature should be
replaced: in the parameter destructuring for SourceSelector change the
onSourceSelect default from () => {} to an expression-body noop like () =>
undefined, or remove the default and make onSourceSelect optional in the Pick
type and guard calls where onSourceSelect is invoked; update the
Pick<SourceSelectorProps, "...onSourceSelect"> usage and any call sites inside
the SourceSelector implementation to handle a possibly undefined onSourceSelect.
Collaborator
|
Seems to make the icons black, could you please revert it to make it more visible? |
…o avoid icons being wrong color, text too etc..
fix: issues with HUD bar not registering clicks properly
fix: mouse events passthrough
…e passthrough and stuff
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hello, this is my first contribution on Recordly.
Summary :
While using Recordly, I had a lot of issues with how the source selection was, it was very jittery, every time you opened it, it completly broke, you never know where it would end up, sometimes opening a popover would put the HUD like all the way down your screen..
So I digged, and I found a component of over 1600 lines (LaunchWindow), which also happens to be this exact HUD.
What I did
Refactoring/Performance
UI
ui/seperator.tsx, this is essentially a Divider component from UI libraries. It was used in LaunchWindow and I think could be useful for other parts of the UI. It supports vertical and horizontal orientation.button.tsxto remove completly the use of a hardcoded IconButton in the LaunchWindow (over 15 occurences) it supports various sizes : default/sm/lg/xl.popover.tsxas well as unstyled option to be able to style the popover however we want, this was made to match the HUD style.launchTheme.cssfor grouping all the style variables used in all popovers and variouslaunch/componentsTests I did
I tested recording a video, with Camera and Microphone, everything worked normally. Recording Screen or single window worked. Changing languages works...
I may not know all the edge cases for now, so please test it out too if possible :)
Additional notes
I found a bug during my testing. Microphone can be stopped visually in the UI, but the Engine does not support blocking the microphone during a recording. I thought it was my fault, but I never changed that.
Codex has that to say :
Why it happens
UI.
start.
native IPC toggle is called (native path).
Thanks,
Binoss.
Copilot generated summary :
This pull request introduces several important improvements and refactorings to the Electron app, focusing on HUD overlay window management, desktop source listing, and UI styling. The main changes include a significant simplification and modernization of the HUD overlay sizing logic, improved caching for desktop source enumeration, and enhanced styling for the launch window dropdown menus.
HUD Overlay Window Refactor and Behavior Changes:
Desktop Source Enumeration and Caching:
registerSourceHandlers, improving performance by avoiding redundant enumeration calls within a short TTL. Also added an option to include or exclude window icons, and refactored related logic for clarity and efficiency. [1] [2] [3] [4] [5] [6]UI and Styling Enhancements:
.menuCardclass now uses backdrop blur and more modern visual effects. [1] [2] [3]Window Type Handling in Renderer:
data-window-typeattribute and adds specific classes for the HUD overlay window, enabling type-specific styling and behavior. [1] [2]These changes collectively modernize the HUD overlay experience, improve performance for source selection, and lay groundwork for further UI theming and customization.
Summary by CodeRabbit
New Features
Performance / Bug Fixes
UI / Style
Refactor
Localization