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

RN 48+ Android issues #8

Closed
kristfal opened this issue Nov 14, 2017 · 36 comments
Closed

RN 48+ Android issues #8

kristfal opened this issue Nov 14, 2017 · 36 comments

Comments

@kristfal
Copy link

In order to support the latest RN version (cant recall if this was changed in 48 or later), we'll need to drop the @override from createJSModules in TouchThroughViewPackage.java.

The open PRs to propTypes should also be merged to support the latest RN version.

@kristfal kristfal changed the title RN 48+ Android build failure RN 48+ Android issues Nov 14, 2017
@kristfal
Copy link
Author

On top of this, I'm getting redbox errors due to SimpleViewManager not being castable to ViewGroupManager using RN 0.50.3. If I do change the view manager to ViewGroupManager, the app runs fine but touches are not passed through.

Any tips on how to proceed?

@RichardLindhout
Copy link
Contributor

@kristfal We're having the same problem at the moment.

@RichardLindhout
Copy link
Contributor

A colleague is looking into it. Maybe we can add or remove some code which will send the touch event through.

@martsie
Copy link

martsie commented Dec 1, 2017

Thanks @RichardLindhout ... if you get a patch happening I'll message Rome2rio and try to push it through

martsie pushed a commit that referenced this issue Dec 8, 2017
@martsie
Copy link

martsie commented Dec 8, 2017

I've referenced this issue in the Readme to be as transparent as possible to new users.

@simonhoss
Copy link

simonhoss commented Dec 8, 2017

Can someone tell me on which react-native version this works? I'm currently thinking to make a downgrade. At the moment we are on 0.47.1

@ianmartorell
Copy link

Is there an update on this? I'd like to use this package with 0.50.3. I can try to look into the issue if no one else is working on it.

@simonhoss
Copy link

I think some trying to find a solution, but without success until now. If you find a solution then this would be awesome

simonhoss referenced this issue in simonhoss/react-native-touch-through-view Dec 22, 2017
@simonhoss
Copy link

I made a PR with a possible fix. Im not super happy with it but I didn't found any other solution, except registering it to the app activity.

@RichardLindhout
Copy link
Contributor

@simonhoss Thanks a lot!

Seems this function is not used?

  //If the touch was not on the list make the slop rect small so react-native dont use this view as responder
 +    public Rect getHitSlopRect() {
 +        if (lastTouchWasNotValid) {
 +            return new Rect(-1000, -1000, -1000, -1000);
 +        }
 +        return new Rect(0, 0, 0, 0);
 +    }

@simonhoss
Copy link

simonhoss commented Jan 2, 2018

This is the main function to prevent react-native using this view. This function comes from the interface ReactHitSlopView

Background:
Every time you touch something in react-native it goes through the views and try to figure out the correct view for the target based on the touch position. I figured out that react-native uses the getHitSlopRect when it exists. The idea of the getHitSlopRect is, that you can make a touch area bigger when you have for example a small button. I used it now to make the touch area super small, so that the the view cannot be the target. That's also the reason why we have to register the touch in the activity and before react-native uses the touch and goes through the view tree, because the getHitSlopRect is checked before the touch method is called for the view.

@kristfal
Copy link
Author

@RichardLindhout Hey! Want to merge this one in (along with the other fixes), or would you consider this repo stale?

@martsie
Copy link

martsie commented Jan 16, 2018

If you folks have tested any PR's and are happy with them then I can merge and publish - just let me know which ones :) I'm not currently using this library in my projects so I'm relying on you fine people to test changes.

@ianmartorell
Copy link

Have you guys tried using View's pointerEvents prop? It solved my specific use case.

@simonhoss
Copy link

simonhoss commented Jan 25, 2018

The problem is when you use pointerEvents that you set the pointerEvents on the transparent header view. When you now try to press a button which is behind that transparent header then the button gets not triggered, because the List which fills the complete screen is the responder.

@martsie
Copy link

martsie commented Jan 30, 2018

pointerEvents doesn't transmit its events through scroll views. See the examples on the project page to get an idea of use cases where you'd need to use this over pointerEvents.

@DVSoftware
Copy link

Any update on this one?

@derekblank
Copy link

derekblank commented Apr 3, 2018

@simonhoss I'm trying your solution, which looks promising. I'm using react-native-navigation, however, which requires that we extend com.reactnativenavigation.controllers.SplashActivity instead of ReactActivity in MainActivity.java.

I attempted to merge the two with the following:

package com.myapp;

import com.reactnativenavigation.controllers.SplashActivity;
import android.view.MotionEvent;
import com.rome2rio.android.reactnativetouchthroughview.TouchThroughTouchHandlerInterface;
import com.rome2rio.android.reactnativetouchthroughview.TouchThroughTouchHandler;

public class MainActivity extends SplashActivity implements TouchThroughTouchHandlerInterface {
    private TouchThroughTouchHandler touchThroughTouchHandler = new TouchThroughTouchHandler();

    @Override
    protected String getMainComponentName() {
        return "myapp";
    }

    public TouchThroughTouchHandler getTouchThroughTouchHandler() {
        return touchThroughTouchHandler;
    }

    @Override
    public boolean dispatchTouchEvent(MotionEvent ev) {
        touchThroughTouchHandler.handleTouchEvent(ev);

        return super.dispatchTouchEvent(ev);
    }
}

but that produces this error on getMainComponentName:

> Task :app:compileDebugJavaWithJavac FAILED
/../../myapp/MainActivity.java:12: error: method does not override or implement a method from a supertype
    @Override
    ^
1 error

Any ideas on possible workarounds? I feel like this is possibly related to a conflict with ViewGroupManager and RNN's SplashActivity. Do you know of another way we could apply the @Override methods for applications not using ReactActivity in MainActivity.java?

@simonhoss
Copy link

Hi @derekblank

I had a look into the documentation and code from react-native-navigation. If you're using react-native-navigation then you don't need getMainComponentName, because react-native-navigation don't need it. The most important part is the TouchTroughHandler and the interface TouchThroughTouchHandlerInterface to make it work

Greets

@derekblank
Copy link

derekblank commented Apr 3, 2018

@simonhoss Thanks - I removed getMainComponentName on MainActivity, and Android built successfully, but it hits the exception "TouchThroughTouchHandlerInterface was not set on app activity" from TouchThroughWrapper.

Perhaps it still doesn't like the way MainActivity extends SplashActivity implements TouchThroughTouchHandlerInterface?

Edit/Update: I logged out the app's current activity and realized that react-native-navigation transfers the main activity from SplashActivity to NavigationActivity.

@simonhoss
Copy link

I will have a look on this and give you feedback if I found something.

@simonhoss
Copy link

@derekblank I created an issue for that here: simonhoss#3

Let us talk there so that we don't fill up this issue

@traviswimer
Copy link

@martsie Do you still plan to merge in any PRs?

I've been using @simonhoss 's branch, and it works well for me on Android.

On iOS, pod install fails because summary is empty. This is because it is trying to pull the description from package.json, which is currently an empty string.

@martsie
Copy link

martsie commented Nov 27, 2018

Can you recap me on which pull request it is - is it #10 ? I can merge and publish today if you're happy with it and there are no backdoors hidden in it that would make me a temporary internet celebrity

@kristfal
Copy link
Author

@martsie I've been using @simonhoss's branch for a while. No issues.

@simonhoss
Copy link

Maybe it makes sense that I update the pull request first? Because I did some further changes on my repo.

@traviswimer
Copy link

I looked through most of the code on @simonhoss 's branch the other day and didn't see any issues.

Just a reminder that it currently isn't working on iOS until a string is added to the package.json description field. Also, there is a lingering System.out.println() in TouchThroughWrapper.java, that is presumably left over from debugging.

I can make a PR to fix those if it would be helpful.

@martsie
Copy link

martsie commented Nov 29, 2018

I'm happy to transfer ownership of the NPM to @simonhoss if you'd like - that may bump up the pace and it sounds like you're all using his branch anyway :)

@simonhoss
Copy link

simonhoss commented Nov 29, 2018

@traviswimer a PR is very welcome!

@martsie for me that would be fine I still maintaining my fork. So I would say it makes sense 😉

@martsie
Copy link

martsie commented Nov 30, 2018

@simonhoss message me on github with your npm username and I'll add you to the package and the change the readme for this project to send people over to your github.

@simonhoss
Copy link

@martsie My npm username is simonhoss same as in github ;)

@martsie
Copy link

martsie commented Dec 5, 2018

@simonhoss I've added you as an owner on NPM - send me the word and I'll update the README of this repo to point to yours 👍

@simonhoss
Copy link

@martsie Thank you. Unfortunately I was not able to make a new npm version, because of some pressure in a other project. But new year, new chance 😄 I will try to change it this week lately next week!

@simonhoss
Copy link

New version on npm is released. @martsie I think this issue can be closed now :D

@martsie Should I leave you as maintainer in npm?

@martsie
Copy link

martsie commented Jan 13, 2019

Yes please @simonhoss - maybe I can help out in the future.

@martsie martsie closed this as completed Jan 13, 2019
@simonhoss
Copy link

Alright. Thank you!

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

No branches or pull requests

8 participants