Skip to content

Conversation

mganandraj
Copy link
Contributor

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.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Mar 8, 2021
@pull-bot
Copy link

pull-bot commented Mar 8, 2021

Messages
📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

📖 📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.

Generated by 🚫 dangerJS against 0877a36

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 8b821f8

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,904,908 +36
android hermes armeabi-v7a 8,402,014 +31
android hermes x86 9,393,783 +28
android hermes x86_64 9,337,989 +44
android jsc arm64-v8a 10,638,972 +44
android jsc armeabi-v7a 10,119,176 +41
android jsc x86 10,689,343 +47
android jsc x86_64 11,273,802 +40

Base commit: 8b821f8

@hank121314
Copy link
Contributor

Might be duplicated #29830 .

@lunaleaps
Copy link
Contributor

@mganandraj Can we confirm if we still need this given that #29830 landed?

@mganandraj
Copy link
Contributor Author

@mganandraj Can we confirm if we still need this given that #29830 landed?

Hei.. this PR can now be closed .. thanks for following up on this..

@github-actions github-actions bot added Needs: Attention Issues where the author has responded to feedback. and removed Needs: Author Feedback labels Sep 11, 2021
@cortinico cortinico closed this Sep 13, 2021
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. Needs: Attention Issues where the author has responded to feedback. p: Microsoft Partner: Microsoft Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants