Skip to content

Commit

Permalink
Fix medium font weights for TextInput on Android (#26434)
Browse files Browse the repository at this point in the history
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: #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 (
    <TextInput
      style={[
        styles.singleLine,
        {fontFamily: 'sans-serif', fontWeight: '500', fontSize: 32},
      ]}
      placeholder="Sans-Serif 500"
      value={value}
      onChangeText={setValue}
    />
  );
}
```

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
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Sep 23, 2019
1 parent 0cafa0f commit 8b9f790
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 134 deletions.
7 changes: 7 additions & 0 deletions RNTester/js/examples/TextInput/TextInputExample.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,13 @@ exports.examples = [
]}
placeholder="Sans-Serif bold"
/>
<TextInput
style={[
styles.singleLine,
{fontFamily: 'sans-serif', fontWeight: '500'},
]}
placeholder="Sans-Serif 500"
/>
<TextInput
style={[
styles.singleLine,
Expand Down
38 changes: 38 additions & 0 deletions RNTester/js/examples/TextInput/TextInputExample.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,44 @@ exports.examples = [
);
},
},
{
title: 'fontFamily, fontWeight and fontStyle',
render: function(): React.Node {
return (
<View>
<TextInput
style={[styles.default, {fontFamily: 'sans-serif'}]}
placeholder="Custom fonts like Sans-Serif are supported"
/>
<TextInput
style={[
styles.default,
{fontFamily: 'sans-serif', fontWeight: 'bold'},
]}
placeholder="Sans-Serif bold"
/>
<TextInput
style={[
styles.default,
{fontFamily: 'sans-serif', fontWeight: '500'},
]}
placeholder="Sans-Serif 500"
/>
<TextInput
style={[
styles.default,
{fontFamily: 'sans-serif', fontStyle: 'italic'},
]}
placeholder="Sans-Serif italic"
/>
<TextInput
style={[styles.default, {fontFamily: 'serif'}]}
placeholder="Serif"
/>
</View>
);
},
},
{
title: 'TextInput Placeholder Styles',
render: function(): React.Node {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* <p>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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 8b9f790

Please sign in to comment.