Skip to content
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

[iOS] Stylus support #3113

Merged
merged 43 commits into from
Sep 24, 2024
Merged

[iOS] Stylus support #3113

merged 43 commits into from
Sep 24, 2024

Conversation

m-bert
Copy link
Contributor

@m-bert m-bert commented Sep 18, 2024

Description

This PR adds stylus support on iOS.

Warning

This PR lacks support for Hover since currently we are unable to test that.

Note

You can read more about this feature in #3107

Test plan

Tested on StylusData example

@m-bert m-bert changed the base branch from main to @mbert/stylus-support-web September 18, 2024 13:16
Base automatically changed from @mbert/stylus-support-web to main September 19, 2024 09:50
@m-bert
Copy link
Contributor Author

m-bert commented Sep 19, 2024

Let me answer those questions, @j-piasecki:

What about hover?

It's hard to say as currently we have no way to test it. The two options that I see are either to implement it and hope that it will work fine, or wait for a device that will allow us to check if it is properly implemented.

Either way, I think we can leave it as TODO and finish it in another PR. Let me know what do you think.

Do we want to add support for rollAngle?

I'm not sure. If we do, we should also add support for twist property (I guess they will be similar). Sadly, my phone doesn't support twist so I was unable to test that. Also I'll have to check if our device does support rollAngle.

The other reason why I have not added it yet is because there's no such thing on Android (at least I haven't seen anything like that in docs).

So in my opinion we can add it, but it is not a big deal if it's missing. What's your stance on that?

@j-piasecki
Copy link
Member

I guess we can add it in the future alongside hover.

apple/Handlers/RNPanHandler.m Outdated Show resolved Hide resolved
apple/Handlers/RNPanHandler.m Outdated Show resolved Hide resolved
apple/RNGHStylusData.h Outdated Show resolved Hide resolved
@tomekzaw
Copy link
Member

tomekzaw commented Sep 23, 2024

Please build this PR for visionOS simulator prior to merging

@m-bert
Copy link
Contributor Author

m-bert commented Sep 23, 2024

Please build this PR for visionOS simulator prior to merging

I've just done it and it builds. I'll check it one more time if the code changes 😅

apple/Handlers/RNPanHandler.m Show resolved Hide resolved
apple/Handlers/RNPanHandler.m Outdated Show resolved Hide resolved
@m-bert m-bert merged commit 744d921 into main Sep 24, 2024
3 checks passed
@m-bert m-bert deleted the @mbert/stylus-support-ios branch September 24, 2024 13:03
@m-bert m-bert mentioned this pull request Oct 8, 2024
@RuudBurger
Copy link

This seems to be breaking tvOS. I get Property 'stylusData' not found on object of type 'RNBetterPanGestureRecognizer *' for this line withStylusData:[panRecognizer.stylusData toDictionary]] in RNPanHandler.m

@m-bert
Copy link
Contributor Author

m-bert commented Oct 24, 2024

Hi @RuudBurger, thanks for reporting this. Should be fixed by #3180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants