TextLayout: take full width if text wrapped

[s77rt]

Summary:

Problem

The calculated width for a multiline text is based on the longest line. However it does not account for text that wraps.

Example if numberOfLines=1 and the text wraps

+---------------------------+
This is a long text that
will wrap
+---------------------------+

The TextView will render

+---------------------------+
This is a long text t...
+---------------------------+

because the calculatedWidth took the width of the first line.

Also see #41770 (comment) for additional context.

Solution

If the text wraps, take the whole width.

+---------------------------+
This is a long text that w...
+---------------------------+

Fixes #39722
Fixes facebook/yoga#1730

Changelog:

[GENERAL] [FIXED] - Fix text not taking full width

Test Plan:

        <Text
          numberOfLines={1}
          style={{
            backgroundColor: 'red',
            alignSelf: 'flex-start',
            color: 'white',
            fontSize: 34,
          }}>
          {'This is a long text that will wrap.'}
        </Text>
        <Text
          numberOfLines={3}
          style={{
            backgroundColor: 'red',
            alignSelf: 'flex-start',
            color: 'white',
            fontSize: 34,
          }}>
          {
            '1\n\ntest\nThis is a long text that will wrap.This is a long text that will wrap.This is a long text that will wrap.\n'
          }
        </Text>
        <Text
          numberOfLines={3}
          style={{
            backgroundColor: 'red',
            alignSelf: 'flex-start',
            color: 'white',
            fontSize: 34,
          }}>
          {
            '1\n\nThis is a long text that will wrap.This is a long text that will wrap.This is a long text that will wrap.\n'
          }
        </Text>

1. Verify that the first and third text take full width
2. Verify that the second text does not take full width

Before	After

[s77rt]

cc @cipolleschi or @realsoelynn You may want to review this since you have the context from #41770

[facebook-github-bot]

@realsoelynn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[NickGerleman]

That also likely fixes issue reported here. facebook/yoga#1730

I think we may want this behavior regardless of max number of lines. Wrapping when we have AtMost constraint should be changed on iOS/Android to take full available width.

[facebook-github-bot]

@realsoelynn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[s77rt]

@NickGerleman It won't fix the other issue. That one is in yoga. I will look into it as well

[s77rt]

@realsoelynn I see a failing internal test. Does this require any action from my side?

[NickGerleman]

@s77rt @realsoelynn the other issue was reported to Yoga, but I highly suspect it is RN measure function issue related to what this change is addressing.

[s77rt]

Making this a draft, will work on what @NickGerleman suggested. It should fix both this issue and the other one

[s77rt]

iOS	Android

[s77rt]

Ready for review. cc @realsoelynn @NickGerleman

[cipolleschi]

FYI: @NickGerleman is on PTO this week. He should be back next week

[facebook-github-bot]

@realsoelynn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[NickGerleman]

Back from PTO! Took a look, and I think we might need to handle max-content differently.

[NickGerleman]

I think we may also reach this path when widthYogaMeasureMode is YogaMeasureMode.UNDEFINED, in which case we are asking for max-content size, and the width is NaN.

[NickGerleman]

I think on L604, we could replace:

if (widthYogaMeasureMode == YogaMeasureMode.AT_MOST && calculatedWidth > width)

with

if (widthYogaMeasureMode == YogaMeasureMode.AT_MOST && (calculatedWidth > width || layout.getLineCount() > 1))

[s77rt]

I think we may also reach this path when widthYogaMeasureMode is YogaMeasureMode.UNDEFINED

In this case, should we keep the calculated width as the max width of the lines?

I think on L604, we could replace

We set calculatedWidth = width only if the text wraps due to overflow. If the user write 2 short lines we still use the width of the maximum line

[NickGerleman]

I think we may have similar case to the above, for measuring max-content, where we don't want to set to size. I think in this case, it looks like that might be represented as Infinity layout constraint which makes its way to the NSTextContainer.

[NickGerleman]

Would this correctly detect wrapping for something like the below?

Hello\n
World\n

I think we may be able to just count lines. https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/TextLayout/Tasks/CountLines.html#//apple_ref/doc/uid/20001810-CJBGBIBB

[s77rt]

No, that's not considered text wrap. In this case we want the current behaviour (text width) otherwise we'd break case 2 from #47435 (comment)

[NickGerleman]

Ah, you are correct about only wanting to do this when we need to break due to wrapping.

That also solves the max-content case, since we will never wrap in this case since we have unconstrained width.

So I think this is good!

[facebook-github-bot]

@realsoelynn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @s77rt in 550b0c0

^{When will my fix make it into a release? | How to file a pick request?}

[facebook-github-bot]

@realsoelynn merged this pull request in 550b0c0.