From 0574c1ef99f116f535e6a9eeef7934e12ae6c655 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Fri, 2 Feb 2024 11:35:45 -0800 Subject: [PATCH] Fix the bug that `TextSpec` doesn't re-layout when `minimallyWide` is 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 --- .../com/facebook/litho/widget/TextSpec.java | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/litho-widget/src/main/java/com/facebook/litho/widget/TextSpec.java b/litho-widget/src/main/java/com/facebook/litho/widget/TextSpec.java index 9764fdb1440..51ca0a953bd 100644 --- a/litho-widget/src/main/java/com/facebook/litho/widget/TextSpec.java +++ b/litho-widget/src/main/java/com/facebook/litho/widget/TextSpec.java @@ -332,7 +332,8 @@ static void onMeasure( @Prop(optional = true, resType = ResType.DIMEN_TEXT) float lineHeight, Output measureLayout, Output measuredWidth, - Output measuredHeight) { + Output measuredHeight, + Output needMinimallyWideLayout) { if (TextUtils.isEmpty(text)) { measureLayout.set(null); @@ -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, @@ -363,7 +375,7 @@ static void onMeasure( letterSpacing, textStyle, typeface, - getTextAlignment(textAlignment, alignment), + resolvedTextAlignment, glyphWarming, layout.getResolvedLayoutDirection(), minEms, @@ -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); @@ -602,6 +619,7 @@ static void onBoundsDefined( @FromMeasure Layout measureLayout, @FromMeasure Integer measuredWidth, @FromMeasure Integer measuredHeight, + @FromMeasure Boolean needMinimallyWideLayout, Output processedText, Output textLayout, Output textLayoutTranslationY, @@ -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( @@ -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