-
Notifications
You must be signed in to change notification settings - Fork 917
Macos tahoe cursor #1175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Macos tahoe cursor #1175
Conversation
WalkthroughAdds 20 Tahoe-specific macOS cursor variants to Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Browser UI (cursors.html)
participant Hash as Cursor Hash Input
participant FromHash as from_hash()
participant Resolve as resolve()
participant Assets as SVG assets / Hotspot data
Note over UI,Hash: User selects a cursor or preview loads
UI->>Hash: provide cursor hash/identifier
Hash->>FromHash: lookup hash -> CursorShapeMacOS variant
FromHash-->>UI: return variant (Tahoe or regular)
UI->>Resolve: request asset & hotspot for variant
Resolve-->>Assets: map variant -> (SVG path, hotspot) or None
Assets-->>UI: return asset path + hotspot (or no asset)
Note right of UI: render SVG preview and hotspot info
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
crates/cursor-info/assets/mac/tahoe/context-menu.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/crosshair.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/default.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/grab.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/grabbing.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/pointer.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/resize-e.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/resize-ew.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/resize-n.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/resize-ns.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/resize-s.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/resize-w.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/text.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/zoom-in.svg
is excluded by!**/*.svg
crates/cursor-info/assets/mac/tahoe/zoom-out.svg
is excluded by!**/*.svg
📒 Files selected for processing (1)
crates/cursor-info/src/macos.rs
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
For desktop IPC, use tauri_specta derive/macros on Rust types/events; do not hand-roll bindings
Files:
crates/cursor-info/src/macos.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
crates/cursor-info/src/macos.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/cursor-info/src/macos.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/
and/or a siblingtests/
directory for each crate insidecrates/*
.
Files:
crates/cursor-info/src/macos.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
This is the Video Demo of the cursorsBuilt-in.Retina.Display.2025-10-11.09.30.19.mp4 |
02f9d05
to
3d197d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/cursor-info/src/macos.rs (1)
46-66
: Enum variant order: Consider aligning ZoomIn/ZoomOut with resolve() method.The enum declares
TahoeZoomOut
beforeTahoeZoomIn
(lines 64-65), but theresolve()
method (lines 215-222) andfrom_hash()
method (lines 313-316) both order them as ZoomIn before ZoomOut. While this doesn't affect correctness, aligning the order improves readability and maintainability.Apply this diff to reorder the enum variants:
TahoeIBeamVerticalForVerticalLayout, - TahoeZoomOut, TahoeZoomIn, + TahoeZoomOut, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/cursor-info/cursors.html
(9 hunks)crates/cursor-info/src/macos.rs
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/cursor-info/src/macos.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/
and/or a siblingtests/
directory for each crate insidecrates/*
.
Files:
crates/cursor-info/src/macos.rs
⏰ 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: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (6)
crates/cursor-info/src/macos.rs (3)
158-165
: LGTM: Asset reuse is appropriate.The reuse of legacy assets for
TahoeDragCopy
,TahoeDragLink
,TahoeOperationNotAllowed
, andTahoeIBeamVerticalForVerticalLayout
is correct, as confirmed in the past review discussion. These cursors are visually identical between Aqua and Tahoe variants.Also applies to: 175-177, 211-214
265-316
: LGTM: Hash mappings are complete.All 19 Tahoe cursor variants have corresponding hash mappings derived from a macOS Tahoe system. The implementation follows the same pattern as the regular macOS cursors.
142-222
: All referenced Tahoe cursor assets are present and correctly located.Verification confirms that all 19 SVG assets referenced in the
resolve()
method exist in the repository:
- 15 Tahoe-specific assets in
crates/cursor-info/assets/mac/tahoe/
- 4 legacy assets reused in
crates/cursor-info/assets/mac/
No issues found.
crates/cursor-info/cursors.html (3)
1-1
: LGTM: HTML and CSS improvements.The changes include standard HTML5 doctype declaration, improved CSS sizing with explicit dimensions and
object-fit: contain
, and minor text formatting improvements. These are all appropriate non-functional enhancements.Also applies to: 53-77, 144-145
520-522
: LGTM: JavaScript formatting improvements.The consistent use of
toFixed(3)
for coordinate display and the addition of line breaks for readability are good improvements that don't change functionality.Also applies to: 540-542, 553-562, 570-572, 602-615
264-377
: ****Verification confirms no consistency issues. Both HTML and Rust contain exactly 19 Tahoe cursor entries with matching names (CamelCase variants in Rust correspond to snake_case names in HTML) and identical hotspot coordinates. The initial concern was based on a flawed hash extraction that didn't account for Rust's use of
include_str!()
macros instead of embedded hashes. All entries are accounted for and properly synchronized between the two files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/cursor-info/src/macos.rs
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/cursor-info/src/macos.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/
and/or a siblingtests/
directory for each crate insidecrates/*
.
Files:
crates/cursor-info/src/macos.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (1)
crates/cursor-info/src/macos.rs (1)
266-320
: LGTM: Complete hash mappings for Tahoe cursorsAll 20 Tahoe cursor variants have corresponding hash mappings with proper SHA-256 hex strings obtained from a macOS Tahoe system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/cursor-info/src/macos.rs (1)
46-66
: Add documentation comments for Tahoe cursor variants.The Tahoe enum variants lack doc comments, unlike the regular macOS cursor variants (lines 9-44) which include references to Apple's documentation. Adding doc comments would improve consistency and clarify that these are Tahoe-specific variants.
Consider adding doc comments:
// macOS Tahoe Cursors + /// Tahoe variant of the standard arrow cursor TahoeArrow, + /// Tahoe variant of the contextual menu cursor TahoeContextualMenu, + /// Tahoe variant of the closed hand cursor TahoeClosedHand, // ... and so on for remaining variants
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/cursor-info/cursors.html
(10 hunks)crates/cursor-info/src/macos.rs
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs
: Format Rust code usingrustfmt
and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/cursor-info/src/macos.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/
and/or a siblingtests/
directory for each crate insidecrates/*
.
Files:
crates/cursor-info/src/macos.rs
🔇 Additional comments (3)
crates/cursor-info/src/macos.rs (1)
142-224
: LGTM! Tahoe cursor implementation is well-structured.The resolve() and from_hash() implementations correctly handle all 20 Tahoe variants with appropriate asset paths, hotspot coordinates, and hash mappings. Reusing legacy assets for visually-identical cursors (drag_copy, drag_link, operation_not_allowed, ibeam_vertical) is efficient and avoids unnecessary duplication.
Also applies to: 233-321
crates/cursor-info/cursors.html (2)
1-1
: LGTM! Formatting improvements enhance readability.The doctype normalization, CSS multiline formatting, and JavaScript trailing comma adjustments are all cosmetic improvements that enhance code readability without changing functionality.
Also applies to: 53-77, 144-145, 520-614
264-377
: LGTM! Tahoe cursor entries are fully consistent with Rust implementation.All 19 Tahoe cursor entries correctly match the Rust implementation:
- Hotspot coordinates are identical
- Hash values match the from_hash() mappings
- SVG paths correspond to the resolve() asset references
- TahoeDisappearingItem is appropriately omitted (returns None in Rust)
I finally succeeded in adding the macOS Tahoe cursor!
To achieve this, I obtained the SVG files of the macOS Tahoe icons. Richie, your comment on the issue really helped it gave me the direction I needed. Thank you!
After adding the cursors, I used the
NSCursor
API to retrieve the cursor hashes for the available public cursors. However, this API doesn’t support every cursor meaning it’s impossible to generate hashes for all of them.And me being a bit of a perfectionist, I spent countless hours (honestly, almost an entire day) trying to find a way to generate hashes for those unsupported cursors. But after a lot of attempts, I realized it’s simply not possible through this API.
Because of that, I was initially hesitant to create this PR. But after reviewing the previous implementation, I noticed that it also only included cursors with public APIs available. So, I decided to go ahead and implement support for those.
I’m both excited and a little nervous to see if I truly solved the issue — but I’m really happy to have achieved the result I was aiming for. From my heart, I’m open to any feedback or suggestions you might have. 💙
I’ve learned so much while working on this feature, and I’m genuinely excited to hear your thoughts! 🚀
/claim #1100
Closes #1100
Uploading Built-in Retina Display 2025-10-11 09:30:19.mp4…
Summary by CodeRabbit
New Features
Documentation
Style