Apply padding to the text attachements

[j-piasecki]

Summary:

Paddings are not applied to inline views in text, this PR fixes that.

Closes #42099

Changelog:

[GENERAL] [FIXED] - Fixed padding not being applied to inline views in text

Test Plan:

A simple test case

      <Text style={{paddingHorizontal: 40, paddingVertical: 40, backgroundColor: 'green', width: 300 }}>
        <View style={{backgroundColor: 'red', width: 50, height: 50}} />
        foobar foobar
        <View style={{backgroundColor: 'red', width: 50, height: 50}} />
      </Text>

iOS before	iOS after	Android before	Android after

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,460,041	+14,284
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,833,515	+15,224
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: c96c893
Branch: main

[NickGerleman]

First things that come to mind:

1. Android and iOS will separately set frame in their own versions of TextLayoutManager. So, before making changes at this specific layer, it would be good to check if iOS does the same thing. Though I can see we are only passing the content frame (frame minus border and padding) as layoutConstraints when measuring string, so I would guess everything is relative to this frame.
2. If we are not offsetting by padding, we are also likely missing space taken by paragraph border as well (also part of the contentFrame insets)
3. We should be reading Fabric LayoutMetrics (see above in same function) instead of from Yoga Node directly. It has contentInsets set which is I think what we're looking for

[j-piasecki]

Android and iOS will separately set frame in their own versions of TextLayoutManager. So, before making changes at this specific layer, it would be good to check if iOS does the same thing. Though I can see we are only passing the content frame (frame minus border and padding) as layoutConstraints when measuring string, so I would guess everything is relative to this frame.

It seems like it, though the exact mechanism differs between Android and iOS. Both pass layoutConstraints to the relevant measure methods, which are then passed on to the platform-specific code.

On iOS, the text storage with the maximum width is created and used to measure text:

react-native/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm

Lines 48 to 54 in be06fd4

	CGSize maximumSize = CGSize{layoutConstraints.maximumSize.width, CGFLOAT_MAX};
	NSTextStorage *textStorage = [self _textStorageAndLayoutManagerWithAttributesString:attributedString
	paragraphAttributes:paragraphAttributes
	size:maximumSize];
	return [self _measureTextStorage:textStorage];

On Android, the min/max size is passed to the java measure method:

react-native/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp

Lines 138 to 149 in be06fd4

	auto size = yogaMeassureToSize(measure(
	fabricUIManager,
	rootTag,
	componentNameRef.get(),
	localDataMap.get(),
	propsMap.get(),
	nullptr,
	minWidth,
	maxWidth,
	minHeight,
	maxHeight,
	attachmentPositions));

and then the max width is used to calculate the text layout:

react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java

Lines 511 to 519 in be06fd4

	Layout layout =
	createLayout(
	text,
	boring,
	width,
	widthYogaMeasureMode,
	includeFontPadding,
	textBreakStrategy,
	hyphenationFrequency);

When it comes to drawing, iOS uses the content frame directly:

react-native/packages/react-native/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm

Line 134 in be06fd4

CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame());

On Android the insets are passed on to the native view as padding:

react-native/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp

Line 623 in be06fd4

if (!cppUpdatePaddingMountItems.empty()) {

react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java

Lines 259 to 261 in be06fd4

	public void setPadding(ReactTextView view, int left, int top, int right, int bottom) {
	view.setPadding(left, top, right, bottom);
	}

But in both cases, the measurement logic itself is unaware of the padding.

[j-piasecki]

If we are not offsetting by padding, we are also likely missing space taken by paragraph border as well (also part of the contentFrame insets)
We should be reading Fabric LayoutMetrics (see above in same function) instead of from Yoga Node directly. It has contentInsets set which is I think what we're looking for

Yes, and yes. Before I saw that the padding itself is not passed to the measurement I tried to access the padding in the measureContent where layoutMetrics are not yet initialized. Then I copied it without updating 🤦.

Using contentInsets also addresses the second point since it includes borders.

[j-piasecki]

@NickGerleman Friendly ping in case you missed the response.

[NickGerleman]

@j-piasecki yeah I think the only change needed here is just moving to content insets

[facebook-github-bot]

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

[facebook-github-bot]

@NickGerleman merged this pull request in 12aef32.

[github-actions]

This pull request was successfully merged by @j-piasecki in 12aef32.

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