Files
react-native/ReactAndroid
Adam Comella 1bdb250906 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
2019-01-14 16:28:19 -08:00
..
2019-01-14 10:34:30 -08:00
2018-05-27 15:17:55 -07:00

Building React Native for Android

See the docs on the website.

Running tests

When you submit a pull request CircleCI will automatically run all tests. To run tests locally, see Testing.