Android Text: More robust logic for handling inherited text props (#22917)

Summary:
Purpose
----------

This commit fixes a bug and prepares us for adding support for the `maxContentSizeMultiplier` prop (it's currently only supported on iOS).

Details
----------

Today we don't explicitly track inheritance of text props. Instead we rely on `SpannableStringBuilder` to handle this for us. Consider this example:

```
<Text style={{fontSize: 10}}>
  <Text style={{letterSpacing: 5}}>
    ...
  </Text>
</Text>
```

In today's implementation, the inner text doesn't know about `fontSize` (i.e. its `mFontSize` instance variable is `Float.NaN`). But everything works properly because the outer `Text` told `SpannableStringBuilder` to apply the font size across the entire string of text.

However, today's approach breaks down when computing the value to apply to the `SpannableStringBuilder` depends on multiple props. Suppose that RN Android supported the `maxContentSizeMultiplier` prop. Then calculating the font size to apply to the `SpannableStringBuilder` would involve both the `fontSize` prop and the `maxContentSizeMultiplier` prop. If `fontSize` was set on an outer `Text` and `maxContentSizeMultiplier` was set on an inner `Text` then the inner `Text` wouldn't be able to calculate the font size to apply to the `SpannableStringBuilder` because the outer `Text's` `fontSize` prop isn't available to it.

The `TextAttributes` class solves this problem. Every `Text` has a `TextAttributes` instance which stores its text props. During rendering, a child's `TextAttributes` is combined with its parent's and handed down the tree. In this way, during rendering a `Text` has access to the relevant text props from itself and its ancestors.

This design is inspired by the [`RCTTextAttributes`](7197aa026b/Libraries/Text/RCTTextAttributes.m) class from RN iOS.

Bug Fix
----------

This refactoring happens to fix a bug. Today, when setting `fontSize` on nested Text, `allowFontScaling` is always treated as though it is true regardless of the value on the root `Text`. For example, the following snippets should render "hello" identically, Instead, the bottom snippet renders "hello" as though `allowFontScaling` is true.

```
<Text allowFontScaling={false} style={{fontSize: 50}}>hello</Text>
<Text allowFontScaling={false}><Text style={{fontSize: 50}}>hello</Text></Text>
```

(The repro assumes you've increased your device's font setting so that the font size multiplier is not 1.0.)

Introducing the `TextAttributes` class fixed this. It forced us to think about how inheritance should work for `allowFontScaling`. In the new implementation, `Text` components use the value of `allowFontScaling` from the outermost `Text` component. This matches the behavior on iOS (the `allowFontScaling` prop gets ignored on virtual text nodes because it doesn't appear [in this list](3749da1312/Libraries/Text/Text.js (L266-L269)).)
Pull Request resolved: https://github.com/facebook/react-native/pull/22917

Reviewed By: mdvacca

Differential Revision: D13630235

Pulled By: shergin

fbshipit-source-id: e58f458de4fc3cdcbec49c8e0509da51966ef93c
This commit is contained in:
Adam Comella 2019-01-14 16:23:19 -08:00 committed by Facebook Github Bot
parent e6f7d69428
commit 1bdb250906
6 changed files with 209 additions and 93 deletions

View File

@ -24,6 +24,7 @@ import com.facebook.react.uimanager.IllegalViewOperationException;
import com.facebook.react.uimanager.LayoutShadowNode;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactShadowNode;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.uimanager.ViewDefaults;
import com.facebook.react.uimanager.ViewProps;
import com.facebook.react.uimanager.annotations.ReactProp;
@ -86,15 +87,23 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
ReactBaseTextShadowNode textShadowNode,
SpannableStringBuilder sb,
List<SetSpanOperation> ops,
TextAttributes parentTextAttributes,
int start) {
TextAttributes textAttributes;
if (parentTextAttributes != null) {
textAttributes = parentTextAttributes.applyChild(textShadowNode.mTextAttributes);
} else {
textAttributes = textShadowNode.mTextAttributes;
}
for (int i = 0, length = textShadowNode.getChildCount(); i < length; i++) {
ReactShadowNode child = textShadowNode.getChildAt(i);
if (child instanceof ReactRawTextShadowNode) {
sb.append(((ReactRawTextShadowNode) child).getText());
} else if (child instanceof ReactBaseTextShadowNode) {
buildSpannedFromShadowNode((ReactBaseTextShadowNode) child, sb, ops, sb.length());
buildSpannedFromShadowNode((ReactBaseTextShadowNode) child, sb, ops, textAttributes, sb.length());
} else if (child instanceof ReactTextInlineImageShadowNode) {
// We make the image take up 1 character in the span and put a corresponding character into
// the text so that the image doesn't run over any following text.
@ -121,15 +130,20 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
start, end, new BackgroundColorSpan(textShadowNode.mBackgroundColor)));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
if (!Float.isNaN(textShadowNode.mLetterSpacing)) {
float effectiveLetterSpacing = textAttributes.getEffectiveLetterSpacing();
if (!Float.isNaN(effectiveLetterSpacing)
&& (parentTextAttributes == null || parentTextAttributes.getEffectiveLetterSpacing() != effectiveLetterSpacing)) {
ops.add(new SetSpanOperation(
start,
end,
new CustomLetterSpacingSpan(textShadowNode.mLetterSpacing)));
new CustomLetterSpacingSpan(effectiveLetterSpacing)));
}
}
if (textShadowNode.mFontSize != UNSET) {
ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(textShadowNode.mFontSize)));
int effectiveFontSize = textAttributes.getEffectiveFontSize();
if (// `getEffectiveFontSize` always returns a value so don't need to check for anything like
// `Float.NaN`.
parentTextAttributes == null || parentTextAttributes.getEffectiveFontSize() != effectiveFontSize) {
ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(effectiveFontSize)));
}
if (textShadowNode.mFontStyle != UNSET
|| textShadowNode.mFontWeight != UNSET
@ -168,10 +182,12 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
textShadowNode.mTextShadowRadius,
textShadowNode.mTextShadowColor)));
}
if (!Float.isNaN(textShadowNode.getEffectiveLineHeight())) {
float effectiveLineHeight = textAttributes.getEffectiveLineHeight();
if (!Float.isNaN(effectiveLineHeight)
&& (parentTextAttributes == null || parentTextAttributes.getEffectiveLineHeight() != effectiveLineHeight)) {
ops.add(
new SetSpanOperation(
start, end, new CustomLineHeightSpan(textShadowNode.getEffectiveLineHeight())));
start, end, new CustomLineHeightSpan(effectiveLineHeight)));
}
if (textShadowNode.mTextTransform != TextTransform.UNSET) {
ops.add(
@ -184,11 +200,6 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
}
}
protected int getDefaultFontSize() {
return mAllowFontScaling ? (int) Math.ceil(PixelUtil.toPixelFromSP(ViewDefaults.FONT_SIZE_SP))
: (int) Math.ceil(PixelUtil.toPixelFromDIP(ViewDefaults.FONT_SIZE_SP));
}
protected static Spannable spannedFromShadowNode(
ReactBaseTextShadowNode textShadowNode, String text) {
SpannableStringBuilder sb = new SpannableStringBuilder();
@ -206,16 +217,10 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
sb.append(text);
}
buildSpannedFromShadowNode(textShadowNode, sb, ops, 0);
if (textShadowNode.mFontSize == UNSET) {
int defaultFontSize = textShadowNode.getDefaultFontSize();
ops.add(new SetSpanOperation(0, sb.length(), new AbsoluteSizeSpan(defaultFontSize)));
}
buildSpannedFromShadowNode(textShadowNode, sb, ops, null, 0);
textShadowNode.mContainsImages = false;
textShadowNode.mHeightOfTallestInlineImage = Float.NaN;
float heightOfTallestInlineImage = Float.NaN;
// While setting the Spans on the final text, we also check whether any of them are images.
int priority = 0;
@ -223,9 +228,9 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
if (op.what instanceof TextInlineImageSpan) {
int height = ((TextInlineImageSpan) op.what).getHeight();
textShadowNode.mContainsImages = true;
if (Float.isNaN(textShadowNode.mHeightOfTallestInlineImage)
|| height > textShadowNode.mHeightOfTallestInlineImage) {
textShadowNode.mHeightOfTallestInlineImage = height;
if (Float.isNaN(heightOfTallestInlineImage)
|| height > heightOfTallestInlineImage) {
heightOfTallestInlineImage = height;
}
}
@ -235,6 +240,8 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
priority++;
}
textShadowNode.mTextAttributes.setHeightOfTallestInlineImage(heightOfTallestInlineImage);
return sb;
}
@ -255,19 +262,14 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
: -1;
}
protected float mLineHeight = Float.NaN;
protected float mLetterSpacing = Float.NaN;
protected TextAttributes mTextAttributes;
protected boolean mIsColorSet = false;
protected boolean mAllowFontScaling = true;
protected int mColor;
protected boolean mIsBackgroundColorSet = false;
protected int mBackgroundColor;
protected int mNumberOfLines = UNSET;
protected int mFontSize = UNSET;
protected float mFontSizeInput = UNSET;
protected float mLineHeightInput = UNSET;
protected float mLetterSpacingInput = Float.NaN;
protected int mTextAlign = Gravity.NO_GRAVITY;
protected int mTextBreakStrategy =
(Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ? 0 : Layout.BREAK_STRATEGY_HIGH_QUALITY;
@ -311,16 +313,8 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
protected boolean mContainsImages = false;
protected float mHeightOfTallestInlineImage = Float.NaN;
public ReactBaseTextShadowNode() {}
// Returns a line height which takes into account the requested line height
// and the height of the inline images.
public float getEffectiveLineHeight() {
boolean useInlineViewHeight =
!Float.isNaN(mLineHeight)
&& !Float.isNaN(mHeightOfTallestInlineImage)
&& mHeightOfTallestInlineImage > mLineHeight;
return useInlineViewHeight ? mHeightOfTallestInlineImage : mLineHeight;
public ReactBaseTextShadowNode() {
mTextAttributes = new TextAttributes();
}
// Return text alignment according to LTR or RTL style
@ -342,36 +336,22 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
markUpdated();
}
@ReactProp(name = ViewProps.LINE_HEIGHT, defaultFloat = UNSET)
@ReactProp(name = ViewProps.LINE_HEIGHT, defaultFloat = Float.NaN)
public void setLineHeight(float lineHeight) {
mLineHeightInput = lineHeight;
if (lineHeight == UNSET) {
mLineHeight = Float.NaN;
} else {
mLineHeight =
mAllowFontScaling
? PixelUtil.toPixelFromSP(lineHeight)
: PixelUtil.toPixelFromDIP(lineHeight);
}
mTextAttributes.setLineHeight(lineHeight);
markUpdated();
}
@ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = Float.NaN)
public void setLetterSpacing(float letterSpacing) {
mLetterSpacingInput = letterSpacing;
mLetterSpacing = mAllowFontScaling
? PixelUtil.toPixelFromSP(mLetterSpacingInput)
: PixelUtil.toPixelFromDIP(mLetterSpacingInput);
mTextAttributes.setLetterSpacing(letterSpacing);
markUpdated();
}
@ReactProp(name = ViewProps.ALLOW_FONT_SCALING, defaultBoolean = true)
public void setAllowFontScaling(boolean allowFontScaling) {
if (allowFontScaling != mAllowFontScaling) {
mAllowFontScaling = allowFontScaling;
setFontSize(mFontSizeInput);
setLineHeight(mLineHeightInput);
setLetterSpacing(mLetterSpacingInput);
if (allowFontScaling != mTextAttributes.getAllowFontScaling()) {
mTextAttributes.setAllowFontScaling(allowFontScaling);
markUpdated();
}
}
@ -395,16 +375,9 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
markUpdated();
}
@ReactProp(name = ViewProps.FONT_SIZE, defaultFloat = UNSET)
@ReactProp(name = ViewProps.FONT_SIZE, defaultFloat = Float.NaN)
public void setFontSize(float fontSize) {
mFontSizeInput = fontSize;
if (fontSize != UNSET) {
fontSize =
mAllowFontScaling
? (float) Math.ceil(PixelUtil.toPixelFromSP(fontSize))
: (float) Math.ceil(PixelUtil.toPixelFromDIP(fontSize));
}
mFontSize = (int) fontSize;
mTextAttributes.setFontSize(fontSize);
markUpdated();
}

View File

@ -60,7 +60,7 @@ public class ReactTextShadowNode extends ReactBaseTextShadowNode {
// TODO(5578671): Handle text direction (see View#getTextDirectionHeuristic)
TextPaint textPaint = sTextPaintInstance;
textPaint.setTextSize(mFontSize != UNSET ? mFontSize : getDefaultFontSize());
textPaint.setTextSize(mTextAttributes.getEffectiveFontSize());
Layout layout;
Spanned text =
Assertions.assertNotNull(

View File

@ -0,0 +1,143 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
/*
* Currently, TextAttributes consists of a subset of text props that need to be passed from parent
* to child so inheritance can be implemented correctly. An example complexity that causes a prop
* to end up in TextAttributes is when multiple props need to be considered together to determine
* the rendered aka effective value. For example, to figure out the rendered/effective font size,
* you need to take into account the fontSize and allowFontScaling props.
*/
package com.facebook.react.views.text;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ViewDefaults;
public class TextAttributes {
private boolean mAllowFontScaling = true;
private float mFontSize = Float.NaN;
private float mLineHeight = Float.NaN;
private float mLetterSpacing = Float.NaN;
private float mHeightOfTallestInlineImage = Float.NaN;
public TextAttributes() {
}
public TextAttributes applyChild(TextAttributes child) {
TextAttributes result = new TextAttributes();
// allowFontScaling is always determined by the root Text
// component so don't allow the child to overwrite it.
result.mAllowFontScaling = mAllowFontScaling;
result.mFontSize = !Float.isNaN(child.mFontSize) ? child.mFontSize : mFontSize;
result.mLineHeight = !Float.isNaN(child.mLineHeight) ? child.mLineHeight : mLineHeight;
result.mLetterSpacing = !Float.isNaN(child.mLetterSpacing) ? child.mLetterSpacing : mLetterSpacing;
result.mHeightOfTallestInlineImage = !Float.isNaN(child.mHeightOfTallestInlineImage) ? child.mHeightOfTallestInlineImage : mHeightOfTallestInlineImage;
return result;
}
// Getters and setters
//
public boolean getAllowFontScaling() {
return mAllowFontScaling;
}
public void setAllowFontScaling(boolean value) {
mAllowFontScaling = value;
}
public float getFontSize() {
return mFontSize;
}
public void setFontSize(float value) {
mFontSize = value;
}
public float getLineHeight() {
return mLineHeight;
}
public void setLineHeight(float value) {
mLineHeight = value;
}
public float getLetterSpacing() {
return mLetterSpacing;
}
public void setLetterSpacing(float value) {
mLetterSpacing = value;
}
public float getHeightOfTallestInlineImage() {
return mHeightOfTallestInlineImage;
}
public void setHeightOfTallestInlineImage(float value) {
mHeightOfTallestInlineImage = value;
}
// Getters for effective values
//
// In general, these return `Float.NaN` if the property doesn't have a value.
//
// Always returns a value because uses a hardcoded default as a fallback.
public int getEffectiveFontSize() {
float fontSize = !Float.isNaN(mFontSize) ? mFontSize : ViewDefaults.FONT_SIZE_SP;
return mAllowFontScaling
? (int) Math.ceil(PixelUtil.toPixelFromSP(fontSize))
: (int) Math.ceil(PixelUtil.toPixelFromDIP(fontSize));
}
public float getEffectiveLineHeight() {
if (Float.isNaN(mLineHeight)) {
return Float.NaN;
}
float lineHeight = mAllowFontScaling
? PixelUtil.toPixelFromSP(mLineHeight)
: PixelUtil.toPixelFromDIP(mLineHeight);
// Take into account the requested line height
// and the height of the inline images.
boolean useInlineViewHeight =
!Float.isNaN(mHeightOfTallestInlineImage)
&& mHeightOfTallestInlineImage > lineHeight;
return useInlineViewHeight ? mHeightOfTallestInlineImage : lineHeight;
}
public float getEffectiveLetterSpacing() {
if (Float.isNaN(mLetterSpacing)) {
return Float.NaN;
}
return mAllowFontScaling
? PixelUtil.toPixelFromSP(mLetterSpacing)
: PixelUtil.toPixelFromDIP(mLetterSpacing);
}
public String toString() {
return (
"TextAttributes {"
+ "\n getAllowFontScaling(): " + getAllowFontScaling()
+ "\n getFontSize(): " + getFontSize()
+ "\n getEffectiveFontSize(): " + getEffectiveFontSize()
+ "\n getHeightOfTallestInlineImage(): " + getHeightOfTallestInlineImage()
+ "\n getLetterSpacing(): " + getLetterSpacing()
+ "\n getEffectiveLetterSpacing(): " + getEffectiveLetterSpacing()
+ "\n getLineHeight(): " + getLineHeight()
+ "\n getEffectiveLineHeight(): " + getEffectiveLineHeight()
+ "\n}"
);
}
}

View File

@ -23,6 +23,7 @@ import android.text.method.QwertyKeyListener;
import android.text.style.AbsoluteSizeSpan;
import android.text.style.BackgroundColorSpan;
import android.text.style.ForegroundColorSpan;
import android.util.TypedValue;
import android.view.Gravity;
import android.view.KeyEvent;
import android.view.MotionEvent;
@ -38,6 +39,7 @@ import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.views.text.CustomStyleSpan;
import com.facebook.react.views.text.ReactTagSpan;
import com.facebook.react.views.text.ReactTextUpdate;
import com.facebook.react.views.text.TextAttributes;
import com.facebook.react.views.text.TextInlineImageSpan;
import com.facebook.react.views.view.ReactViewBackgroundManager;
import java.util.ArrayList;
@ -82,7 +84,7 @@ public class ReactEditText extends EditText {
private final InternalKeyListener mKeyListener;
private boolean mDetectScrollMovement = false;
private boolean mOnKeyPress = false;
private float mLetterSpacingPt = 0;
private TextAttributes mTextAttributes;
private ReactViewBackgroundManager mReactBackgroundManager;
@ -109,6 +111,9 @@ public class ReactEditText extends EditText {
mStagedInputType = getInputType();
mKeyListener = new InternalKeyListener();
mScrollWatcher = null;
mTextAttributes = new TextAttributes();
applyTextAttributes();
}
// After the text changes inside an EditText, TextView checks if a layout() has been requested.
@ -635,25 +640,28 @@ public class ReactEditText extends EditText {
}
public void setLetterSpacingPt(float letterSpacingPt) {
mLetterSpacingPt = letterSpacingPt;
updateLetterSpacing();
mTextAttributes.setLetterSpacing(letterSpacingPt);
applyTextAttributes();
}
@Override
public void setTextSize (float size) {
super.setTextSize(size);
updateLetterSpacing();
public void setFontSize(float fontSize) {
mTextAttributes.setFontSize(fontSize);
applyTextAttributes();
}
@Override
public void setTextSize (int unit, float size) {
super.setTextSize(unit, size);
updateLetterSpacing();
}
protected void applyTextAttributes() {
// In general, the `getEffective*` functions return `Float.NaN` if the
// property hasn't been set.
// `getEffectiveFontSize` always returns a value so don't need to check for anything like
// `Float.NaN`.
setTextSize(TypedValue.COMPLEX_UNIT_PX, mTextAttributes.getEffectiveFontSize());
protected void updateLetterSpacing() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
setLetterSpacing(PixelUtil.toPixelFromSP(mLetterSpacingPt) / getTextSize());
float effectiveLetterSpacing = mTextAttributes.getEffectiveLetterSpacing();
if (!Float.isNaN(effectiveLetterSpacing)) {
setLetterSpacing(effectiveLetterSpacing / getTextSize());
}
}
}

View File

@ -101,9 +101,6 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
int inputType = editText.getInputType();
editText.setInputType(inputType & (~InputType.TYPE_TEXT_FLAG_MULTI_LINE));
editText.setReturnKeyType("done");
editText.setTextSize(
TypedValue.COMPLEX_UNIT_PX,
(int) Math.ceil(PixelUtil.toPixelFromSP(ViewDefaults.FONT_SIZE_SP)));
return editText;
}
@ -204,9 +201,7 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
@ReactProp(name = ViewProps.FONT_SIZE, defaultFloat = ViewDefaults.FONT_SIZE_SP)
public void setFontSize(ReactEditText view, float fontSize) {
view.setTextSize(
TypedValue.COMPLEX_UNIT_PX,
(int) Math.ceil(PixelUtil.toPixelFromSP(fontSize)));
view.setFontSize(fontSize);
}
@ReactProp(name = ViewProps.FONT_FAMILY)

View File

@ -101,10 +101,7 @@ public class ReactTextInputShadowNode extends ReactBaseTextShadowNode
if (mLocalData != null) {
mLocalData.apply(editText);
} else {
editText.setTextSize(
TypedValue.COMPLEX_UNIT_PX,
mFontSize == UNSET ?
(int) Math.ceil(PixelUtil.toPixelFromSP(ViewDefaults.FONT_SIZE_SP)) : mFontSize);
editText.setTextSize(TypedValue.COMPLEX_UNIT_PX, mTextAttributes.getEffectiveFontSize());
if (mNumberOfLines != UNSET) {
editText.setLines(mNumberOfLines);