From 2674d9bf7c8b9d668d1a4fe072346b0534bb0f1d Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 8 Jun 2023 12:06:38 -0700 Subject: [PATCH] Exclude trailing whitespace on measuring text width (#37590) 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/37590 Test Plan: After applying the changes: image Code snippet: ``` 1{'\n'} ``` Reviewed By: rshest Differential Revision: D46501420 Pulled By: NickGerleman fbshipit-source-id: fba4acd38747f09791ce8af4439ee88ab12ac827 --- .../com/facebook/react/views/text/FontMetricsUtil.java | 4 +++- .../facebook/react/views/text/ReactTextShadowNode.java | 4 +++- .../java/com/facebook/react/views/text/ReactTextView.java | 4 +++- .../com/facebook/react/views/text/TextLayoutManager.java | 8 ++++++-- .../react/views/text/TextLayoutManagerMapBuffer.java | 8 ++++++-- 5 files changed, 21 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..af97ee9cb91f08 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.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..1e4d1f0b1f3b7d 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,9 @@ public long measure( layoutWidth = width; } else { for (int lineIndex = 0; lineIndex < lineCount; lineIndex++) { - float lineWidth = layout.getLineWidth(lineIndex); + boolean endsWithNewLine = 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..402244d022d723 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,13 @@ 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.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..baee29108b8514 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,9 @@ public static long measureText( calculatedWidth = width; } else { for (int lineIndex = 0; lineIndex < calculatedLineCount; lineIndex++) { - float lineWidth = layout.getLineWidth(lineIndex); + boolean endsWithNewLine = text.charAt(layout.getLineEnd(lineIndex) - 1) == '\n'; + float lineWidth = + endsWithNewLine ? layout.getLineMax(lineIndex) : layout.getLineWidth(lineIndex); if (lineWidth > calculatedWidth) { calculatedWidth = lineWidth; } @@ -462,11 +464,13 @@ 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.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..f0107204f8b962 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,9 @@ public static long measureText( calculatedWidth = width; } else { for (int lineIndex = 0; lineIndex < calculatedLineCount; lineIndex++) { - float lineWidth = layout.getLineWidth(lineIndex); + boolean endsWithNewLine = text.charAt(layout.getLineEnd(lineIndex) - 1) == '\n'; + float lineWidth = + endsWithNewLine ? layout.getLineMax(lineIndex) : layout.getLineWidth(lineIndex); if (lineWidth > calculatedWidth) { calculatedWidth = lineWidth; } @@ -484,12 +486,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.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