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

Fix for bug where mouse coordinates are wrong #239

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

russellmcc
Copy link

This occurs when our main react view is not the contentView of a
window. Touch coordinates are in window coordinates, but hitTest
expects coordinates in the coordinate system of our superview.

This is my first PR, let me know what I can do to get this in.

This occurs when our main react view is not the contentView of a
window.  Touch coordinates are in window coordinates, but `hitTest`
expects coordinates in the coordinate system of our superview.

This is my first PR, let me know what I can do to get this in.
@aleclarson
Copy link
Collaborator

When does self.view not have a superview?

Also, please try out #228, which is the future of "touch" handling in react-native-macos.

@russellmcc
Copy link
Author

russellmcc commented Apr 11, 2019

According to apple's docs, NSViews have a nil superview when they are not in a view hierarchy. I think the most common case for this is when it's the contentView of the window. I'm not familiar enough with react to know if react views can never be the contentView - if this is the case, I'm happy to remove the check.

Thanks for the tip on #228! I'll try out the branch soon and let you know how it goes.

@russellmcc
Copy link
Author

russellmcc commented Apr 12, 2019

I just checked, #228 first of all still has this problem, and additionally has another problem for clients that don't use RCTWindow - it seems that line 90 on RCTContentView.m on that branch flips the coordinate system - but this is inappropriate when not using RCTWindow. After removing this line, that branch showed the same problems as master. I can see what happens if I use RCTWindow instead, but in my use case I'm a view in a window someone else created, so this approach won't work for me.

@russellmcc
Copy link
Author

Sorry, I don't have the ability to move to RCTWindow as it stands now since my NSWindow is created long before my RCTBridge!

This mirrors the behavior of upstream `react-native` for ios.
@russellmcc
Copy link
Author

I've just added another, related commit to this branch; I hope that's okay. More than happy to split these out into separate PRs if that's more convenient for you.

An explanation: I am trying to use lottie views as interactive controls. The current logic of checking if something is an RCTView doesn't work for this, because the actual type is LOTAnimationView. I've replaced this logic with the logic used in the ios version here, I think this makes more sense and is less surprising. Using this method I think is much more robust.

@russellmcc
Copy link
Author

Anyone had time to give this another look? I'm still motivated to land this; and am willing to help however I can!

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.

2 participants