Skip to content

Implement cursor: pointer for iOS / visionOS - #2080

Merged
Saad Najmi (Saadnajmi) merged 1 commit into
microsoft:mainfrom
Saadnajmi:cursor
Mar 3, 2024
Merged

Implement cursor: pointer for iOS / visionOS#2080
Saad Najmi (Saadnajmi) merged 1 commit into
microsoft:mainfrom
Saadnajmi:cursor

Conversation

@Saadnajmi

@SaadnajmiSaad Najmi (Saadnajmi) commented Feb 18, 2024•
edited
Loading

Copy link
Copy Markdown
Collaborator

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary:

Port react#43078 to React Native macOS. We already had a cursor prop for macOS, so this is mostly refactoring the implementation to be shared. I also added back the example page of the macOS cursors which seemed to get lost from #760 during some merges. Also as a bonus, we also implement cursor for Fabric on macOS 🎉

Changelog:

[IOS] [ADDED] - Implement cursor style prop

Test Plan:

See the example page running on all three apple platforms:

Screen.Recording.2024-02-26.at.11.33.52.PM.mov

This was referenced Feb 18, 2024
Closed
Closed
@Saadnajmi
Saad Najmi (Saadnajmi) marked this pull request as ready for review February 23, 2024 10:22
@Saadnajmi
Saad Najmi (Saadnajmi) requested a review from a team as a code ownerFebruary 23, 2024 10:22
This was referenced Feb 27, 2024
Merged
Merged
tido64
Tommy Nguyen (tido64) reviewed Feb 27, 2024
View reviewed changes
Comment threadpackages/react-native/Libraries/Text/RCTTextAttributes.mm Outdated
Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/conversions.h Outdated
Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/conversions.h Outdated
Comment threadpackages/react-native/Libraries/Text/RCTTextAttributes.mm Outdated
@SaadnajmiSaad Najmi (Saadnajmi) mentioned this pull request Feb 27, 2024
Closed
tido64
Tommy Nguyen (tido64) approved these changes Feb 29, 2024
View reviewed changes
Comment threadpackages/react-native/React/Views/RCTView.m Outdated
Comment threadpackages/react-native/React/Views/RCTView.m Outdated
@Saadnajmi
Saad Najmi (Saadnajmi) merged commit 70e4f2f into microsoft:mainMar 3, 2024
Facebook Community Bot (facebook-github-bot) pushed a commit to react/react-native that referenced this pull request Mar 5, 2024
@Saadnajmi@facebook-github-bot
feat(iOS): Implement cursor style prop (#43078)
73664f5
Summary:
Implement the cursor style prop for iOS (and consequently, visionOS), as described in this RFC: react-native-community/discussions-and-proposals#750
See related PR in React Native macOS, where we target macOS and visionOS (not running in iPad compatibility mode) with the same change: microsoft#2080
Docs update: react/react-native-website#4033
## Changelog:
[IOS] [ADDED] - Implement cursor style prop
Pull Request resolved: #43078
Test Plan:
See the added example page, running on iOS with the new architecture enabled. This also runs the same on the old architecture.
https://github.com/facebook/react-native/assets/6722175/2af60a0c-1c1f-45c4-8d66-a20f6d5815df
See the example page running on all three apple platforms. The JS is slightly different because:
1. The "macOS Cursors" example is not part of this PR but the one in React Native macOS.
2. This PR (and exapmple) has went though a bunch of iterations and It got hard taking videos of every change 😅
https://github.com/facebook/react-native/assets/6722175/7775ba7c-8624-4873-a735-7665b94b7233
## Notes
- React Native macOS added the cursor prop to View with microsoft#760 and Text with microsoft#1469 . Much of the implementation comes from there.
- Due to an Apple bug, as of iOS 17.4 Beta 4, the shape of the iOS cursor hover effect doesn't render in the correct bounds (but it does on visionOS). I've worked around it with an ifdef. The result is that the hover effect will work on iOS and visionOS, but not iPad apps running in compatibility mode on visionOS.
Reviewed By: NickGerleman
Differential Revision: D54512945
Pulled By: vincentriemer
fbshipit-source-id: 699e3a01a901f55a466a2c1a19f667aede5aab80
Alex Hunt (huntie) pushed a commit to react/react-native that referenced this pull request Mar 11, 2024
@Saadnajmi@huntie
feat(iOS): Implement cursor style prop (#43078)
7395765
Summary:
Implement the cursor style prop for iOS (and consequently, visionOS), as described in this RFC: react-native-community/discussions-and-proposals#750
See related PR in React Native macOS, where we target macOS and visionOS (not running in iPad compatibility mode) with the same change: microsoft#2080
Docs update: react/react-native-website#4033
## Changelog:
[IOS] [ADDED] - Implement cursor style prop
Pull Request resolved: #43078
Test Plan:
See the added example page, running on iOS with the new architecture enabled. This also runs the same on the old architecture.
https://github.com/facebook/react-native/assets/6722175/2af60a0c-1c1f-45c4-8d66-a20f6d5815df
See the example page running on all three apple platforms. The JS is slightly different because:
1. The "macOS Cursors" example is not part of this PR but the one in React Native macOS.
2. This PR (and exapmple) has went though a bunch of iterations and It got hard taking videos of every change 😅
https://github.com/facebook/react-native/assets/6722175/7775ba7c-8624-4873-a735-7665b94b7233
## Notes
- React Native macOS added the cursor prop to View with microsoft#760 and Text with microsoft#1469 . Much of the implementation comes from there.
- Due to an Apple bug, as of iOS 17.4 Beta 4, the shape of the iOS cursor hover effect doesn't render in the correct bounds (but it does on visionOS). I've worked around it with an ifdef. The result is that the hover effect will work on iOS and visionOS, but not iPad apps running in compatibility mode on visionOS.
Reviewed By: NickGerleman
Differential Revision: D54512945
Pulled By: vincentriemer
fbshipit-source-id: 699e3a01a901f55a466a2c1a19f667aede5aab80
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tido64Tommy Nguyen (tido64)tido64 approved these changes
+1 more reviewer
@motiz88Moti Zilberman (motiz88)motiz88 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Saadnajmi@motiz88@tido64