From 0db0d263ac81b91899b5b8fb36afd2e6b466c701 Mon Sep 17 00:00:00 2001 From: Adam Comella Date: Wed, 16 Jan 2019 05:30:50 -0800 Subject: [PATCH] Android Text: Fix `letterSpacing` rendering when `fontSize` changes (#22993) Summary: If you set `letterSpacing` on an outer text and then set `fontSize` on an inner text, the wrong `letterSpacing` will be rendered. Here's an example: ``` Some text ``` `fontSize` affects letter spacing. In this case, the bug is that setting `fontSize` to `20` doesn't cause the letter spacing to be recalculated. Notice that the logic for applying letter spacing only applies it if the node has a different value for `getEffectiveLetterSpacing()` than its parent. The problem is that `getEffectiveLetterSpacing()` doesn't represent the final letter spacing value -- it doesn't take `fontSize` into account. Consequently, it sometimes incorrectly skips applying letter spacing as illustrated above. The fix is to ensure `getEffectiveLetterSpacing()` represents the final rendered letter spacing value. To do this, some of the letter spacing calculation code was moved from `CustomLetterSpacingSpan` to `getEffectiveLetterSpacing()`. Pull Request resolved: https://github.com/facebook/react-native/pull/22993 Differential Revision: D13671505 Pulled By: cpojer fbshipit-source-id: 400f36d3fd71ab7cb6cba8508baa71f2973f8f0e --- .../facebook/react/views/text/CustomLetterSpacingSpan.java | 4 +--- .../java/com/facebook/react/views/text/TextAttributes.java | 6 +++++- .../com/facebook/react/views/textinput/ReactEditText.java | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java index 699e406bf19..cbe2c2e49b5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java @@ -42,10 +42,8 @@ public class CustomLetterSpacingSpan extends MetricAffectingSpan { } private void apply(TextPaint paint) { - // mLetterSpacing and paint.getTextSize() are both in pixels, - // yielding an accurate em value. if (!Float.isNaN(mLetterSpacing)) { - paint.setLetterSpacing(mLetterSpacing / paint.getTextSize()); + paint.setLetterSpacing(mLetterSpacing); } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributes.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributes.java index 4778d454093..d0fdbc76769 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributes.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributes.java @@ -121,9 +121,13 @@ public class TextAttributes { return Float.NaN; } - return mAllowFontScaling + float letterSpacingPixels = mAllowFontScaling ? PixelUtil.toPixelFromSP(mLetterSpacing) : PixelUtil.toPixelFromDIP(mLetterSpacing); + + // `letterSpacingPixels` and `getEffectiveFontSize` are both in pixels, + // yielding an accurate em value. + return letterSpacingPixels / getEffectiveFontSize(); } public String toString() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 1e20f0616b9..df3476801fb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -667,7 +667,7 @@ public class ReactEditText extends EditText { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { float effectiveLetterSpacing = mTextAttributes.getEffectiveLetterSpacing(); if (!Float.isNaN(effectiveLetterSpacing)) { - setLetterSpacing(effectiveLetterSpacing / getTextSize()); + setLetterSpacing(effectiveLetterSpacing); } } }