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

Fix TouchableWithoutFeedback's Lack of View #23871

Closed
wants to merge 11 commits into from
Closed

Fix TouchableWithoutFeedback's Lack of View #23871

wants to merge 11 commits into from

Conversation

RyanElliott10
Copy link
Contributor

@RyanElliott10 RyanElliott10 commented Mar 12, 2019

Summary

This PR addresses the issue regarding TouchableWithoutFeedback components not correctly working without a <View> embedded within them. The most current issue can be found here.

render() now returns a View -- rather than just cloning the children -- with all of the props passed into the TouchableWithoutFeedback component, as well as all children. This addresses the issue where the children's props weren't being properly passed down into a View.

Changelog

[General] [Fixed] - render() now wraps all props and child props correctly in a view

Test Plan

Pass all CircleCI Tests

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.
  • flow found some issues.

Libraries/Components/Touchable/TouchableWithoutFeedback.js Outdated Show resolved Hide resolved
Libraries/Components/Touchable/TouchableWithoutFeedback.js Outdated Show resolved Hide resolved
Libraries/Components/Touchable/TouchableWithoutFeedback.js Outdated Show resolved Hide resolved
nativeID={this.props.nativeID}
testID={this.props.testID}
onLayout={this.props.onLayout}
hitSlop={this.props.hitStop}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get this.props.hitStop because property hitStop is missing in propTypes of React component [1].

onResponderMove={this.touchableHandleResponderMove}
onResponderRelease={this.touchableHandleResponderRelease}
onResponderTerminate={this.touchableHandleResponderTerminate}>
{this.props.children}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get this.props.children because property children is missing in propTypes of React component [1].

{this.props.children}
{Touchable.renderDebugView({
color: 'red',
hitSlop: this.props.hitSlop

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

{this.props.children}
{Touchable.renderDebugView({
color: 'red',
hitSlop: this.props.hitSlop

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact object type [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

nativeID={this.props.nativeID}
testID={this.props.testID}
onLayout={this.props.onLayout}
hitSlop={this.props.hitSlop}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot create View element because inexact object type [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

onResponderMove={this.touchableHandleResponderMove}
onResponderRelease={this.touchableHandleResponderRelease}
onResponderTerminate={this.touchableHandleResponderTerminate}>
{this.props.children}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get this.props.children because property children is missing in propTypes of React component [1].

onResponderRelease={this.touchableHandleResponderRelease}
onResponderTerminate={this.touchableHandleResponderTerminate}>
{this.props.children}
{Touchable.renderDebugView({color: 'red', hitSlop: this.props.hitSlop})}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

onResponderRelease={this.touchableHandleResponderRelease}
onResponderTerminate={this.touchableHandleResponderTerminate}>
{this.props.children}
{Touchable.renderDebugView({color: 'red', hitSlop: this.props.hitSlop})}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact object type [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.
  • flow found some issues.

Libraries/Components/Touchable/TouchableWithoutFeedback.js Outdated Show resolved Hide resolved
Libraries/Components/Touchable/TouchableWithoutFeedback.js Outdated Show resolved Hide resolved
onResponderMove={that.touchableHandleResponderMove}
onResponderRelease={that.touchableHandleResponderRelease}
onResponderTerminate={that.touchableHandleResponderTerminate}>
{that.props.children}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get that.props.children because property children is missing in propTypes of React component [1].

onResponderRelease={that.touchableHandleResponderRelease}
onResponderTerminate={that.touchableHandleResponderTerminate}>
{that.props.children}
{Touchable.renderDebugView({color: 'red', hitSlop: that.props.hitSlop})}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

onResponderRelease={that.touchableHandleResponderRelease}
onResponderTerminate={that.touchableHandleResponderTerminate}>
{that.props.children}
{Touchable.renderDebugView({color: 'red', hitSlop: that.props.hitSlop})}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact object type [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

nativeID={that.props.nativeID}
testID={that.props.testID}
onLayout={that.props.onLayout}
hitSlop={that.props.hitSlop}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot create View element because inexact object type [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

onResponderMove={that.touchableHandleResponderMove}
onResponderRelease={that.touchableHandleResponderRelease}
onResponderTerminate={that.touchableHandleResponderTerminate}>
{that.props.children}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get that.props.children because property children is missing in propTypes of React component [1].

onResponderRelease={that.touchableHandleResponderRelease}
onResponderTerminate={that.touchableHandleResponderTerminate}>
{that.props.children}
{Touchable.renderDebugView({color: 'red', hitSlop: that.props.hitSlop})}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

onResponderRelease={that.touchableHandleResponderRelease}
onResponderTerminate={that.touchableHandleResponderTerminate}>
{that.props.children}
{Touchable.renderDebugView({color: 'red', hitSlop: that.props.hitSlop})}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact object type [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 12, 2019
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

onStartShouldSetResponder={this.touchableHandleStartShouldSetResponder}
onResponderTerminationRequest={
this.touchableHandleResponderTerminationRequest
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get this.props.children because property children is missing in propTypes of React component [1].

}
onResponderGrant={this.touchableHandleResponderGrant}
onResponderMove={this.touchableHandleResponderMove}
onResponderRelease={this.touchableHandleResponderRelease}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.
  • flow found some issues.

onResponderTerminationRequest: this
.touchableHandleResponderTerminationRequest,
onResponderGrant: this.touchableHandleResponderGrant,
onResponderMove: this.touchableHandleResponderMove,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react/jsx-no-undef: 'Animated' is not defined.

@@ -60,7 +59,7 @@ export type Props = $ReadOnly<{|
delayPressIn?: ?number,
delayPressOut?: ?number,
disabled?: ?boolean,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use exports as a type because exports is a value. To get the type of a value use typeof.

@@ -69,7 +68,7 @@ export type Props = $ReadOnly<{|
onPress?: ?(event: PressEvent) => mixed,
onPressIn?: ?(event: PressEvent) => mixed,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use exports as a type because exports is a value. To get the type of a value use typeof.

@@ -161,7 +160,7 @@ const TouchableWithoutFeedback = ((createReactClass({
* reactivated! Move it back and forth several times while the scroll view

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exports [1] is not a React propType.

@@ -170,7 +169,7 @@ const TouchableWithoutFeedback = ((createReactClass({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exports [1] is not a React propType.

onResponderTerminationRequest: this
.touchableHandleResponderTerminationRequest,
onResponderGrant: this.touchableHandleResponderGrant,
onResponderMove: this.touchableHandleResponderMove,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot resolve name Animated.

hitSlop={this.props.hitSlop}
onStartShouldSetResponder={this.touchableHandleStartShouldSetResponder}
onResponderTerminationRequest={
this.touchableHandleResponderTerminationRequest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get this.props.children because property children is missing in propTypes of React component [1].

this.touchableHandleResponderTerminationRequest
}
onResponderGrant={this.touchableHandleResponderGrant}
onResponderMove={this.touchableHandleResponderMove}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

@@ -60,7 +59,7 @@ export type Props = $ReadOnly<{|
delayPressIn?: ?number,
delayPressOut?: ?number,
disabled?: ?boolean,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use exports as a type because exports is a value. To get the type of a value use typeof.

@@ -69,7 +68,7 @@ export type Props = $ReadOnly<{|
onPress?: ?(event: PressEvent) => mixed,
onPressIn?: ?(event: PressEvent) => mixed,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use exports as a type because exports is a value. To get the type of a value use typeof.

}
onResponderGrant={this.touchableHandleResponderGrant}
onResponderMove={this.touchableHandleResponderMove}
onResponderRelease={this.touchableHandleResponderRelease}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

@@ -60,7 +61,7 @@ export type Props = $ReadOnly<{|
delayPressIn?: ?number,
delayPressOut?: ?number,
disabled?: ?boolean,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use exports as a type because exports is a value. To get the type of a value use typeof.

@@ -69,7 +70,7 @@ export type Props = $ReadOnly<{|
onPress?: ?(event: PressEvent) => mixed,
onPressIn?: ?(event: PressEvent) => mixed,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use exports as a type because exports is a value. To get the type of a value use typeof.

@@ -161,7 +162,7 @@ const TouchableWithoutFeedback = ((createReactClass({
* reactivated! Move it back and forth several times while the scroll view

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exports [1] is not a React propType.

@@ -170,7 +171,7 @@ const TouchableWithoutFeedback = ((createReactClass({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exports [1] is not a React propType.

hitSlop={this.props.hitSlop}
onStartShouldSetResponder={this.touchableHandleStartShouldSetResponder}
onResponderTerminationRequest={
this.touchableHandleResponderTerminationRequest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot get this.props.children because property children is missing in propTypes of React component [1].

this.touchableHandleResponderTerminationRequest
}
onResponderGrant={this.touchableHandleResponderGrant}
onResponderMove={this.touchableHandleResponderMove}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot call Touchable.renderDebugView with object literal bound to the first parameter because inexact undefined [1] is incompatible with exact EdgeInsetsProp [2] in property hitSlop.

@cpojer
Copy link
Contributor

cpojer commented Mar 13, 2019

Thanks for your PR. We are planning on deprecating these components in the near future and will be open sourcing a new way of handling pressable elements. I don't think it makes sense at this point to introduce such a big change to a component like this so I'm going to close this PR but feel free to use this in your own code :)

@mgcrea
Copy link

mgcrea commented Apr 25, 2019

This issue keeps re-surfacing, would have been nice to properly fix it as it breaks a lot of code, (eg. #23740).

I haven't been able to make this PR work (encounters a <View> inside <Text> error).

@cpojer Any idea when the new touchables will be released?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: TouchableWithoutFeedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants