Fix `textAlign` with inline views on the new architecture on Android

[j-piasecki]

Summary:

On the new architecture on Android on the new arch, textAlign style was ignored (Layout.Alignment.ALIGN_NORMAL was always used) during the measurement of text. During this phase, the positions of attachments are also calculated, which results in inline views being always positioned as if alignment to the left was set. This PR updates the measurement logic to also take textAlign into account during measurement.

Fixes https://github.com/facebook/react-native/issues/41008

Changelog:

[ANDROID] [FIXED] - Fixed textAlign not being taken into account when positioning views inlined in text

Test Plan:

I've been testing on the following code

import { SafeAreaView, Text, View } from "react-native";

function InlineView(props) {
  return (<View style={{margin: 10}} >
    <Text style={{ textAlign: props.textAlign, backgroundColor: 'cyan' }}>
        Parent Text
        <Text style={{ fontWeight: 'bold' }}>Child Text</Text>
        <View style={{width: 50, height: 50, backgroundColor: 'red'}} />
        <Text style={{ fontWeight: 'bold' }}>Child Text</Text>
        {props.long && <Text style={{ fontWeight: 'bold' }}>aaaa a aaaa aaaaaa aaa a a a aaaaa sdsds dsdSAD asd ASDasd ASDas</Text>}
      </Text>
  </View>)
}

export default function Test() {
  return (
    <SafeAreaView style={{ flex: 1 }}>
      <Text style={{textAlign: 'center', fontSize: 20}}>BoringLayout</Text>
      <InlineView textAlign="left" />
      <InlineView textAlign="center" />
      <InlineView textAlign="right" />
      <InlineView textAlign="justify" />

      <Text style={{textAlign: 'center', fontSize: 20}}>StaticLayout</Text>
      <InlineView textAlign="left" long />
      <InlineView textAlign="center" long />
      <InlineView textAlign="right" long />
      <InlineView textAlign="justify" long/>
    </SafeAreaView>
  );
}

Old architecture	New architecture

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,410,758	-49,283
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,784,276	-49,248
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: b40b3b31a2447abfcae753b60eacd812329427d3 Branch: main

[facebook-github-bot]

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

[j-piasecki]

I would also really appreciate it if you could double-check if everything works in RTL layouts. I was sure that Layout.Alignment.ALIGN_NORMAL and Layout.Alignment.ALIGN_OPPOSITE would work without any additional logic, but they always end up aligning the text to the left and right respectively regardless of whether RTL is enabled or not.

[j-piasecki]

I've pushed some changes and here is what it look like:

The code I've tested

import { SafeAreaView, ScrollView, Text, View } from "react-native";

function InlineViewArabic(props) {
  return (<View style={{margin: 10}} >
    <Text style={{ textAlign: props.textAlign, backgroundColor: 'cyan' }}>
      تخطيط الاختبار
      <View style={{width: 50, height: 50, backgroundColor: 'red'}} />
      <Text style={{ fontWeight: 'bold' }}>تخطيط الاختبار</Text>
      {props.long && <Text style={{ fontWeight: 'bold' }}>تخطيط الاختبارتخطيط الاختبارتخطيط الاختبارتخطيط الاختبارتخطيط الاختبارتخطيط الاختبار</Text>}
    </Text>
  </View>)
}

function InlineView(props) {
  return (<View style={{margin: 10}} >
    <Text style={{ textAlign: props.textAlign, backgroundColor: 'cyan' }}>
      Parent text
      <View style={{width: 50, height: 50, backgroundColor: 'red'}} />
      <Text style={{ fontWeight: 'bold' }}>Child text</Text>
      {props.long && <Text style={{ fontWeight: 'bold' }}>aaaa a aaaa aaaaaa aaa a a a aaaaa sdsds dsdSAD asd ASDasd ASDas</Text>}
    </Text>
  </View>)
}

const TextView = InlineView;

export default function Test() {
  return (
    <SafeAreaView style={{ flex: 1 }}>
      <ScrollView style={{ flex: 1 }}>
        <Text style={{textAlign: 'center', fontSize: 20}}>BoringLayout</Text>
        <TextView textAlign="left" />
        <TextView textAlign="center" />
        <TextView textAlign="right" />

        <Text style={{textAlign: 'center', fontSize: 20}}>StaticLayout</Text>
        <TextView textAlign="left" long />
        <TextView textAlign="center" long />
        <TextView textAlign="right" long />
      </ScrollView>
    </SafeAreaView>
  );
}

iOS

old arch latin	old arch arabic	new arch latin	new arch arabic

Android old arch

latin ltr	arabic ltr	latin rtl	arabic rtl

Android new arch

latin ltr	arabic ltr	latin rtl	arabic rtl

The potential problem is that text alignment is decided based on the script, so forcing the RTL layout doesn't change anything.

[NickGerleman]

The potential problem is that text alignment is decided based on the script, so forcing the RTL layout doesn't change anything.

You can call the TextDirectionHeuristic isRTL function on the CharSequence to see which it resolves the paragraph to.

To my understanding, the way this works, is that the text direction set influences paragraph text direction, but strongly LTR or RTL subspans can form different directions (and... bidirectional text is complicated). So, we can use the heuristic to see how it will be oriented, and orient based on that.

Note that the CSS writing mode spec explicitly says browsers will prefer inherited direction to the script-based algorithm for determining paragraph-level writing direction (base direction). https://www.w3.org/TR/css-writing-modes-3/#bidi-para-direction I think this is what is being passed as part of the AttributedString being checked before.

Doing that actually makes things simpler, since we just set the paragraph layout direction to RTL or LTR explicitly, and don't need to check the heuristic to know paragraph alignment. Scripts inside may go in different directions though, which is I think the right behavior. But I kinda think that should probably be a separate change, since it effects rendering of LTR scripts in LTR more generally (even if probably the right behavior).

[j-piasecki]

I updated it to set text direction on the layout based on the heuristic, and then changed the logic around alignment and gravity since Android seems to be matching Gravity.START to the edge based on the script.

Not sure if that's what you meant, but this is the result

latin ltr	arabic ltr	latin rtl	arabic rtl

[facebook-github-bot]

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

[NickGerleman]

I updated it to set text direction on the layout based on the heuristic, and then changed the logic around alignment and gravity since Android seems to be matching Gravity.START to the edge based on the script.

Not sure if that's what you meant, but this is the result

It's pretty close to what I had in mind, and looks like it behaves consistently everywhere. Let's ship it! Thank you for all the iteration here.

[facebook-github-bot]

@cortinico merged this pull request in facebook/react-native@1f087995608fd016a8a3dd84c0ca88a9239d96b9.

[github-actions[bot]]

This pull request was successfully merged by @j-piasecki in 1f087995608fd016a8a3dd84c0ca88a9239d96b9.

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