From 5bc7f0441d588981750bdaec63e1ffaeffb0ec6f Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 16 Dec 2019 18:11:33 -0800 Subject: [PATCH] Ensure that TextAttribute prop parsing in BaseTextProps uses proper defaults Summary: Motivation: in Marketplace, there's a TextInput in JS that sends { lineHeight: null } to C++. In Paper this was correctly handled as a default value, which just causes that property to not be set. In C++ in Fabric, it was being parsed incorrectly as 0 (because of incorrect defaults being passed into `convertRawProp`), causing the text to not be visible at all. The solution is to make sure that its default value is NaN, which is handled correctly, and causes the Text to render again. Changelog: [Internal] Reviewed By: shergin Differential Revision: D19128049 fbshipit-source-id: fe985428f3ed8b90d56cfa387fbc2d1476d19d36 --- .../fabric/attributedstring/TextAttributes.h | 5 + .../text/basetext/BaseTextProps.cpp | 112 ++++++++++++++---- 2 files changed, 91 insertions(+), 26 deletions(-) diff --git a/ReactCommon/fabric/attributedstring/TextAttributes.h b/ReactCommon/fabric/attributedstring/TextAttributes.h index 734d898cf12..1c28424fad6 100644 --- a/ReactCommon/fabric/attributedstring/TextAttributes.h +++ b/ReactCommon/fabric/attributedstring/TextAttributes.h @@ -71,6 +71,11 @@ class TextAttributes : public DebugStringConvertible { // Special folly::Optional isHighlighted{}; + + // TODO T59221129: document where this value comes from and how it is set. + // It's not clear if this is being used properly, or if it's being set at all. + // Currently, it is intentionally *not* being set as part of BaseTextProps + // construction. folly::Optional layoutDirection{}; #pragma mark - Operations diff --git a/ReactCommon/fabric/components/text/basetext/BaseTextProps.cpp b/ReactCommon/fabric/components/text/basetext/BaseTextProps.cpp index 383868a4f23..12b53d2da05 100644 --- a/ReactCommon/fabric/components/text/basetext/BaseTextProps.cpp +++ b/ReactCommon/fabric/components/text/basetext/BaseTextProps.cpp @@ -17,74 +17,131 @@ namespace react { static TextAttributes convertRawProp( const RawProps &rawProps, + const TextAttributes sourceTextAttributes, const TextAttributes defaultTextAttributes) { auto textAttributes = TextAttributes{}; // Color - textAttributes.foregroundColor = - convertRawProp(rawProps, "color", defaultTextAttributes.foregroundColor); + textAttributes.foregroundColor = convertRawProp( + rawProps, + "color", + sourceTextAttributes.foregroundColor, + defaultTextAttributes.foregroundColor); textAttributes.backgroundColor = convertRawProp( - rawProps, "backgroundColor", defaultTextAttributes.backgroundColor); - textAttributes.opacity = - convertRawProp(rawProps, "opacity", defaultTextAttributes.opacity); + rawProps, + "backgroundColor", + sourceTextAttributes.backgroundColor, + defaultTextAttributes.backgroundColor); + textAttributes.opacity = convertRawProp( + rawProps, + "opacity", + sourceTextAttributes.opacity, + defaultTextAttributes.opacity); // Font - textAttributes.fontFamily = - convertRawProp(rawProps, "fontFamily", defaultTextAttributes.fontFamily); - textAttributes.fontSize = - convertRawProp(rawProps, "fontSize", defaultTextAttributes.fontSize); + textAttributes.fontFamily = convertRawProp( + rawProps, + "fontFamily", + sourceTextAttributes.fontFamily, + defaultTextAttributes.fontFamily); + textAttributes.fontSize = convertRawProp( + rawProps, + "fontSize", + sourceTextAttributes.fontSize, + defaultTextAttributes.fontSize); textAttributes.fontSizeMultiplier = convertRawProp( - rawProps, "fontSizeMultiplier", defaultTextAttributes.fontSizeMultiplier); - textAttributes.fontWeight = - convertRawProp(rawProps, "fontWeight", defaultTextAttributes.fontWeight); - textAttributes.fontStyle = - convertRawProp(rawProps, "fontStyle", defaultTextAttributes.fontStyle); + rawProps, + "fontSizeMultiplier", + sourceTextAttributes.fontSizeMultiplier, + defaultTextAttributes.fontSizeMultiplier); + textAttributes.fontWeight = convertRawProp( + rawProps, + "fontWeight", + sourceTextAttributes.fontWeight, + defaultTextAttributes.fontWeight); + textAttributes.fontStyle = convertRawProp( + rawProps, + "fontStyle", + sourceTextAttributes.fontStyle, + defaultTextAttributes.fontStyle); textAttributes.fontVariant = convertRawProp( - rawProps, "fontVariant", defaultTextAttributes.fontVariant); + rawProps, + "fontVariant", + sourceTextAttributes.fontVariant, + defaultTextAttributes.fontVariant); textAttributes.allowFontScaling = convertRawProp( - rawProps, "allowFontScaling", defaultTextAttributes.allowFontScaling); + rawProps, + "allowFontScaling", + sourceTextAttributes.allowFontScaling, + defaultTextAttributes.allowFontScaling); textAttributes.letterSpacing = convertRawProp( - rawProps, "letterSpacing", defaultTextAttributes.letterSpacing); + rawProps, + "letterSpacing", + sourceTextAttributes.letterSpacing, + defaultTextAttributes.letterSpacing); // Paragraph - textAttributes.lineHeight = - convertRawProp(rawProps, "lineHeight", defaultTextAttributes.lineHeight); - textAttributes.alignment = - convertRawProp(rawProps, "textAlign", defaultTextAttributes.alignment); + textAttributes.lineHeight = convertRawProp( + rawProps, + "lineHeight", + sourceTextAttributes.lineHeight, + defaultTextAttributes.lineHeight); + textAttributes.alignment = convertRawProp( + rawProps, + "textAlign", + sourceTextAttributes.alignment, + defaultTextAttributes.alignment); textAttributes.baseWritingDirection = convertRawProp( rawProps, "baseWritingDirection", + sourceTextAttributes.baseWritingDirection, defaultTextAttributes.baseWritingDirection); // Decoration textAttributes.textDecorationColor = convertRawProp( rawProps, "textDecorationColor", + sourceTextAttributes.textDecorationColor, defaultTextAttributes.textDecorationColor); textAttributes.textDecorationLineType = convertRawProp( rawProps, "textDecorationLine", + sourceTextAttributes.textDecorationLineType, defaultTextAttributes.textDecorationLineType); textAttributes.textDecorationLineStyle = convertRawProp( rawProps, "textDecorationLineStyle", + sourceTextAttributes.textDecorationLineStyle, defaultTextAttributes.textDecorationLineStyle); textAttributes.textDecorationLinePattern = convertRawProp( rawProps, "textDecorationLinePattern", + sourceTextAttributes.textDecorationLinePattern, defaultTextAttributes.textDecorationLinePattern); // Shadow textAttributes.textShadowOffset = convertRawProp( - rawProps, "textShadowOffset", defaultTextAttributes.textShadowOffset); + rawProps, + "textShadowOffset", + sourceTextAttributes.textShadowOffset, + defaultTextAttributes.textShadowOffset); textAttributes.textShadowRadius = convertRawProp( - rawProps, "textShadowRadius", defaultTextAttributes.textShadowRadius); + rawProps, + "textShadowRadius", + sourceTextAttributes.textShadowRadius, + defaultTextAttributes.textShadowRadius); textAttributes.textShadowColor = convertRawProp( - rawProps, "textShadowColor", defaultTextAttributes.textShadowColor); + rawProps, + "textShadowColor", + sourceTextAttributes.textShadowColor, + defaultTextAttributes.textShadowColor); // Special textAttributes.isHighlighted = convertRawProp( - rawProps, "isHighlighted", defaultTextAttributes.isHighlighted); + rawProps, + "isHighlighted", + sourceTextAttributes.isHighlighted, + defaultTextAttributes.isHighlighted); return textAttributes; } @@ -92,7 +149,10 @@ static TextAttributes convertRawProp( BaseTextProps::BaseTextProps( const BaseTextProps &sourceProps, const RawProps &rawProps) - : textAttributes(convertRawProp(rawProps, sourceProps.textAttributes)){}; + : textAttributes(convertRawProp( + rawProps, + sourceProps.textAttributes, + TextAttributes{})){}; #pragma mark - DebugStringConvertible