Add tvOS Target #448
Add tvOS Target #448
Conversation
|
Sweet! Thanks @hufkens for working on this. Please tag me here once the PR is ready for review. In the meantime could you please add "[WIP]" (for work-in-progress) to the PR title so that it is clear it should not be looked at just yet |
|
@kmagiera I'm currently working on fixing react-native-screens too. But I'm a bit confused because it was also a dependency of react-navigation and in version 2.14 it was not included, but 2.18 it was included in react-navigation. If I want to use the latest 3.0.x how is react-navigation-screens and react-navigation-gesture-handler related to react-navigation? Could you help me with that? |
|
With react-navigation by default you don't use any of the native screens code unless you explicitly opt in (by calling |
|
Thanks for the clarification. The problem we have now is that on TV platforms (Android, AppleTV) react-navigation doesn't work nice with the focusable elements. It should be solved by using react-native-screens and that's why we need the library to have the tvOS target. Also when upgrading to the latest react-navigation, it adds this library as a dependency. So your library needs a tvOS target to work for us. The problem with gestures is that I need to exclude some code because it's not supported on tvOS. I'll ping you when I have a working version and you can verify. |
|
@kmagiera It should be ok now. It works in my project. |
|
Glad to hear that @hufkens, but prior to merging this I'd discuss one thing: What is the usage pattern behind using multi-touch handlers on tvOS devices? Would you expect to be able to use it (e.g. because of the fact you could share more code) or maybe it would make sense to warn about the fact these are being used. As far as I understand the changes you've made things like rotation, pinch will render but there is no way to interact with them. |
|
I added the conditional compiling checks because these API's are not available on tvOS. There are no pinch, rotation, ... gestures. Not sure that this is the best way to do it. Also not sure how react-navigation uses the code underneath. Feel free to make some changes, but I guess that is what it is. The classes (skeleton) still exists but on tvOS it will just not do anything. |
|
Cool. I think this is good for now. I think it could've been better if we exclude rotation/pinch and other iOS specific handlers from tvOS targets so that they don't compile at all and to provide stub implementations on tvOS for these. This way we could avoid adding #ifdef blocks and hopefully prevent tvOS compatibility breaks in the future. Will create a ticket for that and merge this change in to unblock you |
This is a fix for #448. `ifndef TARGET_OS_TV` is not proper check since `TARGET_OS_TV` is defined, but set to `NO` https://github.com/kmagiera/react-native-gesture-handler/pull/475/files#diff-18049faae8f7be012debf7181b7ee576L24 Also, it seems to unwanted change. This code is no compilable.
|
Seems latest fixes are not included on |
Work in progress. I added a new -tvOS Target to the libraries project. Also some gestures are not supported on tvOS. I fixed that with a compiler flag. But there might be a better way to solve that.
This is a fix for software-mansion#448. `ifndef TARGET_OS_TV` is not proper check since `TARGET_OS_TV` is defined, but set to `NO` https://github.com/kmagiera/react-native-gesture-handler/pull/475/files#diff-18049faae8f7be012debf7181b7ee576L24 Also, it seems to unwanted change. This code is no compilable.
Work in progress. I added a new -tvOS Target to the libraries project. Also some gestures are not supported on tvOS. I fixed that with a compiler flag. But there might be a better way to solve that.