Skip to content

Commit 715bf9c

Browse files
hannojgmeta-codesync[bot]
authored andcommitted
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: bartlomiejbloniarz
1 parent e2cd2bb commit 715bf9c

14 files changed

Lines changed: 60 additions & 23 deletions

File tree

packages/react-native/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
#import <React/RCTConversions.h>
1212
#import <React/RCTImageBlurUtils.h>
1313
#import <React/RCTImageResponseObserverProxy.h>
14+
#import <react/featureflags/ReactNativeFeatureFlags.h>
1415
#import <react/renderer/components/image/ImageComponentDescriptor.h>
1516
#import <react/renderer/components/image/ImageEventEmitter.h>
1617
#import <react/renderer/components/image/ImageProps.h>
18+
#import <react/renderer/graphics/Color.h>
1719
#import <react/renderer/imagemanager/ImageRequest.h>
1820
#import <react/renderer/imagemanager/RCTImagePrimitivesConversions.h>
1921

@@ -63,7 +65,15 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
6365

6466
// `tintColor`
6567
if (oldImageProps.tintColor != newImageProps.tintColor) {
66-
_imageView.tintColor = RCTUIColorFromSharedColor(newImageProps.tintColor);
68+
if (ReactNativeFeatureFlags::enableImageTransparentTintColor()) {
69+
if (newImageProps.tintColor.has_value()) {
70+
_imageView.tintColor = RCTUIColorFromSharedColor(newImageProps.tintColor.value());
71+
} else {
72+
_imageView.tintColor = nil;
73+
}
74+
} else {
75+
_imageView.tintColor = RCTUIColorFromSharedColor(newImageProps.tintColor.value_or(SharedColor{}));
76+
}
6777
}
6878

6979
[super updateProps:props oldProps:oldProps];

packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8+
#include <react/featureflags/ReactNativeFeatureFlags.h>
89
#include <react/renderer/components/image/ImageProps.h>
910
#include <react/renderer/components/image/conversions.h>
1011
#include <react/renderer/core/propsConversions.h>
1112
#include <react/renderer/debug/debugStringConvertibleUtils.h>
13+
#include <react/renderer/graphics/Color.h>
1214

1315
namespace facebook::react {
1416

@@ -199,8 +201,24 @@ folly::dynamic ImageProps::getDiffProps(const Props* prevProps) const {
199201
result["capInsets"] = convertEdgeInsets(capInsets);
200202
}
201203

202-
if (tintColor != oldProps->tintColor) {
203-
result["tintColor"] = *tintColor;
204+
if (ReactNativeFeatureFlags::enableImageTransparentTintColor()) {
205+
// New: emit any defined tint (including transparent); emit null to clear on
206+
// unset.
207+
if (tintColor != oldProps->tintColor) {
208+
if (tintColor.has_value()) {
209+
result["tintColor"] = *tintColor.value();
210+
} else {
211+
result["tintColor"] = folly::dynamic(nullptr);
212+
}
213+
}
214+
} else {
215+
// pre-`std::optional` behavior
216+
SharedColor tintColorValue = tintColor.value_or(SharedColor{});
217+
SharedColor prevTintColorValue =
218+
oldProps->tintColor.value_or(SharedColor{});
219+
if (tintColorValue != prevTintColorValue) {
220+
result["tintColor"] = *tintColorValue;
221+
}
204222
}
205223

206224
if (internal_analyticTag != oldProps->internal_analyticTag) {

packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class ImageProps final : public ViewProps {
3131
ImageResizeMode resizeMode{ImageResizeMode::Stretch};
3232
Float blurRadius{};
3333
EdgeInsets capInsets{};
34-
SharedColor tintColor{};
34+
std::optional<SharedColor> tintColor{};
3535
std::string internal_analyticTag{};
3636
std::string resizeMethod{"auto"};
3737
Float resizeMultiplier{1.f};

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/android/react/renderer/imagemanager/ImageRequestParams.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class ImageRequestParams {
2626
Float resizeMultiplier,
2727
bool shouldNotifyLoadEvents,
2828
SharedColor overlayColor,
29-
SharedColor tintColor,
29+
std::optional<SharedColor> tintColor,
3030
Float fadeDuration,
3131
bool progressiveRenderingEnabled,
3232
ImageSource loadingIndicatorSource,
@@ -55,7 +55,7 @@ class ImageRequestParams {
5555
Float resizeMultiplier{};
5656
bool shouldNotifyLoadEvents{};
5757
SharedColor overlayColor{};
58-
SharedColor tintColor{};
58+
std::optional<SharedColor> tintColor{};
5959
Float fadeDuration{};
6060
bool progressiveRenderingEnabled{};
6161
ImageSource loadingIndicatorSource{};

packages/react-native/ReactCommon/react/renderer/imagemanager/platform/android/react/renderer/imagemanager/conversions.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#pragma once
99

10+
#include <react/featureflags/ReactNativeFeatureFlags.h>
1011
#include <react/renderer/core/graphicsConversions.h>
1112
#include <react/renderer/imagemanager/ImageRequestParams.h>
1213
#include <react/renderer/imagemanager/primitives.h>
@@ -75,8 +76,16 @@ inline void serializeImageRequestParams(MapBufferBuilder &builder, const ImageRe
7576
if (isColorMeaningful(imageRequestParams.overlayColor)) {
7677
builder.putInt(IS_KEY_OVERLAY_COLOR, toAndroidRepr(imageRequestParams.overlayColor));
7778
}
78-
if (isColorMeaningful(imageRequestParams.tintColor)) {
79-
builder.putInt(IS_KEY_TINT_COLOR, toAndroidRepr(imageRequestParams.tintColor));
79+
if (ReactNativeFeatureFlags::enableImageTransparentTintColor()) {
80+
if (imageRequestParams.tintColor.has_value()) {
81+
builder.putInt(IS_KEY_TINT_COLOR, toAndroidRepr(imageRequestParams.tintColor.value()));
82+
}
83+
} else {
84+
// pre-`std::optional` behavior
85+
SharedColor tintColor = imageRequestParams.tintColor.value_or(SharedColor{});
86+
if (isColorMeaningful(tintColor)) {
87+
builder.putInt(IS_KEY_TINT_COLOR, toAndroidRepr(tintColor));
88+
}
8089
}
8190
builder.putInt(IS_KEY_FADE_DURATION, static_cast<int32_t>(imageRequestParams.fadeDuration));
8291
builder.putBool(IS_KEY_PROGRESSIVE_RENDERING_ENABLED, imageRequestParams.progressiveRenderingEnabled);

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2594,7 +2594,7 @@ class facebook::react::ImageProps : public facebook::react::HostPlatformViewProp
25942594
public facebook::react::ImageSource loadingIndicatorSource;
25952595
public facebook::react::ImageSources sources;
25962596
public facebook::react::SharedColor overlayColor;
2597-
public facebook::react::SharedColor tintColor;
2597+
public std::optional<facebook::react::SharedColor> tintColor;
25982598
public std::string resizeMethod;
25992599
public virtual facebook::react::ComponentName getDiffPropsImplementationTarget() const override;
26002600
public virtual folly::dynamic getDiffProps(const facebook::react::Props* prevProps) const override;
@@ -2616,7 +2616,7 @@ class facebook::react::ImageRequest {
26162616

26172617
class facebook::react::ImageRequestParams {
26182618
public ImageRequestParams() = default;
2619-
public ImageRequestParams(facebook::react::Float blurRadius, facebook::react::ImageSource defaultSource, facebook::react::ImageResizeMode resizeMode, std::string resizeMethod, facebook::react::Float resizeMultiplier, bool shouldNotifyLoadEvents, facebook::react::SharedColor overlayColor, facebook::react::SharedColor tintColor, facebook::react::Float fadeDuration, bool progressiveRenderingEnabled, facebook::react::ImageSource loadingIndicatorSource, std::string analyticTag, facebook::react::Size size);
2619+
public ImageRequestParams(facebook::react::Float blurRadius, facebook::react::ImageSource defaultSource, facebook::react::ImageResizeMode resizeMode, std::string resizeMethod, facebook::react::Float resizeMultiplier, bool shouldNotifyLoadEvents, facebook::react::SharedColor overlayColor, std::optional<facebook::react::SharedColor> tintColor, facebook::react::Float fadeDuration, bool progressiveRenderingEnabled, facebook::react::ImageSource loadingIndicatorSource, std::string analyticTag, facebook::react::Size size);
26202620
public bool operator==(const facebook::react::ImageRequestParams& rhs) const = default;
26212621
public bool progressiveRenderingEnabled;
26222622
public bool shouldNotifyLoadEvents;
@@ -2627,8 +2627,8 @@ class facebook::react::ImageRequestParams {
26272627
public facebook::react::ImageSource defaultSource;
26282628
public facebook::react::ImageSource loadingIndicatorSource;
26292629
public facebook::react::SharedColor overlayColor;
2630-
public facebook::react::SharedColor tintColor;
26312630
public facebook::react::Size size;
2631+
public std::optional<facebook::react::SharedColor> tintColor;
26322632
public std::string analyticTag;
26332633
public std::string resizeMethod;
26342634
}

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,7 +2576,7 @@ class facebook::react::ImageProps : public facebook::react::HostPlatformViewProp
25762576
public facebook::react::ImageSource loadingIndicatorSource;
25772577
public facebook::react::ImageSources sources;
25782578
public facebook::react::SharedColor overlayColor;
2579-
public facebook::react::SharedColor tintColor;
2579+
public std::optional<facebook::react::SharedColor> tintColor;
25802580
public std::string resizeMethod;
25812581
public virtual facebook::react::ComponentName getDiffPropsImplementationTarget() const override;
25822582
public virtual folly::dynamic getDiffProps(const facebook::react::Props* prevProps) const override;
@@ -2598,7 +2598,7 @@ class facebook::react::ImageRequest {
25982598

25992599
class facebook::react::ImageRequestParams {
26002600
public ImageRequestParams() = default;
2601-
public ImageRequestParams(facebook::react::Float blurRadius, facebook::react::ImageSource defaultSource, facebook::react::ImageResizeMode resizeMode, std::string resizeMethod, facebook::react::Float resizeMultiplier, bool shouldNotifyLoadEvents, facebook::react::SharedColor overlayColor, facebook::react::SharedColor tintColor, facebook::react::Float fadeDuration, bool progressiveRenderingEnabled, facebook::react::ImageSource loadingIndicatorSource, std::string analyticTag, facebook::react::Size size);
2601+
public ImageRequestParams(facebook::react::Float blurRadius, facebook::react::ImageSource defaultSource, facebook::react::ImageResizeMode resizeMode, std::string resizeMethod, facebook::react::Float resizeMultiplier, bool shouldNotifyLoadEvents, facebook::react::SharedColor overlayColor, std::optional<facebook::react::SharedColor> tintColor, facebook::react::Float fadeDuration, bool progressiveRenderingEnabled, facebook::react::ImageSource loadingIndicatorSource, std::string analyticTag, facebook::react::Size size);
26022602
public bool operator==(const facebook::react::ImageRequestParams& rhs) const = default;
26032603
public bool progressiveRenderingEnabled;
26042604
public bool shouldNotifyLoadEvents;
@@ -2609,8 +2609,8 @@ class facebook::react::ImageRequestParams {
26092609
public facebook::react::ImageSource defaultSource;
26102610
public facebook::react::ImageSource loadingIndicatorSource;
26112611
public facebook::react::SharedColor overlayColor;
2612-
public facebook::react::SharedColor tintColor;
26132612
public facebook::react::Size size;
2613+
public std::optional<facebook::react::SharedColor> tintColor;
26142614
public std::string analyticTag;
26152615
public std::string resizeMethod;
26162616
}

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2591,7 +2591,7 @@ class facebook::react::ImageProps : public facebook::react::HostPlatformViewProp
25912591
public facebook::react::ImageSource loadingIndicatorSource;
25922592
public facebook::react::ImageSources sources;
25932593
public facebook::react::SharedColor overlayColor;
2594-
public facebook::react::SharedColor tintColor;
2594+
public std::optional<facebook::react::SharedColor> tintColor;
25952595
public std::string resizeMethod;
25962596
public virtual facebook::react::ComponentName getDiffPropsImplementationTarget() const override;
25972597
public virtual folly::dynamic getDiffProps(const facebook::react::Props* prevProps) const override;
@@ -2613,7 +2613,7 @@ class facebook::react::ImageRequest {
26132613

26142614
class facebook::react::ImageRequestParams {
26152615
public ImageRequestParams() = default;
2616-
public ImageRequestParams(facebook::react::Float blurRadius, facebook::react::ImageSource defaultSource, facebook::react::ImageResizeMode resizeMode, std::string resizeMethod, facebook::react::Float resizeMultiplier, bool shouldNotifyLoadEvents, facebook::react::SharedColor overlayColor, facebook::react::SharedColor tintColor, facebook::react::Float fadeDuration, bool progressiveRenderingEnabled, facebook::react::ImageSource loadingIndicatorSource, std::string analyticTag, facebook::react::Size size);
2616+
public ImageRequestParams(facebook::react::Float blurRadius, facebook::react::ImageSource defaultSource, facebook::react::ImageResizeMode resizeMode, std::string resizeMethod, facebook::react::Float resizeMultiplier, bool shouldNotifyLoadEvents, facebook::react::SharedColor overlayColor, std::optional<facebook::react::SharedColor> tintColor, facebook::react::Float fadeDuration, bool progressiveRenderingEnabled, facebook::react::ImageSource loadingIndicatorSource, std::string analyticTag, facebook::react::Size size);
26172617
public bool operator==(const facebook::react::ImageRequestParams& rhs) const = default;
26182618
public bool progressiveRenderingEnabled;
26192619
public bool shouldNotifyLoadEvents;
@@ -2624,8 +2624,8 @@ class facebook::react::ImageRequestParams {
26242624
public facebook::react::ImageSource defaultSource;
26252625
public facebook::react::ImageSource loadingIndicatorSource;
26262626
public facebook::react::SharedColor overlayColor;
2627-
public facebook::react::SharedColor tintColor;
26282627
public facebook::react::Size size;
2628+
public std::optional<facebook::react::SharedColor> tintColor;
26292629
public std::string analyticTag;
26302630
public std::string resizeMethod;
26312631
}

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5008,7 +5008,7 @@ class facebook::react::ImageProps : public facebook::react::HostPlatformViewProp
50085008
public facebook::react::ImageSource loadingIndicatorSource;
50095009
public facebook::react::ImageSources sources;
50105010
public facebook::react::SharedColor overlayColor;
5011-
public facebook::react::SharedColor tintColor;
5011+
public std::optional<facebook::react::SharedColor> tintColor;
50125012
public std::string resizeMethod;
50135013
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);
50145014
}

scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4979,7 +4979,7 @@ class facebook::react::ImageProps : public facebook::react::HostPlatformViewProp
49794979
public facebook::react::ImageSource loadingIndicatorSource;
49804980
public facebook::react::ImageSources sources;
49814981
public facebook::react::SharedColor overlayColor;
4982-
public facebook::react::SharedColor tintColor;
4982+
public std::optional<facebook::react::SharedColor> tintColor;
49834983
public std::string resizeMethod;
49844984
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);
49854985
}

0 commit comments

Comments
 (0)