Update opacity when `disabled` prop is changed #17106

maxkomarychev · 2017-12-07T09:45:53Z

If you render

    <TouchableOpacity
        disabled={true}
        style={{opacity: 0.5}}
    >
        ...
    </TouchableOpacity>

and then

    <TouchableOpacity
        disabled={false}
        style={{opacity: 1}}
    >
        ...
    </TouchableOpacity>

The content of TouchableOpacity will still have opacity = 0.5 because real
opacity is controlled by animated property which should be properly updated
when disabled prop changes.

Motivation

Usually when a button or other UI component is built with TouchableOpacity you may want to change it's opacity if state of component is changed (enabled/disabled). Opacity provided in props is overridden with internally-managed animated value. Add extra check when component is updated to trigger opacity animation upon change of disabled flag.

Test Plan

You can use code from #17105.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

Release Notes

[GENERAL][BUGFIX][TouchableOpacity] - trigger animation on opacity upon change in disabled prop.

facebook-github-bot · 2017-12-07T09:46:10Z

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!

pull-bot · 2017-12-07T09:47:10Z

@facebook-github-bot label Android

Generated by 🚫 dangerJS

facebook-github-bot · 2017-12-07T09:59:37Z

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

maxkomarychev · 2017-12-25T21:01:22Z

Hi @hramos, when do you think this PR can be reviewed? Thanks.

facebook-github-bot · 2018-01-06T18:04:22Z

@maxkomarychev I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

fixes facebook#17105 If you render ``` <TouchableOpacity disabled={true} style={{opacity: 0.5}} > ... </TouchableOpacity> ``` and then ``` <TouchableOpacity disabled={false} style={{opacity: 1}} > ... </TouchableOpacity> ``` The content of `TouchableOpacity` will still have opacity = 0.5 because real opacity is controlled by animated property which should be properly updated when `disabled` prop changes.

maxkomarychev · 2018-01-06T20:51:41Z

@dlowder-salesforce @lwinkyawmyat @levsero, could you please review or suggest who can do that? thanks.

maxkomarychev · 2018-01-08T08:58:11Z

@javache

jctf · 2018-01-15T19:08:25Z

Anyone reviewing this??

tobiastornros · 2018-01-21T10:50:47Z

Anyone reviewing this?

danilobuerger · 2018-01-26T18:22:38Z

This again, valid patch, nobody reviewing... :(

douglowder · 2018-01-26T18:56:38Z

I can review it, but I can't merge it.... maybe you can add release notes? @shergin might be the right person for this....

maxkomarychev · 2018-01-26T20:37:10Z

@dlowder-salesforce updated release notes, appreciate your feedback! thanks!

hramos · 2018-01-26T20:46:11Z

@danilobuerger there's over a hundred PRs that have been filed ahead of this one. Please be patient!

danilobuerger · 2018-01-26T20:50:45Z

@hramos Instead of being patient, maybe RN just needs more maintainers. Waiting for months to get something reviewed has a chilling effect on contributors. (Disclaimer: I know, this is open source, everybody is doing this on their free time. My point is: RN needs more maintainers).

hramos · 2018-01-26T21:01:31Z

Please consider the chilling effect you could have on existing maintainers as well. Fortunately, we do provide a path to contributing back to the repo: How to Contribute.

danilobuerger · 2018-01-26T21:05:34Z

@hramos

Please consider the chilling effect you could have on existing maintainers as well.

So I shouldn't speak up if I think something isn't right? How would things ever change if we all had to be quiet and tiptoe around each other?

Fortunately, we do provide a path to contributing back to the repo: How to Contribute.

I know how to contribute, but that's not the issue when having > 100 PRs. It's missing maintainers.

facebook-github-bot · 2018-02-06T18:04:44Z

@maxkomarychev I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

oferRounds · 2018-02-12T16:21:00Z

+1

douglowder · 2018-03-04T01:15:38Z

@maxkomarychev I'm going to review and test this PR.... I'm curious, does TouchableHighlight not have the same issue?

douglowder

Tested the fix, works well on both iOS and tvOS.

douglowder · 2018-03-05T20:57:21Z

@facebook-github-bot shipit

facebook-github-bot · 2018-03-05T20:57:45Z

Something went wrong executing that command, @hramos could you take a look?

facebook-github-bot

@dlowder-salesforce is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

maxkomarychev · 2018-03-06T07:27:56Z

Thanks a lot @dlowder-salesforce .
As for touchable highlight - I did not meet this kind of issue with that component, but I didn't work with it too much though.

ssuchanowski · 2018-11-09T09:27:13Z

has this been merged yet?

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 Dec 7, 2017

maxkomarychev mentioned this pull request Jan 3, 2018

TouchableOpacity does not let you control opacity when disabled prop changes #17105

Closed

maxkomarychev force-pushed the feature/toggle-opacity-when-disabled-changed branch from ff94f25 to 3b2469d Compare January 6, 2018 20:41

mmazzarolo mentioned this pull request Feb 22, 2018

TouchableOpacity does not respond immediately to changes in opacity property #17723

Closed

douglowder self-requested a review March 4, 2018 01:11

douglowder approved these changes Mar 5, 2018

View reviewed changes

facebook-github-bot added the Failed Commands label Mar 5, 2018

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Mar 5, 2018

facebook-github-bot approved these changes Mar 5, 2018

View reviewed changes

facebook-github-bot closed this in 9366ce4 Mar 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update opacity when `disabled` prop is changed #17106

Update opacity when `disabled` prop is changed #17106

maxkomarychev commented Dec 7, 2017 •

edited

Loading

facebook-github-bot commented Dec 7, 2017

pull-bot commented Dec 7, 2017 •

edited

Loading

facebook-github-bot commented Dec 7, 2017

maxkomarychev commented Dec 25, 2017

facebook-github-bot commented Jan 6, 2018

maxkomarychev commented Jan 6, 2018

maxkomarychev commented Jan 8, 2018

jctf commented Jan 15, 2018

tobiastornros commented Jan 21, 2018

danilobuerger commented Jan 26, 2018

douglowder commented Jan 26, 2018

maxkomarychev commented Jan 26, 2018

hramos commented Jan 26, 2018

danilobuerger commented Jan 26, 2018

hramos commented Jan 26, 2018

danilobuerger commented Jan 26, 2018

facebook-github-bot commented Feb 6, 2018

oferRounds commented Feb 12, 2018

douglowder commented Mar 4, 2018

douglowder left a comment

douglowder commented Mar 5, 2018

facebook-github-bot commented Mar 5, 2018

facebook-github-bot left a comment

maxkomarychev commented Mar 6, 2018

ssuchanowski commented Nov 9, 2018

Update opacity when disabled prop is changed #17106

Update opacity when disabled prop is changed #17106

Conversation

maxkomarychev commented Dec 7, 2017 • edited Loading

Motivation

Test Plan

Related PRs

Release Notes

facebook-github-bot commented Dec 7, 2017

pull-bot commented Dec 7, 2017 • edited Loading

facebook-github-bot commented Dec 7, 2017

maxkomarychev commented Dec 25, 2017

facebook-github-bot commented Jan 6, 2018

maxkomarychev commented Jan 6, 2018

maxkomarychev commented Jan 8, 2018

jctf commented Jan 15, 2018

tobiastornros commented Jan 21, 2018

danilobuerger commented Jan 26, 2018

douglowder commented Jan 26, 2018

maxkomarychev commented Jan 26, 2018

hramos commented Jan 26, 2018

danilobuerger commented Jan 26, 2018

hramos commented Jan 26, 2018

danilobuerger commented Jan 26, 2018

facebook-github-bot commented Feb 6, 2018

oferRounds commented Feb 12, 2018

douglowder commented Mar 4, 2018

douglowder left a comment

Choose a reason for hiding this comment

douglowder commented Mar 5, 2018

facebook-github-bot commented Mar 5, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

maxkomarychev commented Mar 6, 2018

ssuchanowski commented Nov 9, 2018

Update opacity when `disabled` prop is changed #17106

Update opacity when `disabled` prop is changed #17106

maxkomarychev commented Dec 7, 2017 •

edited

Loading

pull-bot commented Dec 7, 2017 •

edited

Loading