Skip to content

Commit

Permalink
Exclude trailing whitespace on measuring text width (#37590)
Browse files Browse the repository at this point in the history
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.
<img width="300" alt="image" src="https://github.com/facebook/react-native/assets/50919443/8939092b-caef-4ad8-9b34-2ccef5d20968">

Based on Android doc, `getLineMax` exclude trailing whitespace.
<img width="625" alt="image" src="https://github.com/facebook/react-native/assets/50919443/0b32e842-5fab-4fc7-8fd9-299877b9c553">

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

Pull Request resolved: #37590

Test Plan:
After applying the changes:
<img width="300" alt="image" src="https://github.com/facebook/react-native/assets/50919443/bfbf52b0-7e7d-4c89-8958-6af38d8bc1c7">

Code snippet:
```
<Text style={{backgroundColor: 'red', alignSelf: 'flex-start', color: 'white'}}>
    1{'\n'}
</Text>
```

Reviewed By: rshest

Differential Revision: D46501420

Pulled By: NickGerleman

fbshipit-source-id: fba4acd38747f09791ce8af4439ee88ab12ac827
  • Loading branch information
bernhardoj authored and facebook-github-bot committed Jun 8, 2023
1 parent 6b62f12 commit 2674d9b
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2674d9b

Please sign in to comment.