From 8b9f7900697b2e4bb72b37ed2e6c3d113185d327 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 23 Sep 2019 10:21:36 -0700 Subject: [PATCH] Fix medium font weights for TextInput on Android (#26434) Summary: When using a medium (500) font weight on Android the wrong weight is used for the placeholder and for the first few seconds of input (before it gets text back from JS). To fix it I refactored the way we handle text styles (family, weight, style) to create a typeface to be more like the `Text` component. Since all these 3 props are linked and used to create the typeface object it makes more sense to do it at the end of setting props instead of in each prop handler and trying to recreate the object without losing styles set by other prop handlers. Do do that we now store fontFamily, fontStyle and fontWeight as ivar of the ReactEditText class. At the end of updating prop if any of those changed we recreate the typeface object. This doesn't actually fix the bug but was a first step towards it. There were a bunch of TODOs in the code to remove duplication between `Text` and `TextInput` for parsing and creating the typeface object. To do that I simply moved the code to util functions in a static class. Once the duplication was removed the bug was fixed! I assume proper support for medium font weights was added for `Text` but not in the duplicated code for `TextInput`. ## Changelog [Android] [Fixed] - Fix medium font weights for TextInput on Android Pull Request resolved: https://github.com/facebook/react-native/pull/26434 Test Plan: Tested in my app and in RNTester that custom styles for both text and textinput all seem to work. Repro in RNTester: ```js function Bug() { const [value, setValue] = React.useState(''); return ( ); } ``` Before: ![font-bug-1](https://user-images.githubusercontent.com/2677334/64902280-6889fc00-d672-11e9-8f44-9e524d844a6c.gif) After: ![font-bug-2](https://user-images.githubusercontent.com/2677334/64902282-6cb61980-d672-11e9-8163-ace0f23070b6.gif) Reviewed By: mmmulani Differential Revision: D17468825 Pulled By: JoshuaGross fbshipit-source-id: bc2219facb94668551a06a68b0ee4690e5474d40 --- .../TextInput/TextInputExample.android.js | 7 ++ .../TextInput/TextInputExample.ios.js | 38 ++++++++ .../react/views/text/CustomStyleSpan.java | 34 +------- .../views/text/ReactBaseTextShadowNode.java | 40 +-------- .../react/views/text/ReactTypefaceUtils.java | 87 +++++++++++++++++++ .../react/views/textinput/ReactEditText.java | 38 ++++++++ .../textinput/ReactTextInputManager.java | 71 ++------------- 7 files changed, 181 insertions(+), 134 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.java diff --git a/RNTester/js/examples/TextInput/TextInputExample.android.js b/RNTester/js/examples/TextInput/TextInputExample.android.js index 40a56386e1eb49..aec7240a9dcf79 100644 --- a/RNTester/js/examples/TextInput/TextInputExample.android.js +++ b/RNTester/js/examples/TextInput/TextInputExample.android.js @@ -611,6 +611,13 @@ exports.examples = [ ]} placeholder="Sans-Serif bold" /> + + + + + + + + ); + }, + }, { title: 'TextInput Placeholder Styles', render: function(): React.Node { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java index 11a95a9834ba15..a8fdc73a64844b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java @@ -71,37 +71,9 @@ public int getWeight() { private static void apply( Paint paint, int style, int weight, @Nullable String family, AssetManager assetManager) { - int oldStyle; - Typeface typeface = paint.getTypeface(); - if (typeface == null) { - oldStyle = 0; - } else { - oldStyle = typeface.getStyle(); - } - - int want = 0; - if ((weight == Typeface.BOLD) - || ((oldStyle & Typeface.BOLD) != 0 && weight == ReactTextShadowNode.UNSET)) { - want |= Typeface.BOLD; - } - - if ((style == Typeface.ITALIC) - || ((oldStyle & Typeface.ITALIC) != 0 && style == ReactTextShadowNode.UNSET)) { - want |= Typeface.ITALIC; - } - - if (family != null) { - typeface = ReactFontManager.getInstance().getTypeface(family, want, weight, assetManager); - } else if (typeface != null) { - // TODO(t9055065): Fix custom fonts getting applied to text children with different style - typeface = Typeface.create(typeface, want); - } - - if (typeface != null) { - paint.setTypeface(typeface); - } else { - paint.setTypeface(Typeface.defaultFromStyle(want)); - } + Typeface typeface = ReactTypefaceUtils.applyStyles( + paint.getTypeface(), style, weight, family, assetManager); + paint.setTypeface(typeface); paint.setSubpixelText(true); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java index 361426cdd19870..30417df9a46faf 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java @@ -300,23 +300,6 @@ protected Spannable spannedFromShadowNode( return sb; } - /** - * Return -1 if the input string is not a valid numeric fontWeight (100, 200, ..., 900), otherwise - * return the weight. - * - *

This code is duplicated in ReactTextInputManager TODO: Factor into a common place they can - * both use - */ - private static int parseNumericFontWeight(String fontWeightString) { - // This should be much faster than using regex to verify input and Integer.parseInt - return fontWeightString.length() == 3 - && fontWeightString.endsWith("00") - && fontWeightString.charAt(0) <= '9' - && fontWeightString.charAt(0) >= '1' - ? 100 * (fontWeightString.charAt(0) - '0') - : UNSET; - } - protected TextAttributes mTextAttributes; protected boolean mIsColorSet = false; @@ -490,37 +473,18 @@ public void setFontFamily(@Nullable String fontFamily) { markUpdated(); } - /** - * /* This code is duplicated in ReactTextInputManager /* TODO: Factor into a common place they - * can both use - */ @ReactProp(name = ViewProps.FONT_WEIGHT) public void setFontWeight(@Nullable String fontWeightString) { - int fontWeightNumeric = - fontWeightString != null ? parseNumericFontWeight(fontWeightString) : UNSET; - int fontWeight = fontWeightNumeric != UNSET ? fontWeightNumeric : Typeface.NORMAL; - - if (fontWeight == 700 || "bold".equals(fontWeightString)) fontWeight = Typeface.BOLD; - else if (fontWeight == 400 || "normal".equals(fontWeightString)) fontWeight = Typeface.NORMAL; - + int fontWeight = ReactTypefaceUtils.parseFontWeight(fontWeightString); if (fontWeight != mFontWeight) { mFontWeight = fontWeight; markUpdated(); } } - /** - * /* This code is duplicated in ReactTextInputManager /* TODO: Factor into a common place they - * can both use - */ @ReactProp(name = ViewProps.FONT_STYLE) public void setFontStyle(@Nullable String fontStyleString) { - int fontStyle = UNSET; - if ("italic".equals(fontStyleString)) { - fontStyle = Typeface.ITALIC; - } else if ("normal".equals(fontStyleString)) { - fontStyle = Typeface.NORMAL; - } + int fontStyle = ReactTypefaceUtils.parseFontStyle(fontStyleString); if (fontStyle != mFontStyle) { mFontStyle = fontStyle; markUpdated(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.java new file mode 100644 index 00000000000000..af1696bb06c1bf --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.java @@ -0,0 +1,87 @@ +/** + * 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. + */ +package com.facebook.react.views.text; + +import android.content.res.AssetManager; +import android.graphics.Paint; +import android.graphics.Typeface; + +import androidx.annotation.Nullable; + +public class ReactTypefaceUtils { + public static final int UNSET = -1; + + public static int parseFontWeight(@Nullable String fontWeightString) { + int fontWeightNumeric = + fontWeightString != null ? parseNumericFontWeight(fontWeightString) : UNSET; + int fontWeight = fontWeightNumeric != UNSET ? fontWeightNumeric : Typeface.NORMAL; + + if (fontWeight == 700 || "bold".equals(fontWeightString)) fontWeight = Typeface.BOLD; + else if (fontWeight == 400 || "normal".equals(fontWeightString)) fontWeight = Typeface.NORMAL; + + return fontWeight; + } + + public static int parseFontStyle(@Nullable String fontStyleString) { + int fontStyle = UNSET; + if ("italic".equals(fontStyleString)) { + fontStyle = Typeface.ITALIC; + } else if ("normal".equals(fontStyleString)) { + fontStyle = Typeface.NORMAL; + } + + return fontStyle; + } + + public static Typeface applyStyles(@Nullable Typeface typeface, + int style, int weight, @Nullable String family, AssetManager assetManager) { + int oldStyle; + if (typeface == null) { + oldStyle = 0; + } else { + oldStyle = typeface.getStyle(); + } + + int want = 0; + if ((weight == Typeface.BOLD) + || ((oldStyle & Typeface.BOLD) != 0 && weight == ReactTextShadowNode.UNSET)) { + want |= Typeface.BOLD; + } + + if ((style == Typeface.ITALIC) + || ((oldStyle & Typeface.ITALIC) != 0 && style == ReactTextShadowNode.UNSET)) { + want |= Typeface.ITALIC; + } + + if (family != null) { + typeface = ReactFontManager.getInstance().getTypeface(family, want, weight, assetManager); + } else if (typeface != null) { + // TODO(t9055065): Fix custom fonts getting applied to text children with different style + typeface = Typeface.create(typeface, want); + } + + if (typeface != null) { + return typeface; + } else { + return Typeface.defaultFromStyle(want); + } + } + + /** + * Return -1 if the input string is not a valid numeric fontWeight (100, 200, ..., 900), otherwise + * return the weight. + */ + private static int parseNumericFontWeight(String fontWeightString) { + // This should be much faster than using regex to verify input and Integer.parseInt + return fontWeightString.length() == 3 + && fontWeightString.endsWith("00") + && fontWeightString.charAt(0) <= '9' + && fontWeightString.charAt(0) >= '1' + ? 100 * (fontWeightString.charAt(0) - '0') + : UNSET; + } +} 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 46487c4813412b..02f4a0a061b351 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 @@ -38,6 +38,7 @@ import com.facebook.react.uimanager.UIManagerModule; import com.facebook.react.views.text.ReactSpan; import com.facebook.react.views.text.ReactTextUpdate; +import com.facebook.react.views.text.ReactTypefaceUtils; import com.facebook.react.views.text.TextAttributes; import com.facebook.react.views.text.TextInlineImageSpan; import com.facebook.react.views.view.ReactViewBackgroundManager; @@ -83,6 +84,10 @@ public class ReactEditText extends EditText { private boolean mDetectScrollMovement = false; private boolean mOnKeyPress = false; private TextAttributes mTextAttributes; + private boolean mTypefaceDirty = false; + private @Nullable String mFontFamily = null; + private int mFontWeight = ReactTypefaceUtils.UNSET; + private int mFontStyle = ReactTypefaceUtils.UNSET; private ReactViewBackgroundManager mReactBackgroundManager; @@ -382,6 +387,39 @@ public void setInputType(int type) { setKeyListener(mKeyListener); } + public void setFontFamily(String fontFamily) { + mFontFamily = fontFamily; + mTypefaceDirty = true; + } + + public void setFontWeight(String fontWeightString) { + int fontWeight = ReactTypefaceUtils.parseFontWeight(fontWeightString); + if (fontWeight != mFontWeight) { + mFontWeight = fontWeight; + mTypefaceDirty = true; + } + } + + public void setFontStyle(String fontStyleString) { + int fontStyle = ReactTypefaceUtils.parseFontStyle(fontStyleString); + if (fontStyle != mFontStyle) { + mFontStyle = fontStyle; + mTypefaceDirty = true; + } + } + + public void maybeUpdateTypeface() { + if (!mTypefaceDirty) { + return; + } + + mTypefaceDirty = false; + + Typeface newTypeface = ReactTypefaceUtils.applyStyles( + getTypeface(), mFontStyle, mFontWeight, mFontFamily, getContext().getAssets()); + setTypeface(newTypeface); + } + // VisibleForTesting from {@link TextInputEventsTestCase}. public void requestFocusFromJS() { mShouldAllowFocus = true; diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index 8c9e3209583448..5dfc945ca919ab 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -230,14 +230,7 @@ public void setFontSize(ReactEditText view, float fontSize) { @ReactProp(name = ViewProps.FONT_FAMILY) public void setFontFamily(ReactEditText view, String fontFamily) { - int style = Typeface.NORMAL; - if (view.getTypeface() != null) { - style = view.getTypeface().getStyle(); - } - Typeface newTypeface = - ReactFontManager.getInstance() - .getTypeface(fontFamily, style, view.getContext().getAssets()); - view.setTypeface(newTypeface); + view.setFontFamily(fontFamily); } @ReactProp(name = ViewProps.MAX_FONT_SIZE_MULTIPLIER, defaultFloat = Float.NaN) @@ -245,50 +238,14 @@ public void setMaxFontSizeMultiplier(ReactEditText view, float maxFontSizeMultip view.setMaxFontSizeMultiplier(maxFontSizeMultiplier); } - /** - * /* This code was taken from the method setFontWeight of the class ReactTextShadowNode /* TODO: - * Factor into a common place they can both use - */ @ReactProp(name = ViewProps.FONT_WEIGHT) - public void setFontWeight(ReactEditText view, @Nullable String fontWeightString) { - int fontWeightNumeric = - fontWeightString != null ? parseNumericFontWeight(fontWeightString) : -1; - int fontWeight = UNSET; - if (fontWeightNumeric >= 500 || "bold".equals(fontWeightString)) { - fontWeight = Typeface.BOLD; - } else if ("normal".equals(fontWeightString) - || (fontWeightNumeric != -1 && fontWeightNumeric < 500)) { - fontWeight = Typeface.NORMAL; - } - Typeface currentTypeface = view.getTypeface(); - if (currentTypeface == null) { - currentTypeface = Typeface.DEFAULT; - } - if (fontWeight != currentTypeface.getStyle()) { - view.setTypeface(currentTypeface, fontWeight); - } + public void setFontWeight(ReactEditText view, @Nullable String fontWeight) { + view.setFontWeight(fontWeight); } - /** - * /* This code was taken from the method setFontStyle of the class ReactTextShadowNode /* TODO: - * Factor into a common place they can both use - */ @ReactProp(name = ViewProps.FONT_STYLE) - public void setFontStyle(ReactEditText view, @Nullable String fontStyleString) { - int fontStyle = UNSET; - if ("italic".equals(fontStyleString)) { - fontStyle = Typeface.ITALIC; - } else if ("normal".equals(fontStyleString)) { - fontStyle = Typeface.NORMAL; - } - - Typeface currentTypeface = view.getTypeface(); - if (currentTypeface == null) { - currentTypeface = Typeface.DEFAULT; - } - if (fontStyle != currentTypeface.getStyle()) { - view.setTypeface(currentTypeface, fontStyle); - } + public void setFontStyle(ReactEditText view, @Nullable String fontStyle) { + view.setFontStyle(fontStyle); } @ReactProp(name = "importantForAutofill") @@ -800,6 +757,7 @@ public void setBorderColor(ReactEditText view, int index, Integer color) { @Override protected void onAfterUpdateTransaction(ReactEditText view) { super.onAfterUpdateTransaction(view); + view.maybeUpdateTypeface(); view.commitStagedInputType(); } @@ -813,23 +771,6 @@ private static void checkPasswordType(ReactEditText view) { } } - /** - * This code was taken from the method parseNumericFontWeight of the class ReactTextShadowNode - * TODO: Factor into a common place they can both use - * - *

Return -1 if the input string is not a valid numeric fontWeight (100, 200, ..., 900), - * otherwise return the weight. - */ - private static int parseNumericFontWeight(String fontWeightString) { - // This should be much faster than using regex to verify input and Integer.parseInt - return fontWeightString.length() == 3 - && fontWeightString.endsWith("00") - && fontWeightString.charAt(0) <= '9' - && fontWeightString.charAt(0) >= '1' - ? 100 * (fontWeightString.charAt(0) - '0') - : -1; - } - private static void updateStagedInputTypeFlag( ReactEditText view, int flagsToUnset, int flagsToSet) { view.setStagedInputType((view.getStagedInputType() & ~flagsToUnset) | flagsToSet);