Skip to content

fix(windows): hide native desktop cursor during browser capture#79

Open
LILQK wants to merge 2 commits intowebadderall:mainfrom
LILQK:codex/fix-windows-native-cursor-visibility
Open

fix(windows): hide native desktop cursor during browser capture#79
LILQK wants to merge 2 commits intowebadderall:mainfrom
LILQK:codex/fix-windows-native-cursor-visibility

Conversation

@LILQK
Copy link

@LILQK LILQK commented Mar 20, 2026

Summary

  • restore Windows browser-capture behavior so the native desktop cursor is hidden again
  • keep the custom cursor pipeline visible without the duplicated native cursor
  • switch browser fallback capture to getDisplayMedia constraints with cursor: "never"

Why

On Windows, recordings could show the OS cursor again, which caused a duplicate cursor when Recordly renders its own cursor layer. This brings back the previous expected behavior where only Recordly's cursor rendering is visible.

Notes

  • only src/hooks/useScreenRecorder.ts is included
  • AGENTS.md is intentionally not part of this PR

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling to provide clearer feedback when users deny or cancel screen recording requests.
    • Enhanced system audio capture functionality with better cross-platform compatibility for different operating systems.
    • Standardized cursor visibility behavior consistently across all screen recording operations.

@coderabbitai
Copy link

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9b20e0bd-9268-4a29-b3e5-c39caa3bd51b

📥 Commits

Reviewing files that changed from the base of the PR and between b8a6d85 and e82232f.

📒 Files selected for processing (1)
  • src/hooks/useScreenRecorder.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/hooks/useScreenRecorder.ts

📝 Walkthrough

Walkthrough

The pull request refactors display media capture in useScreenRecorder.ts by introducing shared constraint type definitions, implementing platform-specific capture paths for Windows and non-Windows systems, standardizing cursor behavior to "never", and adding explicit abort error handling to distinguish user-denied requests from other failures.

Changes

Cohort / File(s) Summary
Display Media Capture Refactoring
src/hooks/useScreenRecorder.ts
Added ExtendedDisplayMediaStreamOptions, ChromeDesktopVideoConstraints, and ChromeDesktopAudioConstraints types; replaced audio+video system capture with platform-specific paths using getUserMedia on Windows and getDisplayMedia on non-Windows; standardized cursor behavior to "never"; introduced explicit handling for NotAllowedError and AbortError to distinguish user aborts from failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Whiskers twitching with delight,
Display constraints now unified bright!
Windows and the rest play fair,
Cursor never shows with care—
Abort errors caught with care!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides a summary, motivation, and notes, but is missing several required template sections including Type of Change, Related Issue(s), Screenshots/Video, Testing Guide, and Checklist. Add the missing template sections: explicitly mark the Type of Change (Bug Fix), link to related issues if any, provide screenshots or videos demonstrating the fix, describe testing steps for Windows browser capture, and complete the pre-submission checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: hiding the native desktop cursor on Windows during browser capture, which directly aligns with the PR's primary objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can enable review details to help with troubleshooting, context usage and more.

Enable the reviews.review_details setting to include review details such as the model used, the time taken for each step and more in the review comments.

@webadderall
Copy link
Owner

PR removes system-audio capture path for Windows browser capture, please fix

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 577-583: The cursor:"never" constraint set in
displayMediaVideoConstraints gets lost when applyConstraints() is called; update
the applyConstraints() call on the videoTrack (in useScreenRecorder.ts) to
include cursor: "never" along with frameRate, width, and height (use the
existing TARGET_FRAME_RATE, TARGET_WIDTH, TARGET_HEIGHT constants) and cast to
MediaTrackConstraints so the cursor constraint persists.
- Around line 604-611: The catch block that handles audio capture errors in
useScreenRecorder (around the getDisplayMedia call producing screenMediaStream)
must not retry with audio: false when the user cancelled or denied the share
dialog; modify the catch for audioError to inspect audioError.name (or
error.code) and if it is "NotAllowedError" or "AbortError" (or clearly indicates
user denial/abort) then rethrow or return/exit instead of calling
navigator.mediaDevices.getDisplayMedia again; only perform the fallback to
video-only for other error names (non-user-denial/unrecoverable) so that user
cancellations don't trigger a second picker.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f4081559-ae2d-47e5-b1af-25ea30a3fcdb

📥 Commits

Reviewing files that changed from the base of the PR and between c1cb222 and b8a6d85.

📒 Files selected for processing (2)
  • -
  • src/hooks/useScreenRecorder.ts
💤 Files with no reviewable changes (1)

@LILQK LILQK force-pushed the codex/fix-windows-native-cursor-visibility branch from e82232f to b8a6d85 Compare March 22, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants