Fixing resetting image tintColor on Android#31114
Closed
mganandraj wants to merge 1 commit intofacebook:mainfrom
Closed
Fixing resetting image tintColor on Android#31114mganandraj wants to merge 1 commit intofacebook:mainfrom
mganandraj wants to merge 1 commit intofacebook:mainfrom
Conversation
|
Base commit: 8b821f8 |
Base commit: 8b821f8 |
Contributor
|
Might be duplicated #29830 . |
4 tasks
Contributor
|
@mganandraj Can we confirm if we still need this given that #29830 landed? |
Contributor
Author
Hei.. this PR can now be closed .. thanks for following up on this.. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Android, trying to reset the 'tintColor' prop of an Image (by removing the prop from the component) is not removing the color filter from the native view, but tries to set the filter value to artibitrary default value. In our feature (just upgraded to RN 0.63), it results in the image going invisible as the view manager tries to set the color filter to the default value of '0'.
Attaching a snippet below with the repro.
The image view manager assumes that the react JS sents the prop updates with value set to 'null' when the intention is to remove the attribute. This PR by @tom-un introduced a change in behavior to send a default value when the prop value from JS is set to null. The change affects a couple of other properties too.
This change introduces a new PropSetter which allows null values and associates it with props which expects null values.
.........................................
class MyTouchable extends React.Component {
constructor(props) {
super(props);
this.state = {imageStyle: [{tintColor: '#D2D2D2'}]};
}
_onPressButton() {
this.setState({imageStyle: [undefined]});
}
render() {
return (
<Image
source={require('../../assets/uie_thumb_normal.png')}
style={this.state.imageStyle}
/>
);
}
}
Changelog
On Android, resetting the 'tintColor' prop of an Image is not removing the color filter. In our feature, it results in the image going invisible as the view manager tried to set the color filter to the default value of '0'.
The image view manager assumes that the react sends the prop updates with value set to 'null' when the intention is to remove the attribute. This PR by @tom-un introduced a change in behavior to send a default value when the prop value from JS is set to null.
[CATEGORY] [TYPE] - Message
Test Plan
Fixing a regression. Not a new behaviour or API. Verified that the behaviour of the Image component is identical to RN62.