Add Android support for fontVariant prop #27006

mcuelenaere · 2019-10-25T11:12:41Z

Summary

Android was missing support for the fontVariant prop in TextViews, this PR adds that.

Changelog

[Android] [Added] - Add Android support for fontVariant prop

Test Plan

Since I can't get RNTester to work locally (it crashes when loading libyoga.so on No implementation found for long com.facebook.yoga.YogaNative.jni_YGConfigNew()), I'll post some screenshots below of our app showing the difference.

We are using a slightly different version of this commit, since we're still on 0.60, but the gist remains the same when rebased on master.

Before:

After:

Android Lollipop and above support the Paint.setFontFeatureSettings() method, so use that to support the fontVariant TextView prop.

janicduplessis

Looks good to me, just some minor comments to improve nullability annotations.

janicduplessis · 2019-10-28T03:52:08Z

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

@@ -38,6 +43,36 @@ public static int parseFontStyle(@Nullable String fontStyleString) {
    return fontStyle;
  }

+  public static String parseFontVariant(ReadableArray fontVariantArray) {


@Nullable String and @Nullable ReadableArray

janicduplessis · 2019-10-28T03:52:25Z

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

@@ -483,6 +491,16 @@ public void setFontWeight(@Nullable String fontWeightString) {
    }
  }

+  @ReactProp(name = ViewProps.FONT_VARIANT)
+  public void setFontVariant(ReadableArray fontVariantArray) {


@Nullable ReadableArray

janicduplessis · 2019-10-28T03:53:37Z

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

+  public void setFontVariant(ReadableArray fontVariantArray) {
+    String fontFeatureSettings = ReactTypefaceUtils.parseFontVariant(fontVariantArray);
+
+    if (!Objects.equals(fontFeatureSettings, mFontFeatureSettings)) {


I'm not familiar with Object.equals, does it compare all values for an array?

This isn't comparing the ReadableArray, but the String returned from ReactTypefaceUtils.parseFontVariant() call with this.mFontFeatureSettings.

https://docs.oracle.com/javase/7/docs/api/java/util/Objects.html#equals(java.lang.Object,%20java.lang.Object)

TBH, I stumbled upon this via a Stackoverflow post when looking for a shorter way to write this check so it was new to me as well ;)

I might be wrong here, but looks like java.util.Objects and it's static methods was introduced in API level 19 (https://developer.android.com/reference/java/util/Objects). So I think it can possibly crash on Android 4 devices with NoClassDefFoundError: java.util.Objects, when Object.equals is called. We're investigating the similar crash in react-native-camera here: react-native-camera/react-native-camera#2528.

I can replace this with https://github.com/openjdk-mirror/jdk7u-jdk/blob/master/src/share/classes/java/util/Objects.java#L57 if required

janicduplessis · 2019-10-28T03:53:51Z

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

@@ -155,6 +162,14 @@ private float getFloatProp(String name, float defaultvalue) {
    }
  }

+  private ReadableArray getArrayProp(String name) {


@Nullable ReadableArray

janicduplessis · 2019-10-28T03:54:18Z

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

@@ -280,6 +295,10 @@ public void setFontFamily(@Nullable String fontFamily) {
    mFontFamily = fontFamily;
  }

+  public void setFontVariant(ReadableArray fontVariant) {


@Nullable ReadableArray

janicduplessis · 2019-10-28T03:54:59Z

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

  private final @Nullable String mFontFamily;

  public CustomStyleSpan(
      int fontStyle,
      int fontWeight,
+      String fontFeatureSettings,


@Nullable String

janicduplessis · 2019-10-28T14:51:39Z

Cc @mdvacca @JoshuaGross

facebook-github-bot

@mdvacca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

mdvacca

Thanks for working on this @mcuelenaere!

The code looks good to me, I'm landing it!

As a follow up, can you help updating the documentation? https://facebook.github.io/react-native/docs/text-style-props#fontvariant
Thank you!

react-native-bot · 2019-10-29T07:30:06Z

This pull request was successfully merged by @mcuelenaere in c2c4b43.

^{When will my fix make it into a release? | Upcoming Releases}

fontVariant support for Android was added with facebook/react-native#27006

mcuelenaere · 2019-10-29T09:27:15Z

As a follow up, can you help updating the documentation? https://facebook.github.io/react-native/docs/text-style-props#fontvariant
Thank you!

facebook/react-native-website#1443

…< 26 Summary: This diff fixes a crash when using TextInput.FontVariant prop in Android API level < 26 Changelog: Fix TextInput.FontVariant prop in Android API level < 26 (related to PR #27006) Reviewed By: JoshuaGross Differential Revision: D18331807 fbshipit-source-id: 5eac4d9e38eb099fae1287d128f3f8c249b0b8bc

…< 26 Summary: This diff fixes a crash when using TextInput.FontVariant prop in Android API level < 26 Changelog: Fix TextInput.FontVariant prop in Android API level < 26 (related to PR facebook#27006) Reviewed By: JoshuaGross Differential Revision: D18331807 fbshipit-source-id: 5eac4d9e38eb099fae1287d128f3f8c249b0b8bc

fontVariant support for Android was added with facebook/react-native#27006

…< 26 Summary: This diff fixes a crash when using TextInput.FontVariant prop in Android API level < 26 Changelog: Fix TextInput.FontVariant prop in Android API level < 26 (related to PR facebook#27006) Reviewed By: JoshuaGross Differential Revision: D18331807 fbshipit-source-id: 5eac4d9e38eb099fae1287d128f3f8c249b0b8bc

fontVariant support for Android was added with facebook/react-native#27006

Add Android support for fontVariant prop

fcef13c

Android Lollipop and above support the Paint.setFontFeatureSettings() method, so use that to support the fontVariant TextView prop.

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 25, 2019

react-native-bot added Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Oct 25, 2019

janicduplessis suggested changes Oct 28, 2019

View reviewed changes

Add @nullable annotations

a9931f3

janicduplessis approved these changes Oct 28, 2019

View reviewed changes

facebook-github-bot reviewed Oct 28, 2019

View reviewed changes

mdvacca approved these changes Oct 29, 2019

View reviewed changes

facebook-github-bot closed this in c2c4b43 Oct 29, 2019

react-native-bot added the Merged This PR has been merged. label Oct 29, 2019

mcuelenaere added a commit to mcuelenaere/react-native-website that referenced this pull request Oct 29, 2019

Update docs to reflect fontVariant Android support

c91de48

fontVariant support for Android was added with facebook/react-native#27006

mcuelenaere mentioned this pull request Oct 29, 2019

Update docs to reflect fontVariant Android support facebook/react-native-website#1443

Merged

rachelnabors pushed a commit to facebook/react-native-website that referenced this pull request Nov 18, 2019

Update docs to reflect fontVariant Android support (#1443)

e96738c

fontVariant support for Android was added with facebook/react-native#27006

espipj pushed a commit to espipj/react-native-website that referenced this pull request Feb 8, 2020

Update docs to reflect fontVariant Android support (facebook#1443)

097e9ad

fontVariant support for Android was added with facebook/react-native#27006

JackWillie added a commit to JackWillie/react-native-website that referenced this pull request Nov 27, 2022

Update docs to reflect fontVariant Android support (#1443)

32fce84

fontVariant support for Android was added with facebook/react-native#27006

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Android support for fontVariant prop #27006

Add Android support for fontVariant prop #27006

mcuelenaere commented Oct 25, 2019

janicduplessis left a comment

janicduplessis Oct 28, 2019

janicduplessis Oct 28, 2019

janicduplessis Oct 28, 2019

mcuelenaere Oct 28, 2019

haikov Oct 29, 2019

mcuelenaere Oct 29, 2019

janicduplessis Oct 28, 2019

janicduplessis Oct 28, 2019

janicduplessis Oct 28, 2019

janicduplessis commented Oct 28, 2019

facebook-github-bot left a comment

mdvacca left a comment

react-native-bot commented Oct 29, 2019

mcuelenaere commented Oct 29, 2019

Add Android support for fontVariant prop #27006

Add Android support for fontVariant prop #27006

Conversation

mcuelenaere commented Oct 25, 2019

Summary

Changelog

Test Plan

janicduplessis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janicduplessis commented Oct 28, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

mdvacca left a comment

Choose a reason for hiding this comment

react-native-bot commented Oct 29, 2019

mcuelenaere commented Oct 29, 2019