peacechen / react-native-modal-selector

A cross-platform (iOS / Android), selector/picker component for React Native that is highly customizable and supports sections.
MIT License
369 stars 129 forks source link

TouchableWithoutFeedback from PR 31 causes onChange not to be called when rendered in a component, possibly elsewhere #116

Closed JolivetteSax closed 5 years ago

JolivetteSax commented 5 years ago

Steps to reproduce Start expo project -> expo install react-native-modal-selector - render component with the simple example from a sub component, not the homescreen, items are not clickable in android.

Oddly, the simple example works as advertised from the home screen. After some debugging I tracked it down to the touchablewithoutfeedback. This seems like a superflous feature since there's a cancel button... I can't figure out why it interferes, as no simple changes make a difference, and onPress isn't being called. I noticed there are other issues with onChange with various workarounds suggested, this may clear them up also.

To fix: This PR https://github.com/peacechen/react-native-modal-selector/pull/31 needs to be rolled back or fixed. I don't see that it's particularly useful, especially if it prevents the entire component from working.

Is this project still maintained by anybody?

JolivetteSax commented 5 years ago

If there is anyone maintaining this project I'm happy to help figure out what's going on.

There are literally hundreds of forks, I wish I knew a way to determine if there was a successor...

JolivetteSax commented 5 years ago

This might be specific to the version of reactnative that expo uses, since they closed an issue that smells like this over here: https://github.com/facebook/react-native/issues/23740

peacechen commented 5 years ago

Thanks @JolivetteSax for reporting and investigating this issue. This is the official fork of react-native-modal-picker, renamed to react-native-modal-selector and published on npm since d-a-n is unresponsive.

I haven't had a chance to try this on RN 0.59. If you find a solution or work-around, please submit a PR. It would be ideal if FB fixes this core bug.

JolivetteSax commented 5 years ago

Thank you so much for your time. I totally agree with your assessment. I can't find any settings or workarounds to make it work other than removing the touchablewithoutfeedback component wrapper. Bummer.

Unfortunately I think this falls into one of those drawbacks of using Expo type things. They are sort of stuck on a RN version for a period of time, and there's no way to know when they'll update. My package.json says:

"react-native": "https://github.com/expo/react-native/archive/sdk-33.0.0.tar.gz",

For now the pros of using Expo for my prototype is still outweighing even this problem so I went ahead and snarfed the code for now, esp since it was just a couple of files. So I guess the only feature I'm missing is the ability to dismiss by touching elsewhere on the opacity. Is it customary to bother making a fork in such a case? I dunno. There are SO many forks of this already, I was glad to see your reply.

I was able to move on with life and do a demo so I'm grateful for your community efforts to make this a really nice alternative to the standard Picker. At some point when I "eject" I suppose I'll have to return to this to see if the new RN's have indeed fixed this problem - but by then perhaps this will be OBE with native apps or webviews....

I'm also inclined to leave the issue open so other expo users can find this easily.

mikaello commented 5 years ago

Unrelated to this issue, but related to finding maintained forks: https://github.com/musically-ut/lovely-forks Helped me multiple times

peacechen commented 5 years ago

@JolivetteSax If you'd like to take a stab at working around the RN bug, one approach could be to conditionally change the wrapping component based on backdropPressToClose. For instance, starting at this line: https://github.com/peacechen/react-native-modal-selector/blob/master/index.js#L247

The code could be:

    const closeOverlay = this.props.backdropPressToClose;
    const CloseWrapper = closeOverlay ? TouchableWithoutFeedback : View;

        return (
            <CloseWrapper key={'modalSelector' + (componentIndex++)} onPress={() => {closeOverlay && this.close()}}>
              ...
            </CloseWrapper>

Alternatively, substitute TouchableWithoutFeedback for TouchableOpacity and set activeOpacity=1. That's assuming TouchableOpacity works properly in RN 0.59. We'll review and merge your PR prompty.

JolivetteSax commented 5 years ago

Hey I like that idea! I'll give it a go. :)

peacechen commented 5 years ago

Published in v1.1.1