Implement adjustsFontSizeToFit on Android

[janicduplessis]

Summary

This adds support for adjustsFontSizeToFit and minimumFontScale on Android. The implementation tries to match closely the behaviour on iOS (hardcoded 4px min size for example). It uses a simpler linear algorithm for now, opened to improving it now if it is a deal breaker or in a follow up.

See https://twitter.com/janicduplessis/status/1171147709979516929 for a more detailed thread about the implementation

Changelog

[Android] [Added] - Implement adjustsFontSizeToFit on Android

Test Plan

Tested by adding the existing adjustsFontSizeToFit example from the iOS text page to android. Also added a case for limiting size by using maxHeight instead of numberOfLines.

[janicduplessis]

Not related but it was unused (I worked on this feature so it should be safe).

[janicduplessis]

We could use some sort of binary search here like on iOS but I didn't do it for now since it adds quite a bit of complexity. In my tests it took between 5-10 iterations here.

[JoshuaGross]

For now when this is merged in, we should probably add documentation indicating that this feature could be pretty slow.

Is it possible for this while loop to never terminate? Should we enforce a max # of iterations so that we never get into an infinite loop?

[janicduplessis]

👍

The loop will always terminate because of the currentFontSize > minimumFontSize condition. The minimum font size will never be smaller than 4.

[janicduplessis]

I had to do this otherwise the ellipsis still appeared some times, it seemed to happen when there was whitespace at the end of the line. The only downside is the ellipsis won't show if we hit the minimum font scale, not sure how big of a deal this is.

[JoshuaGross]

So to be clear, that means if there's a very long chunk of text and you hit the minimum font size, the text would be truncated without ellipses? That seems like a bug to me, though I probably wouldn't block this PR on it, but how does iOS handle it? How difficult do you think it is to fix both issues?

[janicduplessis]

Exactly, text would be truncated without ellipses. AFAIK it only happens on Android and seemed to happen when the text had room to render according to our measurements but the ellipsis would still show when drawing the text.

Since ellispsising should never happen when adjusting size to fit unless we hit the minimum size that seemed like an okay compromise since I'm pretty sure hitting minimum size is rare and the default is so small it would barely be noticeable.

I haven't investigated why it happens as it would probably require debugging deep inside android text rendering pipeline where the ellipsis rendering code is to figure out why it appears.

[michaelknoch]

What about using the android builtin feature for automatic font sizing?
TextViewCompat .setAutoSizeTextTypeUniformWithConfiguration

We are using this feature in our project successfully even on android api 21. I can publish the package if you are interested in the code @janicduplessis

[kmagiera]

Saw this PR linked on tt and really happy that we may soon be able to use adjust prop cross platform.

One thing I’m not sure about (even after reading or description and your tt thread) is why this has to happen in measure phase. I think adjust should work even on text views with fixed dimensions in which case measuring isn’t needed (I think we actually call text measure on android even when not needed because it’s responsible for creating spannables that are then used on the UI thread). I am perhaps missing something but if in fact we don’t need to do it on shadow node / measure phase I think compact library solution should be possible to adapt.

[janicduplessis]

@michaelknoch I did a quick attempt at using this api but was not successful, I’d love to see the code if you got it working. However I’m not sure how well it would integrate with RN compared to the solution here, will go into more detail after.

[janicduplessis]

@kmagiera The main reason is that it will affect the layout of the text. Since we’re reducing the font size the height will change, this is why I thought it made sense to do during layout.

You can see it in the following pictures https://imgur.com/a/YLtxut0.

It should also work with fixed height, I actually only tested with a maxHeight but I can check to make sure measure gets called there too.

Do you see any issue with doing this in the measure phase? And do you see a better time to do it?

As for the compat lib solution I think it will be hard and not optimal to communicate the layout back to yoga. I also assume that it does the adjusting on the main thread, I think we can leverage background thread layout of RN to make our implementation better. I can check the source for the Android implementation see if they use any tricks to make it better.

[shergin]

Importing to run internal tests.

[facebook-github-bot]

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

[shergin]

@rigdern Adam, your input would be greatly appreciated.

[janicduplessis]

Did a bit of research on the AOSP implementation and it's pretty similar to what we have for the measuring code, like I thought everything is done on the main thread, autoSizeText is called in onLayout. https://android.googlesource.com/platform/frameworks/base.git/+/master/core/java/android/widget/TextView.java#9467, https://android.googlesource.com/platform/frameworks/base.git/+/master/core/java/android/widget/TextView.java#9491.

I think the main improvement needed is to binary search the font sizes for perf.

[janicduplessis]

@shergin Do you think this implementation also makes sense for fabric? Would it be easy to integrate?

[facebook-github-bot]

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

[JoshuaGross]

@janicduplessis Can you add the new prop(s) to flow types in AndroidTextInputNativeComponent.js?

[janicduplessis]

@JoshuaGross This is for Text and not TextInput, looks like Text doesn't use codegen yet.

[janicduplessis]

@kmagiera @shergin @JoshuaGross Any change needed on my part to help land this?

[conrc]

Any update? Eagerly awaiting this merge :)

[yurykorzun]

I've been waiting for so long for this feature to be available on Android, I hope it gets merged soon!

[kdo1234]

This is probably a popular pull request. Is there anything that a volunteer can do to help get this merged? Thank you

[rebirthtobi]

Eagerly waiting for this

[yurykorzun]

If anyone wants to try this feature before this pull request is merged, you can try to reference my fork of RN 0.61.5 with the applied patch updated aar binary
"dependencies": {
...
"react-native": "Flyclops/react-native#adjust-text-size-android"
...
}

[janicduplessis]

ping @JoshuaGross @shergin @cpojer

[andre-krueger]

Will this be included in 0.62? This is a really crucial feature.

We currently have a workaround for this in our projects, but it doesn't work correctly all the time and the performance issues are another drawback.

[janicduplessis]

@JoshuaGross Did you have any issues landing this? Also I assume this might cause issues with fabric since this feature would need to be supported there too.

[facebook-github-bot]

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

[JoshuaGross]

@janicduplessis It should be fine - pending internal code review and testing. We will need to support this for Fabric but I'm not too worried, I think it will ~reasonably easy to support.

[JoshuaGross]

I likely won't land this until Monday PST, but I will try to do it by then.

[react-native-bot]

This pull request was successfully merged by @janicduplessis in 2c1913f.

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

[brianwestphal]

This doesn't seem to support ellipses yet, once the minimumFontScale is reached and not enough space is available, like it does on iOS.

[janicduplessis]

@brianwestphal Sadly this is a known issue and haven't found a proper way to deal with it #26389 (comment)

[SrBrahma]

Sorry for commenting on a old issue, but, does adjustsFontSizeToFit works for Android or not? Here on my Redmi Note 8 (Android) it is working, but the docs says it's a iOS only feature.

[janicduplessis]

I works, the docs might not have been updated

[giantslogik]

This seems to be broken on Android 9. Seems to display with fontSize 4 always.

Worked with android 10.

[winardis]

@giantslogik mine worked fined on my device running Android 9 with react-native version 0.63.3
but i'm not so sure now, because you mentioned that on your end it always returns 4 as fontSize

also the documentation didn't update that it supports Android yet, so kinda confused here as well