feat: Add Android Paper implementation of inset logical properties

[gabrieldonadel]

Summary:

This PR adds Paper support to inset logical properties on Android as requested on #34425. This implementation includes the addition of the following style properties

• inset, equivalent to top, bottom, right and left.
• insetBlock, equivalent to top and bottom.
• insetBlockEnd, equivalent to bottom.
• insetBlockStart, equivalent to top.
• insetInline, equivalent to right and left.
• insetInlineEnd, equivalent to right or left.
• insetInlineStart, equivalent to right or left.

One thing that still needs to be figured out is how to respect the same precedence as here when using Paper (cc @NickGerleman)

iOS changes are in a separate PR to facilitate code review #36241

Changelog:

[ANDROID] [ADDED] - Add Paper implementation of inset logical properties

Test Plan:

1. Open the RNTester app and navigate to the View page
2. Test the new style properties through the Insets section

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,605,045	+432
android	hermes	armeabi-v7a	7,918,955	+432
android	hermes	x86	9,090,917	+436
android	hermes	x86_64	8,946,005	+440
android	jsc	arm64-v8a	9,174,237	+404
android	jsc	armeabi-v7a	8,365,906	+413
android	jsc	x86	9,231,895	+407
android	jsc	x86_64	9,490,592	+411

Base commit: a6690bf
Branch: main

[NickGerleman]

Left a comment on the iOS PR around the precedence, but yeah, I think that is the main thing left here. Thank you for splitting these as well!

[gabrieldonadel]

Hey @NickGerleman, I'm glad to share that I managed to fix the precedence problem on Android as well. Fix this by using a very similar approach to #36241, adding a mPosition param to the LayoutShadowNode. Please let me know if this approach looks good to you.

[NickGerleman]

It looks like there is already a pattern for this in ReactShadowNodeImpl, e.g:

privatefinalfloat[] mPadding = newfloat[Spacing.ALL + 1];
privatefinalboolean[] mPaddingIsPercent = newboolean[Spacing.ALL + 1];

Could we put these in the same place, with the same implementation?

We should also change up Spacing.ALL. E.g. rename that to Spacing.ALL_EDGES, then make Spacing.ALL include everything (and use it where we support the extra values).

[NickGerleman]

I'm looking closer at the impl of this, and spacingType is translated directly to a Yoga Edge.

We already added some more different values to Spacing, but since these can only accept Yoga Edge parameters, is it possible to use a more constrained type? E.g. changing sinks which expect a valid Yoga enum to instead use YogaEdge?

[NickGerleman]

Dynamic is probably heavier than we need here. Could keep these as MutableYogaValue?

[gabrieldonadel]

Sorry for the delay on this, I'll try to take a look this week*!

[necolas]

What are the next steps to get this PR merged?

[NickGerleman]

For 0.72 we took the step of adding Fabric specific typings for this and the others.

[necolas]

Do we still need these in Paper? If time is limited, I think we're better off focusing on fleshing out support in Fabric than the legacy renderer.

[github-actions]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[gabrieldonadel]

Should we still implement these?

[NickGerleman]

Should we still implement these?

As of recent we've been wanting to avoid changes to Paper. Since inset properties are layout only, the Fabric impl we have for specifically that should work across platforms.

The gap we have for logical property new arch support is on borders. Solving that in the short term would like adding support in the Android View View Manager, along with the iOS View ComponentView, to the existing structure resolving edge values.

There's a longer term change, I am trying to allocate time to make, which would eventually move built-in view managers and component views to a "computed style", which would mean view managers and component views only have to deal with the four physical edges.

[react-native-bot]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[react-native-bot]

This PR was closed because it has been stalled for 7 days with no activity.