Inverted descent/ascent Android prioritisation to match iOS lineHeight behaviour

Summary:
We noticed that on Android the lineHeight behaviour is different from iOS for built in fonts and custom fonts. The problem becomes visible when the lineHeight approaches the fontSize, showing a cut-off on the bottom of the TextView. This issue has been raised before in #10712. There is a mention of a PR with a fix in that issue, which has not been merged yet. This implementation is a less intrusive fix leaving the current lineHeight approach in place and fixing the discrepancy only.

This proposed change prioritises ascent over descent for reduction, making the lineHeight functionality behave identical to iOS.

There is no existing test covering the lineHeight property and its behaviour in the CustomLineHeightSpan. This PR contains new unit tests that covers the various scenario's for the lineHeight calculations.

The original behaviour, before the change can against these unit tests. The case that fails is `shouldReduceAscentThird`, which can be made to succeed on the old code by changing the asserts to:
```
    assertThat(fm.top).isEqualTo(-5);
    assertThat(fm.ascent).isEqualTo(-5);
    assertThat(fm.descent).isEqualTo(-4);
    assertThat(fm.bottom).isEqualTo(-4);
```
The unit test succeeds for the current implementation, which has the values for ascent and descent inverted.

Below screenshots show before, after and iOS:

BEFORE
![screen shot 2017-10-18 at 15 35 41](https://user-images.githubusercontent.com/1605731/31721688-58d7086a-b41a-11e7-8186-9a201e2acb01.png)

AFTER
![screen shot 2017-10-18 at 15 37 02](https://user-images.githubusercontent.com/1605731/31721665-473cf86c-b41a-11e7-94d5-7a70eaf99889.png)

iOS
![screen shot 2017-10-18 at 15 35 22](https://user-images.githubusercontent.com/1605731/31721712-707e30a6-b41a-11e7-9baa-f886a66837e6.png)

[ANDROID] [BUGFIX] [Text] - Fix the lineHeight behaviour on Android to match iOS
Closes https://github.com/facebook/react-native/pull/16448

Differential Revision: D6221854

Pulled By: andreicoman11

fbshipit-source-id: 7292f0f05f212d79678ac9d73e8a46bf93f1a7c6
This commit is contained in:
Bartol Karuza
2017-11-06 04:48:00 -08:00
committed by Facebook Github Bot
parent b9f21dc2be
commit 3f1b021506
2 changed files with 111 additions and 12 deletions

View File

@@ -0,0 +1,89 @@
package com.facebook.react.views.text;
import android.graphics.Paint;
import static org.fest.assertions.api.Assertions.assertThat;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.robolectric.RobolectricTestRunner;
@RunWith(RobolectricTestRunner.class)
@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"})
public class CustomLineHeightSpanTest {
@Test
public void shouldIncreaseAllMetricsProportionally() {
CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(22);
Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
fm.top = -10;
fm.ascent = -5;
fm.descent = 5;
fm.bottom = 10;
customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
assertThat(fm.top).isEqualTo(-11);
assertThat(fm.ascent).isEqualTo(-6);
assertThat(fm.descent).isEqualTo(6);
assertThat(fm.bottom).isEqualTo(11);
}
@Test
public void shouldReduceTopFirst() {
CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(19);
Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
fm.top = -10;
fm.ascent = -5;
fm.descent = 5;
fm.bottom = 10;
customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
assertThat(fm.top).isEqualTo(-9);
assertThat(fm.ascent).isEqualTo(-5);
assertThat(fm.descent).isEqualTo(5);
assertThat(fm.bottom).isEqualTo(10);
}
@Test
public void shouldReduceBottomSecond() {
CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(14);
Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
fm.top = -10;
fm.ascent = -5;
fm.descent = 5;
fm.bottom = 10;
customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
assertThat(fm.top).isEqualTo(-5);
assertThat(fm.ascent).isEqualTo(-5);
assertThat(fm.descent).isEqualTo(5);
assertThat(fm.bottom).isEqualTo(9);
}
@Test
public void shouldReduceAscentThird() {
CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(9);
Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
fm.top = -10;
fm.ascent = -5;
fm.descent = 5;
fm.bottom = 10;
customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
assertThat(fm.top).isEqualTo(-4);
assertThat(fm.ascent).isEqualTo(-4);
assertThat(fm.descent).isEqualTo(5);
assertThat(fm.bottom).isEqualTo(5);
}
@Test
public void shouldReduceDescentLast() {
CustomLineHeightSpan customLineHeightSpan = new CustomLineHeightSpan(4);
Paint.FontMetricsInt fm = new Paint.FontMetricsInt();
fm.top = -10;
fm.ascent = -5;
fm.descent = 5;
fm.bottom = 10;
customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm);
assertThat(fm.top).isEqualTo(0);
assertThat(fm.ascent).isEqualTo(0);
assertThat(fm.descent).isEqualTo(4);
assertThat(fm.bottom).isEqualTo(4);
}
}