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

Add modalOnResponderTerminationRequest property #47

Merged
merged 3 commits into from
Oct 31, 2016
Merged

Conversation

jdoyle65
Copy link
Contributor

Was having issues where a custom Gesture Responder on one of my views was being triggered when scrolling up and down on the iOS Date Picker Modal. Added this property to override onResponderTerminationRequest callback on the Date Picker Modal.

By default the behaviour will be the same as before.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.4%) to 96.552% when pulling 2b18c31 on jdoyle65:master into df91a40 on xgfe:master.

@feyy feyy merged commit bee4777 into xgfe:master Oct 31, 2016
@feyy
Copy link
Contributor

feyy commented Oct 31, 2016

Awesome!

@feyy
Copy link
Contributor

feyy commented Oct 31, 2016

But I think we should set modalOnResponderTerminationRequest return default false to prevent parent component get responder, and add these properties to the picker msak.

feyy added a commit that referenced this pull request Oct 31, 2016
add `modalOnResponderTerminationRequest` function to custom define touch responder logic

#47
@jdoyle65
Copy link
Contributor Author

Actually when it returns true is when it will do the behaviour you're describing (a little bit counter-intuitive, for sure, but that's how it works in React Native).

@jdoyle65
Copy link
Contributor Author

Actually, sorry I just realized I misread your comment. You're right, returning false would probably be a better default behaviour.

It's possible this would be a breaking change for anyone who has written workarounds to this in their own apps.

@feyy
Copy link
Contributor

feyy commented Nov 3, 2016

This feature will prevent the TouchableHighlight's onPress event in my 7P, do you have some idea?

@jdoyle65
Copy link
Contributor Author

jdoyle65 commented Nov 3, 2016

According to the docs (https://facebook.github.io/react-native/docs/gesture-responder-system.html#responder-lifecycle) the default value this function returns is false.

Try passing in a function that returns true, and see if that works. RN will go to the deepest node first to see if it's trying to request being the responder. It will then bubble up to see if any of that node's parent/grandparents are also trying to become the responder. When onResponderTerminationRequest is set to return false, that Component will take over as the responder as it bubbles up, effectively stealing the responder from it's descendant.

So basically your TouchableHighlight requests to be the responder, but when the request bubbles up to the parent, the View Component that has the onResponderTerminationRequest property, it will steal the responder from TouchableHighlight if it's return false

@feyy
Copy link
Contributor

feyy commented Nov 3, 2016

I see. But this bug only grow in 7p, others work fine. And if we can only return true in onResponderTerminationRequest, this function can do so little!But I will return true fix this problem first!

@jdoyle65
Copy link
Contributor Author

jdoyle65 commented Nov 3, 2016

Yeah I find the gesture responder is a bit weird. Is it an OS level problem? Is your 7P on iOS 10 whereas the other devices are on iOS 9?

I found there's definitely slightly differing behaviour between gesture responders on the iOS 9 simulator and my physical device (Nexus 6P running Android 6.0.1)

@feyy
Copy link
Contributor

feyy commented Nov 3, 2016

I find the problem only grow in my 7p and 7p simulator, all with IOS 10. But the user report that his 6S also have this problem. Maybe it's OS version problem.

@jdoyle65
Copy link
Contributor Author

@feyy I'm running into this issue with a tester who is using an iPhone 7 with iOS 10 as well. I have tested on an iPad Mini 2 running iOS 10.1.11 and it works, so it seems it may be device specific.

@hibearpanda
Copy link

@feyy I found this thread via this issue #50. I tested this component on an iphone 6s and iphone 6s plus and couldn't dismiss the modal via the Confirm, Cancel or the modal background easily without tapping it multiple times.

Just wanted to let you know regarding which models aren't working.

Using v1.3.1 right now.

@ghost
Copy link

ghost commented Jul 15, 2019

Great PR. Very helful. 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

Successfully merging this pull request may close these issues.

4 participants