Commit 315ea5d
fix(android): props 2.0 image tintColor=transparent broken (#57668)
Summary:
Pull Request resolved: #57668
When enabling the props 2.0 feature flags I noticed that for an `<Image source={...} style={{ tintColor: 'transparent' }} />` the image is actually still showing, instead of becoming transparent.
### The underlying issue
All color props are defined as `SharedColor`, where a `SharedColor` has a default value of `0`:
https://github.com/facebook/react-native/blob/f3678f51d9873cb19602d7e36a4d8ed71562b9d0/packages/react-native/ReactCommon/react/renderer/graphics/platform/android/react/renderer/graphics/HostPlatformColor.h#L15-L18
https://github.com/facebook/react-native/blob/f3678f51d9873cb19602d7e36a4d8ed71562b9d0/packages/react-native/ReactCommon/react/renderer/graphics/Color.h#L30-L32
> [!NOTE]
> This is a bit confusing to me. `0` is not really an "undefined" color, but its actually "transparent". What we are really saying this way is that all color props have a default value of "transparent". The naming makes me unsure whether this has been intentional.
For other use cases, this seems to make sense. Ie. a user expects their text to have a background color of transparent/"nothing". However, we do not expect our image's tint color to have a default color of "transparent". This would hide all our images.
With props 2.0 we use the `getDiffProp` function, and when we pass `tintColor: 'transparent'` it will be passed to native as `tintColor: 0`. When we then compare the passed prop's value vs the default value here, it will not include the `tintColor`:
https://github.com/facebook/react-native/blob/f3678f51d9873cb19602d7e36a4d8ed71562b9d0/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp#L249-L251
`tintColor` really is an optional color prop, and should be treated as such. The best fix I found was therefor making it really an `std::optional`. Let me know if you think otherwise!
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[ANDROID] [CHANGED] - ImageProps make `tintColor` an `std::optional` to support color `transparent` (`0`) with props 2.0
X-link: #55535
Test Plan:
- In the RNTester app change one of the Tint Color image examples to use tintColor of "transparent"
- The image should be invisible now as all visible pixels turned transparent
- Enable the props 2.0 feature flags
- Run the same example, notice that the tintColor has not been applied
Reviewed By: lenaic
Differential Revision: D93140534
Pulled By: bartlomiejbloniarz1 parent e07fdde commit 315ea5d
14 files changed
Lines changed: 60 additions & 23 deletions
File tree
- packages/react-native
- ReactCommon/react/renderer
- components/image
- imagemanager/platform/android/react/renderer/imagemanager
- React/Fabric/Mounting/ComponentViews/Image
- scripts/cxx-api/api-snapshots
Lines changed: 11 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| 18 | + | |
17 | 19 | | |
18 | 20 | | |
19 | 21 | | |
| |||
63 | 65 | | |
64 | 66 | | |
65 | 67 | | |
66 | | - | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
67 | 77 | | |
68 | 78 | | |
69 | 79 | | |
| |||
Lines changed: 20 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| 13 | + | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
| |||
199 | 201 | | |
200 | 202 | | |
201 | 203 | | |
202 | | - | |
203 | | - | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
204 | 222 | | |
205 | 223 | | |
206 | 224 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
| 34 | + | |
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
58 | | - | |
| 58 | + | |
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| |||
Lines changed: 11 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
75 | 76 | | |
76 | 77 | | |
77 | 78 | | |
78 | | - | |
79 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
80 | 89 | | |
81 | 90 | | |
82 | 91 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2594 | 2594 | | |
2595 | 2595 | | |
2596 | 2596 | | |
2597 | | - | |
| 2597 | + | |
2598 | 2598 | | |
2599 | 2599 | | |
2600 | 2600 | | |
| |||
2616 | 2616 | | |
2617 | 2617 | | |
2618 | 2618 | | |
2619 | | - | |
| 2619 | + | |
2620 | 2620 | | |
2621 | 2621 | | |
2622 | 2622 | | |
| |||
2627 | 2627 | | |
2628 | 2628 | | |
2629 | 2629 | | |
2630 | | - | |
2631 | 2630 | | |
| 2631 | + | |
2632 | 2632 | | |
2633 | 2633 | | |
2634 | 2634 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2576 | 2576 | | |
2577 | 2577 | | |
2578 | 2578 | | |
2579 | | - | |
| 2579 | + | |
2580 | 2580 | | |
2581 | 2581 | | |
2582 | 2582 | | |
| |||
2598 | 2598 | | |
2599 | 2599 | | |
2600 | 2600 | | |
2601 | | - | |
| 2601 | + | |
2602 | 2602 | | |
2603 | 2603 | | |
2604 | 2604 | | |
| |||
2609 | 2609 | | |
2610 | 2610 | | |
2611 | 2611 | | |
2612 | | - | |
2613 | 2612 | | |
| 2613 | + | |
2614 | 2614 | | |
2615 | 2615 | | |
2616 | 2616 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2591 | 2591 | | |
2592 | 2592 | | |
2593 | 2593 | | |
2594 | | - | |
| 2594 | + | |
2595 | 2595 | | |
2596 | 2596 | | |
2597 | 2597 | | |
| |||
2613 | 2613 | | |
2614 | 2614 | | |
2615 | 2615 | | |
2616 | | - | |
| 2616 | + | |
2617 | 2617 | | |
2618 | 2618 | | |
2619 | 2619 | | |
| |||
2624 | 2624 | | |
2625 | 2625 | | |
2626 | 2626 | | |
2627 | | - | |
2628 | 2627 | | |
| 2628 | + | |
2629 | 2629 | | |
2630 | 2630 | | |
2631 | 2631 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5008 | 5008 | | |
5009 | 5009 | | |
5010 | 5010 | | |
5011 | | - | |
| 5011 | + | |
5012 | 5012 | | |
5013 | 5013 | | |
5014 | 5014 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4979 | 4979 | | |
4980 | 4980 | | |
4981 | 4981 | | |
4982 | | - | |
| 4982 | + | |
4983 | 4983 | | |
4984 | 4984 | | |
4985 | 4985 | | |
| |||
0 commit comments