facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.43k stars 24.25k forks source link

Touchables translated into view are not pressable on android #28894

Closed bdrobinson closed 1 month ago

bdrobinson commented 4 years ago

Description

When a touchable is rendered offscreen (or possibly just outside the bounds of its parent) but moved into view using translateX/translateY, it cannot be pressed. This bug is only present on Android, and works fine on iOS.

There's been a few issues related to this that have been closed due to inactivity, so I'm opening a new one. They are:

(@AdamGerthel and @lehresman I'd be interested to know if you got any further debugging these issues?)

React Native version:

0.61.2

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

  1. Open the snack https://snack.expo.io/@bdrobinson/android-offscreen-touchable-bug on an android device

Expected Results

When you press the left/right buttons, both should be expected to respond to your touches (you can run on iOS to verify this). However, on Android only the left one responds to touches, whereas the right doesn't.

Snack, code example, screenshot, or link to a repository:

https://snack.expo.io/@bdrobinson/android-offscreen-touchable-bug (copy+pasted from https://github.com/facebook/react-native/issues/26219)

AdamGerthel commented 4 years ago

(@AdamGerthel and @lehresman I'd be interested to know if you got any further debugging these issues?)

I've used workarounds to solve it in the different cases I've had this issue:

skay97 commented 4 years ago

@AdamGerthel I am facing a similar issue and wanted to see if you could provide some more clarification. By putting an invisible view behind the touchable surface, do you mean you wrapped the touchable opacity in a view that had no styles? In my case, I am translating my FAB, however due to the transformations, I can no longer click the opacity. Due to which, I am having to use the top and bottom properties instead. I would have no issue utilizing those properties, it's just that I also can't use native driver without transforming.

AdamGerthel commented 4 years ago

@skay97 My code is too obscure for it to make immediate sense and it would take me too much time to rewrite it as a re-usable sample. Basically I first render this view: <View style={{ height: x }} /> where x is the height of the view that is un-clickable on Android. I use onLayout on the Animated View to get the height of the view, and then pass it to the other view (which is placed "behind" the animated one). Exactly how to use these values depend on what you're trying to achieve, which transforms you're using etc.

fabOnReact commented 4 years ago

This line does not take in consideration the transformation effect. It calculates the number of child present on the screen before applying the transformation (https://github.com/facebook/react-native/issues/27333#issuecomment-657566412)

https://github.com/facebook/react-native/blob/0060b5de559cd9c785a2a2a6c66f58088fea4dd2/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java#L114-L115

The number of child i is used to loop around each children of the viewGroup and check if the touch was done inside that View, but as the value childIndex was calculated before the transformation, the button View is not included in this loop

https://github.com/facebook/react-native/blob/0060b5de559cd9c785a2a2a6c66f58088fea4dd2/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java#L119-L122

this is the logic checking that click x, y was performed inside the Button View

https://github.com/facebook/react-native/blob/0060b5de559cd9c785a2a2a6c66f58088fea4dd2/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java#L124-L134

I need to apply the transformation before calculation the number of childrens

https://github.com/facebook/react-native/blob/0060b5de559cd9c785a2a2a6c66f58088fea4dd2/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java#L115

I'll try to prepare a pull request tomorrow

Thanks a lot I wish you a good day Fabrizio Bertoglio

fabOnReact commented 4 years ago

https://github.com/facebook/react-native/blob/03489539146556ec5ba6ba07ac338ce200f5b0f4/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java#L320

The transformation is not yet applied and [getChildCount](https://developer.android.com/reference/android/view/ViewGroup#getChildCount()) will return 1. The loop runs and does not check that onTouchEvent x, y coordinates are in the button area.

https://github.com/facebook/react-native/blob/0060b5de559cd9c785a2a2a6c66f58088fea4dd2/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java#L115-L119

I can detect that the child child has been transformed like this

https://github.com/facebook/react-native/blob/0060b5de559cd9c785a2a2a6c66f58088fea4dd2/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java#L122

        Matrix childMatrix = child.getMatrix();
        Log.w("TESTING::TouchTargetHelper", "childMatrix.isIdentity(): " + childMatrix.isIdentity());
        // Logs false for ViewGroup id 0x17 (23) - a transformation has been applied and not taked in consideration

image

calling setTranslationX and re-executing the process could fix this issue, I'll be testing this afternoon.

https://github.com/facebook/react-native/blob/03489539146556ec5ba6ba07ac338ce200f5b0f4/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java#L324

another option is triggering with a callback this process after transformation is applied, instead of duplicating the logic

fabOnReact commented 4 years ago

My opinion is that issue https://github.com/facebook/react-native/issues/28894 and https://github.com/facebook/react-native/issues/27333 created by @bdrobinson and @AdamGerthel should be closed.

Both issues do not include acceptable reproducible examples.

@AdamGerthel in https://github.com/facebook/react-native/issues/27333 should have just applied { height: integer } as his container has height = 0. The TouchableOpacity inside a container with height = 0 can not be clicked.

@bdrobinson you can just fix https://github.com/facebook/react-native/issues/28894 by specifying the correct width of the View you want to transform. The View is not clickable because you apply width: Dimensions.get('window').width() and then shift the View 50% to the left. After transform your View Width is 50% of the Screen.

Unluckily same and probably a duplicate of https://github.com/facebook/react-native/issues/27333#issuecomment-657566412

const styles = StyleSheet.create({
  outerContainer: {
  },
  innerContainer: {
    flexDirection: 'row',
    width:1800,
    transform: [{translateX: -200}]
  },
});

If you want the ViewGroup to be clickable with a transform, you need to make sure it has the correct width. I invested a lot of time in both issues to find this mistake, so I would be thankful if you could close both issues.

Thanks I wish you a good day Fabrizio Bertoglio

bdrobinson commented 4 years ago

Thanks for looking into it so closely @fabriziobertoglio1987! You clearly understand the problem space a lot better than I do so I'm happy to defer to your opinion to make sure that this issue doesn't cause unnecessary noise for maintainers.

However, I would question whether this is indeed an "invalid example" since it works on iOS and fails on Android?

stale[bot] commented 3 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

chaselal commented 3 years ago

This is still an issue. Use translateX to position a view outside the viewport -> animate the view into the viewport using translateX -> the view does not receive touch. Touches pass through to the views behind the animated-in view.

chaselal commented 3 years ago

One use case is:

AdamGerthel commented 3 years ago

I agree that this is still an issue, and I don't agree with what @fabriziobertoglio1987 is saying either. The fact that a view is interpreted as having height: 0 does not mean that I assigned height: 0 to it (which I clearly haven't). My sample works in both iOS and Web, but not Android, and I'm following very basic CSS principles, which is what is expected when using React Native. The fault lies with RN/Android, not my code.

Basically, my take on it is that the height of the View should be calculated by the native Android code rather than being calculated by hand in JS. Assigning a fixed height prevents dynamic content from being placed in the view. Doing that is what I consider to be a bad workaround for an obvious bug. If position: 'absolute', bottom: 0 causes height: 0 on Android, but not in Web or iOS, then I think the fault lies with RN/Android.

See https://github.com/facebook/react-native/issues/27333#issuecomment-657607542 as well.

hellogerard commented 3 years ago

I tried all the ideas mentioned above and nothing worked.

...I found replacement touchables from react-native-gesture-handler and it worked right away. Drop-in replacement. Just replace your react-native imports with:

import {
  TouchableNativeFeedback,
  TouchableHighlight,
  TouchableOpacity,
  TouchableWithoutFeedback,
} from 'react-native-gesture-handler';
AnnaSearl commented 3 years ago

I get the same issue, I want to make a dropdown,in IOS everything is OK, but android is not work, Who can help me...

mrousavy commented 3 years ago

I'm experiencing the same issue. It's a real bummer, because I've created a custom horizontal swiper (carousel), and <Pressable>/<TouchableWithoutFeedback> components are not pressable on android at all. The only workaround I've found so far is by using the TouchableWithoutFeedback export from react-native-gesture-handler since that does not use the JS touch responder system, but I'd feel pretty uncomfortable using that workaround, because I'd have to replace all my app's screens' touchables and I can't use the Pressable API anymore.

@fabriziobertoglio1987 you mentioned you'd prepare a pull request - did you find a native fix for this?

EDIT: Just read the comment about providing valid widths - do you mean the root view should have a width of SCREEN_WIDTH * numberOfHorizontalPages? That's not a good solution. As many users (including me) rely on the removeClippedSubviews property, and that requires views to be offscreen when the parent has overflow: 'hidden', which is redundant if the parent container view is as wide as the whole carousel. (= bad performance) The parent container should always have a viewport of exactly the phone's dimensions and only the horizontal carousel gets translated on the X axis, so RCTView can optimize offscreen views away. Otherwise those views would have to be rendered, even if they're not visible. Imagine the Snapchat homescreen - you don't want to have all 5 screens rendered at the same time, only the screen(s) that are within the viewport should get rendered.

fabOnReact commented 3 years ago

@mrousavy thanks for the reply

If I still remember well my research explained in https://github.com/facebook/react-native/issues/28894#issuecomment-658262246 made me conclude that ReactAndroid will restrict touch events to parent view area available, so for example if you transform the parent view or set height/width = 0, then you can not touch the TouchableWithoutFeedback

This is included in the ReactNative Touchable docs as I explained in https://github.com/facebook/react-native/issues/27333#issuecomment-657566412

regarding TouchableWithoutFeedback from react-native-gesture-handler, they probably don't use the same Android API included above... so that may be the reason you don't have this limitation..

Probably reading their sourcecode may help us understand how to remove this limitation, but I believe the change will be complex

I may be wrong as I did not work for long time on this issue, once I will have more free time I will have a further look at the issue. Thanks a lot.

paulduthoit commented 3 years ago

Hi ! Has anyone found a solution while waiting for the problem to be resolved ? Thanks.

lelukas commented 3 years ago

Also, don't if anyone noticed it but the same happens with FlatLists

stale[bot] commented 2 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

TomasSestak commented 2 years ago

Still valid issue i think

hotaryuzaki commented 2 years ago

I tried all the ideas mentioned above and nothing worked.

...I found replacement touchables from react-native-gesture-handler and it worked right away. Drop-in replacement. Just replace your react-native imports with:

import {
  TouchableNativeFeedback,
  TouchableHighlight,
  TouchableOpacity,
  TouchableWithoutFeedback,
} from 'react-native-gesture-handler';

yes its works. but its has other issue, if underneath the icon has onpress so it will fire also.

github-actions[bot] commented 11 months ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] commented 11 months ago

This issue was closed because it has been stalled for 7 days with no activity.

fabOnReact commented 8 months ago

I'm organizing my next tasks. Are you still interested in adopting a fix for this issue? Or have you already implemented a workaround? If you are interested, I will work on a solution. Thanks

hotaryuzaki commented 8 months ago

I'm organizing my next tasks. Are you still interested in adopting a fix for this issue? Or have you already implemented a workaround? If you are interested, I will work on a solution. Thanks

I think this issue is an important one, because it will be raised to anyone who has a bigger icon in the middle of the bottom tab bar

react-native-bot commented 2 months ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

react-native-bot commented 1 month ago

This issue was closed because it has been stalled for 7 days with no activity.