Revert HUD passthrough and Linux fallback changes#656
Open
webadderall wants to merge 5 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoves ChangesFeature removal: nativeCaptureUnavailable, webcam HUD sync, and source mapping simplification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/launch/hooks/useLaunchHudInteractionState.ts (1)
42-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the HUD interactive while a popover is open.
On Line 45 and Line 84, the ignore-mouse timers still arm even when
openIdis non-null. That lets an already-open HUD menu/popover flip the overlay back to click-through mid-interaction.Suggested fix
if (isInteractive) { isMouseOverHudRef.current = true; if (timeoutRef.current) clearTimeout(timeoutRef.current); window.electronAPI?.hudOverlaySetIgnoreMouse?.(false); - } else { + } else if (openId === null) { isMouseOverHudRef.current = false; if (timeoutRef.current) clearTimeout(timeoutRef.current); timeoutRef.current = setTimeout(() => { if ( + openId === null && !isHudDraggingRef.current && !isWebcamPreviewDraggingRef.current && !webcamPreviewDragStartRef.current && !isMouseOverHudRef.current ) { window.electronAPI?.hudOverlaySetIgnoreMouse?.(true); } }, 300); } }; window.addEventListener("mouseover", handleMouseOver); return () => window.removeEventListener("mouseover", handleMouseOver); - }, [isHudDraggingRef, isWebcamPreviewDraggingRef, webcamPreviewDragStartRef]); + }, [openId, isHudDraggingRef, isWebcamPreviewDraggingRef, webcamPreviewDragStartRef]); ... timeoutRef.current = setTimeout(() => { if ( + openId === null && !isHudDraggingRef.current && !isWebcamPreviewDraggingRef.current && !webcamPreviewDragStartRef.current && !isMouseOverHudRef.current ) { window.electronAPI?.hudOverlaySetIgnoreMouse?.(true); } }, 300); }, - [isHudDraggingRef, isWebcamPreviewDraggingRef, webcamPreviewDragStartRef], + [openId, isHudDraggingRef, isWebcamPreviewDraggingRef, webcamPreviewDragStartRef], );Also applies to: 84-93
🤖 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/useLaunchHudInteractionState.ts` around lines 42 - 55, The timer callback that sets window.electronAPI.hudOverlaySetIgnoreMouse is firing even when a HUD popover/menu is open; update the logic in the hook (the blocks using isMouseOverHudRef, timeoutRef and setTimeout) to first check that openId is null/undefined before arming or executing the timeout callback — i.e., include a guard referencing openId (from the hook state/params) both before scheduling timeoutRef.current = setTimeout(...) and inside the timeout callback so that when openId is non-null the overlay is not switched to ignore-mouse; apply the same change to the second occurrence that uses isHudDraggingRef, isWebcamPreviewDraggingRef and webcamPreviewDragStartRef so popovers remain interactive.
🤖 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 `@electron/ipc/register/sources.ts`:
- Around line 127-129: Restore the Linux portal sentinel mapping when returning
screen rows from register/sources.ts: instead of emitting matchedSource?.id or
`screen:fallback:${displayId}` directly, map portal-like identifiers back to the
sentinel "screen:linux-portal" (or call the previous helper such as
normalizeScreenId or mapSourceToSentinel) so that after explicit selection
useScreenRecorder.ts still sees the portal-safe id; ensure both the
matchedSource present branch and the fallback branch normalize to the sentinel
when appropriate and keep the displayName unchanged.
---
Outside diff comments:
In `@src/components/launch/hooks/useLaunchHudInteractionState.ts`:
- Around line 42-55: The timer callback that sets
window.electronAPI.hudOverlaySetIgnoreMouse is firing even when a HUD
popover/menu is open; update the logic in the hook (the blocks using
isMouseOverHudRef, timeoutRef and setTimeout) to first check that openId is
null/undefined before arming or executing the timeout callback — i.e., include a
guard referencing openId (from the hook state/params) both before scheduling
timeoutRef.current = setTimeout(...) and inside the timeout callback so that
when openId is non-null the overlay is not switched to ignore-mouse; apply the
same change to the second occurrence that uses isHudDraggingRef,
isWebcamPreviewDraggingRef and webcamPreviewDragStartRef so popovers remain
interactive.
🪄 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: bddacc97-8290-4cb9-b3b1-d3556cbaedc5
📒 Files selected for processing (18)
electron/electron-env.d.tselectron/hudOverlayBounds.test.tselectron/hudOverlayBounds.tselectron/ipc/register/project.tselectron/ipc/register/settings.tselectron/ipc/register/sourceMapping.test.tselectron/ipc/register/sourceMapping.tselectron/ipc/register/sources.tselectron/ipc/types.tselectron/main.tselectron/preload.tselectron/windows.tssrc/components/launch/LaunchWindow.tsxsrc/components/launch/hooks/useLaunchHudInteractionState.tssrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/hooks/useScreenRecorder.test.tssrc/hooks/useScreenRecorder.ts
💤 Files with no reviewable changes (9)
- electron/ipc/register/sourceMapping.ts
- electron/ipc/register/sourceMapping.test.ts
- electron/ipc/types.ts
- src/components/launch/LaunchWindow.tsx
- electron/hudOverlayBounds.ts
- electron/hudOverlayBounds.test.ts
- src/components/video-editor/SettingsPanel.tsx
- electron/preload.ts
- src/hooks/useScreenRecorder.test.ts
Comment on lines
127
to
129
| return { | ||
| id: getScreenSourceIdForDisplay({ | ||
| displayId, | ||
| env: process.env, | ||
| matchedSourceId: matchedSource?.id, | ||
| platform: process.platform, | ||
| }), | ||
| id: matchedSource?.id ?? `screen:fallback:${displayId}`, | ||
| name: displayName, |
Contributor
There was a problem hiding this comment.
Preserve the Linux portal sentinel for screen entries.
Line 128 now emits raw matchedSource.id / screen:fallback:*, but the rest of this revert still only treats "screen:linux-portal" as the Linux portal-safe screen id. After an explicit screen selection, src/hooks/useScreenRecorder.ts stops taking the portal branch, and fallback ids are then rejected at Lines 1590-1596 while raw ids reintroduce the extra/stale portal path electron/main.ts is trying to avoid. Please restore the previous helper-style mapping here, or normalize Linux screen rows back to the sentinel before they leave this handler.
Suggested direction
- id: matchedSource?.id ?? `screen:fallback:${displayId}`,
+ id: getScreenSourceIdForDisplay({
+ platform: process.platform,
+ displayId,
+ matchedSource,
+ }),🤖 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 `@electron/ipc/register/sources.ts` around lines 127 - 129, Restore the Linux
portal sentinel mapping when returning screen rows from register/sources.ts:
instead of emitting matchedSource?.id or `screen:fallback:${displayId}`
directly, map portal-like identifiers back to the sentinel "screen:linux-portal"
(or call the previous helper such as normalizeScreenId or mapSourceToSentinel)
so that after explicit selection useScreenRecorder.ts still sees the portal-safe
id; ensure both the matchedSource present branch and the fallback branch
normalize to the sentinel when appropriate and keep the displayName unchanged.
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
1 participant
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
Validation
Summary by CodeRabbit
Improvements
Removed
Tests