From 83d7a48a46c00b99c52a8ac5897c013924e10152 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Mon, 12 Jun 2023 15:28:56 -0700 Subject: [PATCH] Resubmit D46501420 (#37790) Summary: Multiline text in Android shows some extra space. It's easily noticeable when you set the text `alignSelf` to `flex-start`. This is because we are using `layout.getLineWidth` which will include trailing whitespace. image Based on Android doc, `getLineMax` exclude trailing whitespace. image ## Changelog: [ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width Pull Request resolved: https://github.com/facebook/react-native/pull/37790 Test Plan: After applying the changes: image Code snippet: ``` 1{'\n'} ``` Reviewed By: cortinico Differential Revision: D46586516 Pulled By: NickGerleman fbshipit-source-id: 3ea9c150ad92082f9b4d1da453ba0ef04b09ce51 --- .../com/facebook/react/views/text/FontMetricsUtil.java | 4 +++- .../facebook/react/views/text/ReactTextShadowNode.java | 5 ++++- .../com/facebook/react/views/text/ReactTextView.java | 5 ++++- .../facebook/react/views/text/TextLayoutManager.java | 10 ++++++++-- .../react/views/text/TextLayoutManagerMapBuffer.java | 10 ++++++++-- 5 files changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/FontMetricsUtil.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/FontMetricsUtil.java index f19a1b0425888d..84dc328edab3af 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/FontMetricsUtil.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/FontMetricsUtil.java @@ -42,12 +42,14 @@ public static WritableArray getFontMetrics( X_HEIGHT_MEASUREMENT_TEXT, 0, X_HEIGHT_MEASUREMENT_TEXT.length(), xHeightBounds); double xHeight = xHeightBounds.height() / AMPLIFICATION_FACTOR / dm.density; for (int i = 0; i < layout.getLineCount(); i++) { + boolean endsWithNewLine = text.length() > 0 && text.charAt(layout.getLineEnd(i) - 1) == '\n'; + float lineWidth = endsWithNewLine ? layout.getLineMax(i) : layout.getLineWidth(i); Rect bounds = new Rect(); layout.getLineBounds(i, bounds); WritableMap line = Arguments.createMap(); line.putDouble("x", layout.getLineLeft(i) / dm.density); line.putDouble("y", bounds.top / dm.density); - line.putDouble("width", layout.getLineWidth(i) / dm.density); + line.putDouble("width", lineWidth / dm.density); line.putDouble("height", bounds.height() / dm.density); line.putDouble("descender", layout.getLineDescent(i) / dm.density); line.putDouble("ascender", -layout.getLineAscent(i) / dm.density); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java index 2e237f01970fb5..b69452d9384fcf 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java @@ -135,7 +135,10 @@ public long measure( layoutWidth = width; } else { for (int lineIndex = 0; lineIndex < lineCount; lineIndex++) { - float lineWidth = layout.getLineWidth(lineIndex); + boolean endsWithNewLine = + text.length() > 0 && text.charAt(layout.getLineEnd(lineIndex) - 1) == '\n'; + float lineWidth = + endsWithNewLine ? layout.getLineMax(lineIndex) : layout.getLineWidth(lineIndex); if (lineWidth > layoutWidth) { layoutWidth = lineWidth; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java index 43ffc0d31dc2a7..4af57295d9aac6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java @@ -267,11 +267,14 @@ protected void onLayout( // the last offset in the layout will result in an endless loop. Work around // this bug by avoiding getPrimaryHorizontal in that case. if (start == text.length() - 1) { + boolean endsWithNewLine = + text.length() > 0 && text.charAt(layout.getLineEnd(line) - 1) == '\n'; + float lineWidth = endsWithNewLine ? layout.getLineMax(line) : layout.getLineWidth(line); placeholderHorizontalPosition = isRtlParagraph // Equivalent to `layout.getLineLeft(line)` but `getLineLeft` returns incorrect // values when the paragraph is RTL and `setSingleLine(true)`. - ? textViewWidth - (int) layout.getLineWidth(line) + ? textViewWidth - (int) lineWidth : (int) layout.getLineRight(line) - width; } else { // The direction of the paragraph may not be exactly the direction the string is heading diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java index 25bfe983255cc7..ffd5b2f1a5c4c2 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java @@ -407,7 +407,10 @@ public static long measureText( calculatedWidth = width; } else { for (int lineIndex = 0; lineIndex < calculatedLineCount; lineIndex++) { - float lineWidth = layout.getLineWidth(lineIndex); + boolean endsWithNewLine = + text.length() > 0 && text.charAt(layout.getLineEnd(lineIndex) - 1) == '\n'; + float lineWidth = + endsWithNewLine ? layout.getLineMax(lineIndex) : layout.getLineWidth(lineIndex); if (lineWidth > calculatedWidth) { calculatedWidth = lineWidth; } @@ -462,11 +465,14 @@ public static long measureText( // the last offset in the layout will result in an endless loop. Work around // this bug by avoiding getPrimaryHorizontal in that case. if (start == text.length() - 1) { + boolean endsWithNewLine = + text.length() > 0 && text.charAt(layout.getLineEnd(line) - 1) == '\n'; + float lineWidth = endsWithNewLine ? layout.getLineMax(line) : layout.getLineWidth(line); placeholderLeftPosition = isRtlParagraph // Equivalent to `layout.getLineLeft(line)` but `getLineLeft` returns incorrect // values when the paragraph is RTL and `setSingleLine(true)`. - ? calculatedWidth - layout.getLineWidth(line) + ? calculatedWidth - lineWidth : layout.getLineRight(line) - placeholderWidth; } else { // The direction of the paragraph may not be exactly the direction the string is heading diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java index 71703755597285..8cd576494a1c24 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java @@ -429,7 +429,10 @@ public static long measureText( calculatedWidth = width; } else { for (int lineIndex = 0; lineIndex < calculatedLineCount; lineIndex++) { - float lineWidth = layout.getLineWidth(lineIndex); + boolean endsWithNewLine = + text.length() > 0 && text.charAt(layout.getLineEnd(lineIndex) - 1) == '\n'; + float lineWidth = + endsWithNewLine ? layout.getLineMax(lineIndex) : layout.getLineWidth(lineIndex); if (lineWidth > calculatedWidth) { calculatedWidth = lineWidth; } @@ -484,12 +487,15 @@ public static long measureText( // the last offset in the layout will result in an endless loop. Work around // this bug by avoiding getPrimaryHorizontal in that case. if (start == text.length() - 1) { + boolean endsWithNewLine = + text.length() > 0 && text.charAt(layout.getLineEnd(line) - 1) == '\n'; + float lineWidth = endsWithNewLine ? layout.getLineMax(line) : layout.getLineWidth(line); placeholderLeftPosition = isRtlParagraph // Equivalent to `layout.getLineLeft(line)` but `getLineLeft` returns // incorrect // values when the paragraph is RTL and `setSingleLine(true)`. - ? calculatedWidth - layout.getLineWidth(line) + ? calculatedWidth - lineWidth : layout.getLineRight(line) - placeholderWidth; } else { // The direction of the paragraph may not be exactly the direction the string is