Skip to content

Commit

Permalink
Fix the bug that TextSpec doesn't re-layout when minimallyWide is…
Browse files Browse the repository at this point in the history
… enabled

Summary:
# Context
This diff is to fix a bug that was identified from this oncall [post](https://fb.workplace.com/groups/litho.support/permalink/3693122427675765/), basically it's because we didn't re-layout text with the "minimal" size. The issue is hard to be noticed because we usually use the default text alignment(START) rather than `CENTER`.

This should also address the similar [issue](https://fb.workplace.com/groups/litho.support/permalink/2961919757462706/), which was posted few years ago.

`minimallyWide` was introduced by D13595632, which was basically trying to shrink the width of the text to make it take up as less space as possible.

Reviewed By: astreet

Differential Revision: D53041943

fbshipit-source-id: 44c4bee570b8215cb5b9ca79c372ad7f23064f3c
  • Loading branch information
Andrew Wang authored and facebook-github-bot committed Feb 2, 2024
1 parent b6479b9 commit 0574c1e
Showing 1 changed file with 26 additions and 3 deletions.
29 changes: 26 additions & 3 deletions litho-widget/src/main/java/com/facebook/litho/widget/TextSpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ static void onMeasure(
@Prop(optional = true, resType = ResType.DIMEN_TEXT) float lineHeight,
Output<Layout> measureLayout,
Output<Integer> measuredWidth,
Output<Integer> measuredHeight) {
Output<Integer> measuredHeight,
Output<Boolean> needMinimallyWideLayout) {

if (TextUtils.isEmpty(text)) {
measureLayout.set(null);
Expand All @@ -341,6 +342,17 @@ static void onMeasure(
return;
}

TextAlignment resolvedTextAlignment = getTextAlignment(textAlignment, alignment);
boolean isTextAlignmentOverride = false;
if (minimallyWide && resolvedTextAlignment == TEXT_END) {
// To ensure correct minimally wide behavior, we need to perform text layout twice. This is
// because we cannot accurately retrieve the width of each line from TextLayout if we have
// alignment from the end. Therefore, we run the first pass with TEXT_START enforced, and do
// the second layout in onBoundsDefined with the actual alignment setting.
resolvedTextAlignment = TEXT_START;
isTextAlignmentOverride = true;
}

Layout newLayout =
createTextLayout(
context,
Expand All @@ -363,7 +375,7 @@ static void onMeasure(
letterSpacing,
textStyle,
typeface,
getTextAlignment(textAlignment, alignment),
resolvedTextAlignment,
glyphWarming,
layout.getResolvedLayoutDirection(),
minEms,
Expand All @@ -379,7 +391,12 @@ static void onMeasure(

measureLayout.set(newLayout);

final int fullWidth = Math.max(0, SizeSpec.resolveSize(widthSpec, newLayout.getWidth()));
size.width = resolveWidth(widthSpec, newLayout, minimallyWide, minimallyWideThreshold);
if (isTextAlignmentOverride || (minimallyWide && fullWidth != size.width)) {
// either TextAlignment is TEXT_END or minimally wide width is different
needMinimallyWideLayout.set(true);
}

// Adjust height according to the minimum number of lines.
int preferredHeight = LayoutMeasureUtil.getHeight(newLayout);
Expand Down Expand Up @@ -602,6 +619,7 @@ static void onBoundsDefined(
@FromMeasure Layout measureLayout,
@FromMeasure Integer measuredWidth,
@FromMeasure Integer measuredHeight,
@FromMeasure Boolean needMinimallyWideLayout,
Output<CharSequence> processedText,
Output<Layout> textLayout,
Output<Float> textLayoutTranslationY,
Expand All @@ -617,8 +635,12 @@ static void onBoundsDefined(
layout.getWidth() - layout.getPaddingLeft() - layout.getPaddingRight();
final float layoutHeight =
layout.getHeight() - layout.getPaddingTop() - layout.getPaddingBottom();
final boolean forceRelayout = needMinimallyWideLayout != null && needMinimallyWideLayout;

if (measureLayout != null && measuredWidth == layoutWidth && measuredHeight == layoutHeight) {
if (measureLayout != null
&& !forceRelayout
&& measuredWidth == layoutWidth
&& measuredHeight == layoutHeight) {
textLayout.set(measureLayout);
} else {
textLayout.set(
Expand Down Expand Up @@ -881,6 +903,7 @@ private static int getEllipsisOffsetFromPaintAdvance(

return paint.getOffsetForAdvance(text, lineStart, lineEnd, lineStart, lineEnd, isRtl, advance);
}

/**
* @param layout A prepared text layout object
* @return The (zero-indexed) line number at which the text in this layout will be ellipsized, or
Expand Down

0 comments on commit 0574c1e

Please sign in to comment.