Skip to content

Commit 8b9f790

Browse files
janicduplessisfacebook-github-bot
authored andcommitted
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: #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
1 parent 0cafa0f commit 8b9f790

File tree

7 files changed

+181
-134
lines changed

7 files changed

+181
-134
lines changed

RNTester/js/examples/TextInput/TextInputExample.android.js

+7
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,13 @@ exports.examples = [
611611
]}
612612
placeholder="Sans-Serif bold"
613613
/>
614+
<TextInput
615+
style={[
616+
styles.singleLine,
617+
{fontFamily: 'sans-serif', fontWeight: '500'},
618+
]}
619+
placeholder="Sans-Serif 500"
620+
/>
614621
<TextInput
615622
style={[
616623
styles.singleLine,

RNTester/js/examples/TextInput/TextInputExample.ios.js

+38
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,44 @@ exports.examples = [
11221122
);
11231123
},
11241124
},
1125+
{
1126+
title: 'fontFamily, fontWeight and fontStyle',
1127+
render: function(): React.Node {
1128+
return (
1129+
<View>
1130+
<TextInput
1131+
style={[styles.default, {fontFamily: 'sans-serif'}]}
1132+
placeholder="Custom fonts like Sans-Serif are supported"
1133+
/>
1134+
<TextInput
1135+
style={[
1136+
styles.default,
1137+
{fontFamily: 'sans-serif', fontWeight: 'bold'},
1138+
]}
1139+
placeholder="Sans-Serif bold"
1140+
/>
1141+
<TextInput
1142+
style={[
1143+
styles.default,
1144+
{fontFamily: 'sans-serif', fontWeight: '500'},
1145+
]}
1146+
placeholder="Sans-Serif 500"
1147+
/>
1148+
<TextInput
1149+
style={[
1150+
styles.default,
1151+
{fontFamily: 'sans-serif', fontStyle: 'italic'},
1152+
]}
1153+
placeholder="Sans-Serif italic"
1154+
/>
1155+
<TextInput
1156+
style={[styles.default, {fontFamily: 'serif'}]}
1157+
placeholder="Serif"
1158+
/>
1159+
</View>
1160+
);
1161+
},
1162+
},
11251163
{
11261164
title: 'TextInput Placeholder Styles',
11271165
render: function(): React.Node {

ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java

+3-31
Original file line numberDiff line numberDiff line change
@@ -71,37 +71,9 @@ public int getWeight() {
7171

7272
private static void apply(
7373
Paint paint, int style, int weight, @Nullable String family, AssetManager assetManager) {
74-
int oldStyle;
75-
Typeface typeface = paint.getTypeface();
76-
if (typeface == null) {
77-
oldStyle = 0;
78-
} else {
79-
oldStyle = typeface.getStyle();
80-
}
81-
82-
int want = 0;
83-
if ((weight == Typeface.BOLD)
84-
|| ((oldStyle & Typeface.BOLD) != 0 && weight == ReactTextShadowNode.UNSET)) {
85-
want |= Typeface.BOLD;
86-
}
87-
88-
if ((style == Typeface.ITALIC)
89-
|| ((oldStyle & Typeface.ITALIC) != 0 && style == ReactTextShadowNode.UNSET)) {
90-
want |= Typeface.ITALIC;
91-
}
92-
93-
if (family != null) {
94-
typeface = ReactFontManager.getInstance().getTypeface(family, want, weight, assetManager);
95-
} else if (typeface != null) {
96-
// TODO(t9055065): Fix custom fonts getting applied to text children with different style
97-
typeface = Typeface.create(typeface, want);
98-
}
99-
100-
if (typeface != null) {
101-
paint.setTypeface(typeface);
102-
} else {
103-
paint.setTypeface(Typeface.defaultFromStyle(want));
104-
}
74+
Typeface typeface = ReactTypefaceUtils.applyStyles(
75+
paint.getTypeface(), style, weight, family, assetManager);
76+
paint.setTypeface(typeface);
10577
paint.setSubpixelText(true);
10678
}
10779
}

ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java

+2-38
Original file line numberDiff line numberDiff line change
@@ -300,23 +300,6 @@ protected Spannable spannedFromShadowNode(
300300
return sb;
301301
}
302302

303-
/**
304-
* Return -1 if the input string is not a valid numeric fontWeight (100, 200, ..., 900), otherwise
305-
* return the weight.
306-
*
307-
* <p>This code is duplicated in ReactTextInputManager TODO: Factor into a common place they can
308-
* both use
309-
*/
310-
private static int parseNumericFontWeight(String fontWeightString) {
311-
// This should be much faster than using regex to verify input and Integer.parseInt
312-
return fontWeightString.length() == 3
313-
&& fontWeightString.endsWith("00")
314-
&& fontWeightString.charAt(0) <= '9'
315-
&& fontWeightString.charAt(0) >= '1'
316-
? 100 * (fontWeightString.charAt(0) - '0')
317-
: UNSET;
318-
}
319-
320303
protected TextAttributes mTextAttributes;
321304

322305
protected boolean mIsColorSet = false;
@@ -490,37 +473,18 @@ public void setFontFamily(@Nullable String fontFamily) {
490473
markUpdated();
491474
}
492475

493-
/**
494-
* /* This code is duplicated in ReactTextInputManager /* TODO: Factor into a common place they
495-
* can both use
496-
*/
497476
@ReactProp(name = ViewProps.FONT_WEIGHT)
498477
public void setFontWeight(@Nullable String fontWeightString) {
499-
int fontWeightNumeric =
500-
fontWeightString != null ? parseNumericFontWeight(fontWeightString) : UNSET;
501-
int fontWeight = fontWeightNumeric != UNSET ? fontWeightNumeric : Typeface.NORMAL;
502-
503-
if (fontWeight == 700 || "bold".equals(fontWeightString)) fontWeight = Typeface.BOLD;
504-
else if (fontWeight == 400 || "normal".equals(fontWeightString)) fontWeight = Typeface.NORMAL;
505-
478+
int fontWeight = ReactTypefaceUtils.parseFontWeight(fontWeightString);
506479
if (fontWeight != mFontWeight) {
507480
mFontWeight = fontWeight;
508481
markUpdated();
509482
}
510483
}
511484

512-
/**
513-
* /* This code is duplicated in ReactTextInputManager /* TODO: Factor into a common place they
514-
* can both use
515-
*/
516485
@ReactProp(name = ViewProps.FONT_STYLE)
517486
public void setFontStyle(@Nullable String fontStyleString) {
518-
int fontStyle = UNSET;
519-
if ("italic".equals(fontStyleString)) {
520-
fontStyle = Typeface.ITALIC;
521-
} else if ("normal".equals(fontStyleString)) {
522-
fontStyle = Typeface.NORMAL;
523-
}
487+
int fontStyle = ReactTypefaceUtils.parseFontStyle(fontStyleString);
524488
if (fontStyle != mFontStyle) {
525489
mFontStyle = fontStyle;
526490
markUpdated();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* <p>This source code is licensed under the MIT license found in the LICENSE file in the root
5+
* directory of this source tree.
6+
*/
7+
package com.facebook.react.views.text;
8+
9+
import android.content.res.AssetManager;
10+
import android.graphics.Paint;
11+
import android.graphics.Typeface;
12+
13+
import androidx.annotation.Nullable;
14+
15+
public class ReactTypefaceUtils {
16+
public static final int UNSET = -1;
17+
18+
public static int parseFontWeight(@Nullable String fontWeightString) {
19+
int fontWeightNumeric =
20+
fontWeightString != null ? parseNumericFontWeight(fontWeightString) : UNSET;
21+
int fontWeight = fontWeightNumeric != UNSET ? fontWeightNumeric : Typeface.NORMAL;
22+
23+
if (fontWeight == 700 || "bold".equals(fontWeightString)) fontWeight = Typeface.BOLD;
24+
else if (fontWeight == 400 || "normal".equals(fontWeightString)) fontWeight = Typeface.NORMAL;
25+
26+
return fontWeight;
27+
}
28+
29+
public static int parseFontStyle(@Nullable String fontStyleString) {
30+
int fontStyle = UNSET;
31+
if ("italic".equals(fontStyleString)) {
32+
fontStyle = Typeface.ITALIC;
33+
} else if ("normal".equals(fontStyleString)) {
34+
fontStyle = Typeface.NORMAL;
35+
}
36+
37+
return fontStyle;
38+
}
39+
40+
public static Typeface applyStyles(@Nullable Typeface typeface,
41+
int style, int weight, @Nullable String family, AssetManager assetManager) {
42+
int oldStyle;
43+
if (typeface == null) {
44+
oldStyle = 0;
45+
} else {
46+
oldStyle = typeface.getStyle();
47+
}
48+
49+
int want = 0;
50+
if ((weight == Typeface.BOLD)
51+
|| ((oldStyle & Typeface.BOLD) != 0 && weight == ReactTextShadowNode.UNSET)) {
52+
want |= Typeface.BOLD;
53+
}
54+
55+
if ((style == Typeface.ITALIC)
56+
|| ((oldStyle & Typeface.ITALIC) != 0 && style == ReactTextShadowNode.UNSET)) {
57+
want |= Typeface.ITALIC;
58+
}
59+
60+
if (family != null) {
61+
typeface = ReactFontManager.getInstance().getTypeface(family, want, weight, assetManager);
62+
} else if (typeface != null) {
63+
// TODO(t9055065): Fix custom fonts getting applied to text children with different style
64+
typeface = Typeface.create(typeface, want);
65+
}
66+
67+
if (typeface != null) {
68+
return typeface;
69+
} else {
70+
return Typeface.defaultFromStyle(want);
71+
}
72+
}
73+
74+
/**
75+
* Return -1 if the input string is not a valid numeric fontWeight (100, 200, ..., 900), otherwise
76+
* return the weight.
77+
*/
78+
private static int parseNumericFontWeight(String fontWeightString) {
79+
// This should be much faster than using regex to verify input and Integer.parseInt
80+
return fontWeightString.length() == 3
81+
&& fontWeightString.endsWith("00")
82+
&& fontWeightString.charAt(0) <= '9'
83+
&& fontWeightString.charAt(0) >= '1'
84+
? 100 * (fontWeightString.charAt(0) - '0')
85+
: UNSET;
86+
}
87+
}

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

+38
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.facebook.react.uimanager.UIManagerModule;
3939
import com.facebook.react.views.text.ReactSpan;
4040
import com.facebook.react.views.text.ReactTextUpdate;
41+
import com.facebook.react.views.text.ReactTypefaceUtils;
4142
import com.facebook.react.views.text.TextAttributes;
4243
import com.facebook.react.views.text.TextInlineImageSpan;
4344
import com.facebook.react.views.view.ReactViewBackgroundManager;
@@ -83,6 +84,10 @@ public class ReactEditText extends EditText {
8384
private boolean mDetectScrollMovement = false;
8485
private boolean mOnKeyPress = false;
8586
private TextAttributes mTextAttributes;
87+
private boolean mTypefaceDirty = false;
88+
private @Nullable String mFontFamily = null;
89+
private int mFontWeight = ReactTypefaceUtils.UNSET;
90+
private int mFontStyle = ReactTypefaceUtils.UNSET;
8691

8792
private ReactViewBackgroundManager mReactBackgroundManager;
8893

@@ -382,6 +387,39 @@ public void setInputType(int type) {
382387
setKeyListener(mKeyListener);
383388
}
384389

390+
public void setFontFamily(String fontFamily) {
391+
mFontFamily = fontFamily;
392+
mTypefaceDirty = true;
393+
}
394+
395+
public void setFontWeight(String fontWeightString) {
396+
int fontWeight = ReactTypefaceUtils.parseFontWeight(fontWeightString);
397+
if (fontWeight != mFontWeight) {
398+
mFontWeight = fontWeight;
399+
mTypefaceDirty = true;
400+
}
401+
}
402+
403+
public void setFontStyle(String fontStyleString) {
404+
int fontStyle = ReactTypefaceUtils.parseFontStyle(fontStyleString);
405+
if (fontStyle != mFontStyle) {
406+
mFontStyle = fontStyle;
407+
mTypefaceDirty = true;
408+
}
409+
}
410+
411+
public void maybeUpdateTypeface() {
412+
if (!mTypefaceDirty) {
413+
return;
414+
}
415+
416+
mTypefaceDirty = false;
417+
418+
Typeface newTypeface = ReactTypefaceUtils.applyStyles(
419+
getTypeface(), mFontStyle, mFontWeight, mFontFamily, getContext().getAssets());
420+
setTypeface(newTypeface);
421+
}
422+
385423
// VisibleForTesting from {@link TextInputEventsTestCase}.
386424
public void requestFocusFromJS() {
387425
mShouldAllowFocus = true;

0 commit comments

Comments
 (0)