Inverted descent/ascent Android prioritisation to match iOS lineHeight behaviour

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

@@ -1,4 +1,11 @@
-// Copyright 2004-present Facebook. All Rights Reserved.
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
 
 package com.facebook.react.views.text;
 
@@ -27,16 +34,16 @@ public void chooseHeight(
     // This is more complicated that I wanted it to be. You can find a good explanation of what the
     // FontMetrics mean here: http://stackoverflow.com/questions/27631736.
     // The general solution is that if there's not enough height to show the full line height, we
-    // will prioritize in this order: ascent, descent, bottom, top
+    // will prioritize in this order: descent, ascent, bottom, top
 
-    if (-fm.ascent > mHeight) {
-      // Show as much ascent as possible
-      fm.top = fm.ascent = -mHeight;
-      fm.bottom = fm.descent = 0;
+    if (fm.descent > mHeight) {
+      // Show as much descent as possible
+      fm.bottom = fm.descent = Math.min(mHeight, fm.descent);
+      fm.top = fm.ascent = 0;
     } else if (-fm.ascent + fm.descent > mHeight) {
-      // Show all ascent, and as much descent as possible
-      fm.top = fm.ascent;
-      fm.bottom = fm.descent = mHeight + fm.ascent;
+      // Show all descent, and as much ascent as possible
+      fm.bottom = fm.descent;
+      fm.top = fm.ascent = -mHeight + fm.descent;
     } else if (-fm.ascent + fm.bottom > mHeight) {
       // Show all ascent, descent, as much bottom as possible
       fm.top = fm.ascent;
@@ -45,10 +52,13 @@ public void chooseHeight(
       // Show all ascent, descent, bottom, as much top as possible
       fm.top = fm.bottom - mHeight;
     } else {
-      // Show proportionally additional ascent and top
+      // Show proportionally additional ascent / top & descent / bottom
       final int additional = mHeight - (-fm.top + fm.bottom);
-      fm.top -= additional;
-      fm.ascent -= additional;
+
+      fm.top -= additional / 2;
+      fm.ascent -= additional / 2;
+      fm.descent += additional / 2;
+      fm.bottom += additional / 2;
     }
   }
 }

ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.java

@@ -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);
+  }
+}