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

[Touchable] easy way to disable a Touchable* highlight? #2103

Closed
gre opened this issue Jul 23, 2015 · 10 comments
Closed

[Touchable] easy way to disable a Touchable* highlight? #2103

gre opened this issue Jul 23, 2015 · 10 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@gre
Copy link
Contributor

gre commented Jul 23, 2015

Hi,

the current pattern I've been doing for making Buttons that can be "disabled" is to do the following:

return !disabled ? <TouchableOpacity onPress={onPress}>{btn}</TouchableOpacity> : btn;

I need to do that because a disabled button should not have any "highlight" effect.

The problem with such approach is it changes the component tree which means btn will be re-instanciated every time disabled changes. One of the consequence is that if btn wants to do animation, the animation will be suddenly stopped.


Here is my proposal to fix that,

what if I could do that?

return <TouchableOpacity onPress={!disabled ? onPress : null}>{btn}</TouchableOpacity>;

I'm proposing that if onPress is falsy, there should be no highlight effect at all on a Touchable* components.
This would also make the code more maintainable because you don't have to branch your logic.

What do you think?

@ide
Copy link
Contributor

ide commented Jul 23, 2015

Try setting the key prop on your button and using your original approach.

@gre
Copy link
Contributor Author

gre commented Jul 23, 2015

I'm not sure to get how setting key would change anything. btn will change its place in the tree every time disabled change so there will be a new instantiation (and all the related issues).

Also to me, the fact that the button has highlight is very correlated to the fact that you give an onPress handler. Actually I don't see why you would expect a Touchable effect if no action is listened.

I've used this pattern a lot in my react apps, using an handler not only as a callback to call but also as a way to know that the user is listening (so you can adapt the UX without introducing an extra prop).

@browniefed
Copy link
Contributor

Can you check if it is disabled in the press handler? If you are creating a button just create a handlePress inside this button component then call the props.onPress if it isn't disabled.
Also maybe set the activeOpacity to 1 if it is disabled so on press it doesn't look like it's doing anything?

@brentvatne brentvatne changed the title easy way to disable a Touchable* highlight? [Touchable] easy way to disable a Touchable* highlight? Jul 23, 2015
@Intellicode
Copy link
Contributor

This feature would be handy for me as well

@gre
Copy link
Contributor Author

gre commented Jan 2, 2016

to summary my need, I don't expect Touchable* to highlight if no valid handler are provided.
A simple way to implement this is to use activeOpacity=1 if all on*Press handlers are falsy (maybe better is to completely disable the animation to trigger, i'm not aware of the complexity to do this).

If you agree with this I can provide a PR.

@ShawnKuan
Copy link

@gre how to disable the animation when press on it

@Kureev
Copy link
Contributor

Kureev commented Feb 15, 2016

What if we can patch Touchable mixin?

touchableHandleStartShouldSetResponder: function() {
  return !this.props.disabled;
},

should do the trick. I can send over a PR along with adding a new propType

disabled: React.PropTypes.bool

to TouchableWithoutFeedback base class. What do you think?

@gre
Copy link
Contributor Author

gre commented Feb 15, 2016

sounds perfect :) is disabled also on all the Touchable components? (since it's a base class)

@Kureev
Copy link
Contributor

Kureev commented Feb 15, 2016

@gre it should

@ghost ghost closed this as completed in 9951e1a Feb 24, 2016
pglotov pushed a commit to pglotov/react-native that referenced this issue 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
@TheoGit
Copy link

TheoGit commented Sep 7, 2016

Awesome!

@facebook facebook locked as resolved and limited conversation to collaborators Jul 22, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

8 participants