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

Introduce disabling for Touchable* elements #5931

Closed

Conversation

Kureev
Copy link
Contributor

@Kureev Kureev commented Feb 15, 2016

Introduce a disabled property for Touchable* components.

Fix #2103

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @sahrens, @jutaz and @spicyj to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 15, 2016
@satya164
Copy link
Contributor

cc @mkonicek

@facebook-github-bot
Copy link
Contributor

@Kureev updated the pull request.

@Kureev
Copy link
Contributor Author

Kureev commented Feb 19, 2016

@facebook-github-bot liar!

@@ -41,6 +41,10 @@ var TouchableWithoutFeedback = React.createClass({
React.PropTypes.arrayOf(React.PropTypes.oneOf(View.AccessibilityTraits)),
]),
/**
* Disable all component interactions if value is true
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if the value is true / If true, disable all interactions for this component.

@mkonicek
Copy link
Contributor

Does this work also for TouchableHighlight, TouchableBounce, TouchableNativeFeedback etc.?

Do they still animate if you set disabled to true? Do you want them to animate? Should TouchableNativeFeedback (Android-only) animate when disabled?

You could try this in the TouchableExample and ideally add examples there if this approach works well and disables the animations.

@Kureev
Copy link
Contributor Author

Kureev commented Feb 21, 2016

Will do!

@mkonicek
Copy link
Contributor

👍 thank you.

@Kureev Kureev force-pushed the feature/disable-touchable-highlight branch from 76bbd1c to bb304f9 Compare February 22, 2016 00:32
@Kureev
Copy link
Contributor Author

Kureev commented Feb 22, 2016

@mkonicek I've added a "Disabled Touchable*" example to UIExplorer:
image
It works with all Touchables (all of them inherits from TouchableWithoutFeedback which use Touchable mixin I've patched by disabled flag).

Does this work also for TouchableHighlight, TouchableBounce, TouchableNativeFeedback etc.?

Yes

Do they still animate if you set disabled to true?

No

Do you want them to animate?

No, it should behave like a usual text when disabled

Should TouchableNativeFeedback (Android-only) animate when disabled?

No

@facebook-github-bot
Copy link
Contributor

@Kureev updated the pull request.

animationVelocity={0}
underlayColor="rgb(210, 230, 255)"
style={[styles.row, styles.block]}
onPress={() => console.log('custom THW text - hightlight')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: highlight

@mkonicek
Copy link
Contributor

OK, let's shipit.

The test plan doesn't show whether this disables animations for TouchableNativeFeedback but we can fix that later if it still animates with this PR.

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 9951e1a Feb 24, 2016
@Kureev
Copy link
Contributor Author

Kureev commented Feb 24, 2016

I'll make a 2 follow-up PRs:

Thanks for merging this one in, @mkonicek

ghost pushed a commit that referenced this pull request Feb 24, 2016
Summary:Follow-up PR for fixing typos in #5931

**Test plan (required)**
This PR contains only copy changes, so no additional testing is required.

**Code formatting**

This PR code style match the desired [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).

cc mkonicek
Closes #6122

Differential Revision: D2970751

Pulled By: javache

fb-gh-sync-id: 007ab6e732e88f39c7f392f00b2bd668ebb4d7d5
shipit-source-id: 007ab6e732e88f39c7f392f00b2bd668ebb4d7d5
ghost pushed a commit that referenced this pull request Feb 24, 2016
Summary:Follow-up PR for #5931: Adding missing `TouchableNativeFeedback` example.

**Test plan**

- Run UIExplorer on iOS and Android devices

[Android]
- Go to Touchable* examples and scroll down to the `Disabled Touchable*` section. There are two new buttons there: `Enabled TouchableNativeFeedback` and `Disabled TouchableNativeFeedback`. Buttons should behave according their titles.

[iOS]
- Go to Touchable* examples and scroll down to the `Disabled Touchable*` section. As far as `TouchableNativeFeedback` is supported on Android only, you **wouldn't see** any new buttons there

**Code formatting**

This PR code style match the desired [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).

cc mkonicek
Closes #6123

Differential Revision: D2971021

fb-gh-sync-id: 3be18bda15b5b495caaf9e4444fd0deb47a44839
shipit-source-id: 3be18bda15b5b495caaf9e4444fd0deb47a44839
@mkonicek
Copy link
Contributor

💯 Awesome, thanks @Kureev!

@Kureev
Copy link
Contributor Author

Kureev commented Feb 24, 2016

haha, you're welcome, @mkonicek 😉

pglotov pushed a commit to pglotov/react-native that referenced this pull request Mar 15, 2016
Summary:Introduce a `disabled` property for Touchable* components.

Fix facebook#2103
Closes facebook#5931

Differential Revision: D2969790

Pulled By: mkonicek

fb-gh-sync-id: 570a4ff882219f52f2d2ebef3eb73b1df50a0877
shipit-source-id: 570a4ff882219f52f2d2ebef3eb73b1df50a0877
pglotov pushed a commit to pglotov/react-native that referenced this pull request Mar 15, 2016
Summary:Follow-up PR for fixing typos in facebook#5931

**Test plan (required)**
This PR contains only copy changes, so no additional testing is required.

**Code formatting**

This PR code style match the desired [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).

cc mkonicek
Closes facebook#6122

Differential Revision: D2970751

Pulled By: javache

fb-gh-sync-id: 007ab6e732e88f39c7f392f00b2bd668ebb4d7d5
shipit-source-id: 007ab6e732e88f39c7f392f00b2bd668ebb4d7d5
pglotov pushed a commit to pglotov/react-native that referenced this pull request Mar 15, 2016
Summary:Follow-up PR for facebook#5931: Adding missing `TouchableNativeFeedback` example.

**Test plan**

- Run UIExplorer on iOS and Android devices

[Android]
- Go to Touchable* examples and scroll down to the `Disabled Touchable*` section. There are two new buttons there: `Enabled TouchableNativeFeedback` and `Disabled TouchableNativeFeedback`. Buttons should behave according their titles.

[iOS]
- Go to Touchable* examples and scroll down to the `Disabled Touchable*` section. As far as `TouchableNativeFeedback` is supported on Android only, you **wouldn't see** any new buttons there

**Code formatting**

This PR code style match the desired [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).

cc mkonicek
Closes facebook#6123

Differential Revision: D2971021

fb-gh-sync-id: 3be18bda15b5b495caaf9e4444fd0deb47a44839
shipit-source-id: 3be18bda15b5b495caaf9e4444fd0deb47a44839
rozele referenced this pull request in microsoft/react-native-windows May 25, 2016
Summary:Follow-up PR for fixing typos in #5931

**Test plan (required)**
This PR contains only copy changes, so no additional testing is required.

**Code formatting**

This PR code style match the desired [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).

cc mkonicek
Closes facebook/react-native#6122

Differential Revision: D2970751

Pulled By: javache

fb-gh-sync-id: 007ab6e732e88f39c7f392f00b2bd668ebb4d7d5
shipit-source-id: 007ab6e732e88f39c7f392f00b2bd668ebb4d7d5
rozele referenced this pull request in microsoft/react-native-windows May 25, 2016
Summary:Follow-up PR for #5931: Adding missing `TouchableNativeFeedback` example.

**Test plan**

- Run UIExplorer on iOS and Android devices

[Android]
- Go to Touchable* examples and scroll down to the `Disabled Touchable*` section. There are two new buttons there: `Enabled TouchableNativeFeedback` and `Disabled TouchableNativeFeedback`. Buttons should behave according their titles.

[iOS]
- Go to Touchable* examples and scroll down to the `Disabled Touchable*` section. As far as `TouchableNativeFeedback` is supported on Android only, you **wouldn't see** any new buttons there

**Code formatting**

This PR code style match the desired [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).

cc mkonicek
Closes facebook/react-native#6123

Differential Revision: D2971021

fb-gh-sync-id: 3be18bda15b5b495caaf9e4444fd0deb47a44839
shipit-source-id: 3be18bda15b5b495caaf9e4444fd0deb47a44839
This pull request was closed.
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants