Lineheight behaves differently on ios and android

[MarcoPolo]

Description

On ios:

On Android:

note how the on android it seems like the line height is only applied on top of the text, while on ios it's applied to both top and bottom. This is also an issue when you have lineheights < fontSize. On ios it trims from the top, but on android it'll trim from top and bottom, so the text doesn't line up and get's truncated differently.

Reproduction

RNplay demo: https://rnplay.org/apps/OJFGEg

Additional Information

• React Native version: 0.36
• Platform: iOS & Android
• Operating System: MacOS

[ugiacoman]

Just ran into this issue, can confirm still exists

[dsimmons]

Ditto, can confirm that I'm still seeing this on RN v0.45.0 (latest).

It's not apparent to me if this is intended behavior based on the corresponding underlying native capabilities, or rather an unintentional discrepancy.

I'd just like to add that it certainly makes achieving a consistent layout between both platforms challenging, in achieving parity with pre-designed screens. I've resorted to disabling lineHeight on Android in the interim, as I'm unable to control how exactly the text appears.

Is this on anyone's radar by chance?

[henrytao-me]

I did a little test on CustomLineHeightSpan.java. It works on new pure Android project but not on RNTesterApp. I am still investigating what the different between ReactNative TextView and PureAndroid TextView. I would like to pair with ReactNative core team to work on this investigation (if possible).

The idea is simple: use descent and baseline as base metrics, then recalculate bottom, ascent and top based on fontSize and lineHeight. ascent and descent will be centered.

// Copyright 2004-present Facebook. All Rights Reserved.

package com.facebook.react.views.text;

import android.graphics.Paint;
import android.text.style.LineHeightSpan;

/**
 * We use a custom {@link LineHeightSpan}, because `lineSpacingExtra` is broken. Details here:
 * https://github.com/facebook/react-native/issues/7546
 */
public class CustomLineHeightSpan implements LineHeightSpan {
  private final int mHeight;
  private final int mSize;

  CustomLineHeightSpan(float size, float height) {
    this.mSize = (int) size;
    this.mHeight = Math.max((int) height, mSize);
  }

  @Override
  public void chooseHeight(
      CharSequence text,
      int start,
      int end,
      int spanstartv,
      int v,
      Paint.FontMetricsInt fm) {
    int spacing = (mHeight - mSize) / 2;
    fm.bottom = fm.descent + spacing;
    fm.ascent = fm.descent - mSize;
    fm.top = fm.descent - mSize - spacing;
  }
}

[henrytao-me]

Checkout my proposal for CustomLineHeightSpan.java fixed. henrytao-me#1.

[pawlowskim]

Same problems with fonts on RN: 0.44.0. There are few issues as far as I know about this, or very similar behaviour.

[igorczapski]

It's a big problem for me. I'm working on a project in which correct font rendering is critical. As a temporary workaround I have applied lineHeight bigger than fontSize.
I also added paddingBottom and marginTop to negative value. I had to manually adjust these values for each font. This was to prevent Text component from growing due to increased line height. But it's not a production solution, I don't recommend that, this was my last resort.

[peterp]

@henrytao-me it seems that you've made a pull-request against your own forked react-native repo whilst you should have made it against the Facebook repo. Making a pull-request against master is going to be the most effective way of getting some decent feedback.

Please check up on #14611

[henrytao-me]

Thanks @peterp for suggestion. Some how I missed this conversation. I will create new PR soon.

[rajatbeck]

Facing the same issues...any alternatives??

[bartolkaruza]

I've opened a PR fixing this issue. The reason I worked on a simpler solution than @henrytao-me's proposed solution is that the number of cases when working with font rendering is high and the only difference between Android and iOS rendering is the priority of ascent over descent. The change in #16448 touches only that and adds some unit test coverage for confidence that nothing else changes in the calculation. I'm hoping this will make it easier to review and land this fix soon.