Added constraint of child type to touchablewithoutfeedback #517
Conversation
| @@ -78,8 +88,15 @@ var TouchableWithoutFeedback = React.createClass({ | |||
| }, | |||
|
|
|||
| render: function(): ReactElement { | |||
| // Note(vjeux): use cloneWithProps once React has been upgraded | |||
| var child = onlyChild(this.props.children); | |||
| var child = cloneWithProps( | |||
There was a problem hiding this comment.
Actually, can you use React.cloneElement, cloneWithProps isn't working because it blows away the key
|
It's the same code that is used on touchablehighlights currently i believe. Maybe i should revise both and add them to this commit? |
|
Yeah would be nice, thanks |
|
Uhm... i think that actually cloneWithProps in this instance should be removed (maybe?) either way it gets cloned again 3 lines below https://github.com/dzannotti/react-native/blob/master/Libraries/Components/Touchable/TouchableWithoutFeedback.js#L101 All i know is that it silently failed to me, without this check, and it does not now. No side effects. @sahrens for reference |
|
Any news on this? Would like to get this merged in, as currently using TouchableWithouFeedback without a native child view fails silently. |
|
I pulled in your change but it actually broke some internal code. But looking at it more, I'm not sure I understand why you even need to do ensureIsNative here. On the other Touchable elements it makes sense because the component does setNativeProps on the ref, but this one doesn't. |
|
It silently fails without the check, which internal code did it break? in all my tests it worked as expected, but things might have changed since i made this PR |
|
"It silently fails without the check" what silently fails? |
|
any non native components wrapped into TouchableWithoutFeedback don't receive any onPress events unless the onlychild is View |
|
I see what you mean! Need to figure out what the best way to handle this, but I disagree that enforcing that children are native components is the right call. should be able to work, otherwise you break the ability to build abstractions. |
|
@vjeux that's how it's done on TouchableWithHighlight |
facebook-github-bot
added
the
CLA Signed
Summary: Added constraint of child type to touchablewithoutfeedback to match touchablehighlight (this would have failed silently previously), also added the cloneWithProps as by note of @vjeux Closes facebook#517 Github Author: Daniele Zannotti <d.zannotti@me.com> Test Plan: Imported from GitHub, without a `Test Plan:` line.
Summary: Added constraint of child type to touchablewithoutfeedback to match touchablehighlight (this would have failed silently previously), also added the cloneWithProps as by note of @vjeux Closes facebook#517 Github Author: Daniele Zannotti <d.zannotti@me.com> Test Plan: Imported from GitHub, without a `Test Plan:` line.
Using `export PATH=$PATH:$ANDROID_HOME/emulator` instead of `export PATH=$PATH:$ANDROID_HOME/tools` Because of Android Studio Emulator path Update, (https://stackoverflow.com/questions/40931254/could-not-launch-emulator-in-android-studio/51902207#51902207)
Added constraint of child type to touchablewithoutfeedback to match touchablehighlight (this would have failed silently previously), also added the cloneWithProps as by note of @vjeux