Skip to content

Commit

Permalink
do truncation inline with main layout
Browse files Browse the repository at this point in the history
Summary:
Reviewing a new batch of screenshots after D54698703 showed some more flaws in how we're truncating.

The initial height of the text layout is derived from the layout object
https://www.internalfb.com/code/fbsource/[05747540c982b8816899085a43b5ec9160d12f08]/fbandroid/libraries/rendercore/rendercore-text/src/main/java/com/facebook/rendercore/text/TextMeasurementUtils.java?lines=295

But the layout logic can add and remove some spacing from the initial height in the following scenarios:
https://www.internalfb.com/code/fbsource/[05747540c982b8816899085a43b5ec9160d12f08]/fbandroid/libraries/rendercore/rendercore-text/src/main/java/com/facebook/rendercore/text/TextMeasurementUtils.java?lines=297-298%2C300-302

https://www.internalfb.com/code/fbsource/[05747540c982b8816899085a43b5ec9160d12f08]/fbandroid/libraries/rendercore/rendercore-text/src/main/java/com/facebook/rendercore/text/TextMeasurementUtils.java?lines=319-325

This eventually results in the final reported height being different from the actual layout's height.

In our current approach to truncation, we compare the reported (modified) height against the initial text layout's height and then go ahead to truncate which is not really correct.

What we should do is truncate the initial layout against the height constraints in the measure spec and then go on to modify the spacing as needed. This should be more correct.
----
I expect most (if not all) of the failing tests to start passing after this lands. This is the last change to the logic (hopefully)

Reviewed By: rooju

Differential Revision: D54874150

fbshipit-source-id: 1b6028e8836cc20faf5b059718654d5985eccc91
  • Loading branch information
Daniel Famakin authored and facebook-github-bot committed Mar 15, 2024
1 parent 4380d0a commit 8c2f2d8
Showing 1 changed file with 31 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,6 @@ public static MountableLayoutResult layout(
}

Pair<Rect, TextLayout> result = layout(androidContext, widthSpec, heightSpec, text, textStyle);
final boolean fitTextToConstraints =
textStyle.shouldTruncateTextUsingConstraints
&& textStyle.maxLines == Integer.MAX_VALUE
&& View.MeasureSpec.getMode(heightSpec) != View.MeasureSpec.UNSPECIFIED;
if (fitTextToConstraints) {
final int measuredHeightConstraint = result.first.height();
final boolean isSizeCompatible =
LayoutMeasureUtil.getHeight(result.second.layout) <= measuredHeightConstraint;
if (!isSizeCompatible) {
result =
maybeFitTextToConstraints(
androidContext, heightSpec, widthSpec, textStyle, text, result);
}
}

if (textStyle.roundedBackgroundProps != null && text instanceof Spannable) {
result =
calculateLayoutWithBackgroundSpan(
Expand All @@ -117,39 +102,6 @@ public static MountableLayoutResult layout(
renderUnit, result.first.width(), result.first.height(), result.second);
}

private static Pair<Rect, TextLayout> maybeFitTextToConstraints(
Context context,
int heightSpec,
int widthSpec,
TextStyle textStyle,
CharSequence text,
Pair<Rect, TextLayout> initialResult) {
final Layout layout = initialResult.second.layout;

final int measuredHeightConstraint = initialResult.first.height();

// Make sure we have the bare minimum line count if we've exhausted all lines and weren't able
// to fit. This allows to show the ellipsis in the best case scenario signaling truncation
int linesWithinConstrainedBounds = 1;
int lineIndex = layout.getLineCount() - 1;
while (lineIndex >= 0) {
if (layout.getLineBottom(lineIndex) <= measuredHeightConstraint) {
linesWithinConstrainedBounds = lineIndex + 1;
break;
}

lineIndex -= 1;
}

if (layout.getLineCount() > linesWithinConstrainedBounds) {
textStyle.setMaxLines(linesWithinConstrainedBounds);
textStyle.setEllipsize(TextUtils.TruncateAt.END);
return layout(context, widthSpec, heightSpec, text, textStyle);
}

return initialResult;
}

public static Pair<Rect, TextLayout> calculateLayoutWithBackgroundSpan(
final Context context,
final Pair<Rect, TextLayout> result,
Expand Down Expand Up @@ -259,6 +211,37 @@ public static Pair<Rect, TextLayout> layout(
TextMeasurementUtils.createTextLayout(
context, textStyle, widthSpec, heightSpec, processedText);

// check if the layout should truncate based on the size constraints
int linesWithinConstrainedBounds = -1;
if (View.MeasureSpec.getMode(heightSpec) != View.MeasureSpec.UNSPECIFIED) {
final int heightConstraint = View.MeasureSpec.getSize(heightSpec);
final boolean fitTextToConstraints =
textStyle.shouldTruncateTextUsingConstraints
&& textStyle.maxLines == Integer.MAX_VALUE
&& LayoutMeasureUtil.getHeight(layout) > heightConstraint;

if (fitTextToConstraints) {
linesWithinConstrainedBounds = 1;
int lineIndex = layout.getLineCount() - 1;
while (lineIndex >= 0) {
if (layout.getLineBottom(lineIndex) <= heightConstraint) {
linesWithinConstrainedBounds = lineIndex + 1;
break;
}

lineIndex -= 1;
}
}
}

if (linesWithinConstrainedBounds != -1) {
// we have constrained the number of lines that can fit, so truncate the layout
textStyle.setMaxLines(linesWithinConstrainedBounds);
layout =
TextMeasurementUtils.createTextLayout(
context, textStyle, widthSpec, heightSpec, processedText);
}

final int layoutWidth =
View.resolveSize(
layout.getWidth()
Expand Down

0 comments on commit 8c2f2d8

Please sign in to comment.