Implement baseline alignment function on the new architecture
Summary:
On the new architecture, the setup that would allow Yoga to read the baseline of a node was missing. This PR adds it:
- adds new ShadowNode trait -
BaselineYogaNodethat marks a node as having a custom baseline function - adds
yogaNodeBaselineCallbackConnectorthat's responsible for allowing Yoga to call baseline function on the node - changes signatures of
lastBaselineandfirstBaselineto acceptLayoutContextas the first argument, which is necessary to build an attributed string - adds implementation of
lastBaselinethat's invoked byyogaNodeBaselineCallbackConnector - adds methods for calculating the last baseline in platform-specific
TextLayoutManagers, using the same approach on both Android and iOS (this differs from the old architecture where calculations were different)
Changelog:
[GENERAL] [FIXED] - Fixed alignItems: 'baseline' not working correctly on the new architecture
Test Plan:
Tested on the relevant part of RNTester:
Android
| Old arch | New arch before | New arch after |
|---|---|---|
iOS
| Old arch | New arch before | New arch after |
|---|---|---|
/rebase
Autorebase failed as we're doing GHA work at the moment. @j-piasecki if you could rebase the CI should be green
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 20,383,026 | -902,098 |
| android | hermes | armeabi-v7a | n/a | -- |
| android | hermes | x86 | n/a | -- |
| android | hermes | x86_64 | n/a | -- |
| android | jsc | arm64-v8a | 23,579,561 | -902,438 |
| android | jsc | armeabi-v7a | n/a | -- |
| android | jsc | x86 | n/a | -- |
| android | jsc | x86_64 | n/a | -- |
Base commit: 8c8c77b974ce3d4c3036361e56276a6ff2b02208 Branch: main
The way Paper is handling multi-line baseline selection in flex containers seems inconsistent with browsers/flexbox spec. In browser, first-line is chosen. https://jsfiddle.net/jq49r8hb/
I suspect we could get away with changing the behavior in RN to align with the spec and browsers. But I'm curious what @cortinico thinks about this.
I've also updated the relevant Android part of RNTester to match what's on iOS:
@j-piasecki I think this is looking good. Have we tested/do we have validation for multi-line?
I still kinda think we should switch Paper behavior for web behavior though, now that we have the chance to 😅.
I've updated the logic to calculate the baseline based on the first line. There's a small issue with padding and TextInputs - layoutMetrics are set too late, so I took them directly from yogaNode, I'm not sure if that's the correct approach.
Here's the result:
And the code
import { SafeAreaView, View, Text, TextInput } from 'react-native';
export default function App() {
return (
<SafeAreaView style={{flex: 1, flexDirection: 'row', alignItems: 'baseline'}}>
<View style={{width: 50, height: 50, backgroundColor: 'red'}} />
<View style={{width: 150, backgroundColor: 'red'}}>
<Text style={{fontSize: 15}}>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</Text>
</View>
<View style={{width: 150, backgroundColor: 'red'}}>
<Text style={{fontSize: 11}}>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</Text>
</View>
<TextInput style={{padding: 0, paddingTop: 10, backgroundColor: 'red'}} value='Lorem ipsum' />
</SafeAreaView>
)
}
edit:
It looks like it broke the existing example:
Using getContentWithMeasuredAttachments solved that problem on Android, it's still affecting iOS. There's also an issue of how views nested in text are processed - every view is effectively treated as having its baseline at the bottom, which results in this cascade when nesting:
I got it working okay-ish on Android by doing some wrong assumptions and hard-coding a few things:
I believe that it would be necessary to have every attachment also have a baseline property to fix that properly.
I needed to update how measureLines works on iOS. Previously it was using data about the font used for the first character. This isn't correct when there are multiple spans styled differently on the same line, or when an inline view is present in the text.
I've changed it to use the y-position of the first character, which I believe is the baseline. Then I set the descender to lineRect.height - baseline, which should be equal to the descend if I understand the methods correctly.
This fixed the interleaving View and Text example in RNTester and I didn't see anything obviously broken either.
The one problem left is that the baseline is "cascading" when nesting multiple views and texts together
import React from "react";
import { Text, View, SafeAreaView } from "react-native";
export default class TextBaseLineLayoutExample extends React.Component {
render() {
const texts = [];
for (let i = 9; i >= 0; i--) {
texts.push(
<Text key={i} style={{fontSize: 8 + i * 5, backgroundColor: '#eee'}}>
{i}
</Text>,
);
}
const marker = (
<View style={{width: 20, height: 20, backgroundColor: 'gray'}} />
);
return (
<SafeAreaView>
<View style={{flexDirection: 'row', alignItems: 'baseline', backgroundColor: 'red'}}>
{marker}
<Text style={{fontSize: 15}} selectable={true}>
Text1
<View
style={{
flexDirection: 'row',
alignItems: 'baseline',
backgroundColor: 'cyan',
}}>
<Text style={{fontSize: 20}}>Text2
{marker}
<View
style={{
flexDirection: 'row',
alignItems: 'baseline',
backgroundColor: 'yellow',
}}>
<Text style={{fontSize: 25}}>Text3
{marker}
<View
style={{
flexDirection: 'row',
alignItems: 'baseline',
backgroundColor: 'orange',
}}>
<Text style={{fontSize: 15}}>Text4
{marker}
</Text>
</View>
</Text>
</View>
</Text>
</View>
</Text>
<View style={{ width: 30, height: 30, backgroundColor: 'gray'}} />
</View>
</SafeAreaView>
);
}
}
As mentioned above, this is caused by every attachment being positioned on the baseline, so views containing text are effectively moved up by the descent. This one might be tricky to solve - I think it would need to align with the last baseline of the attached view.
And there's the matter of reading padding directly from the yogaNode in TextInputs.
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
I suspect we could get away with changing the behavior in RN to align with the spec and browsers. But I'm curious what @cortinico thinks about this.
Sorry I missed this ping @NickGerleman. As I suspect this is more of a niche feature in React Native, it makes sense to align to the web spec and pick the first line in multiline string for the sake of alingment.
As mentioned above, this is caused by every attachment being positioned on the baseline, so views containing text are effectively moved up by the descent. This one might be tricky to solve - I think it would need to align with the last baseline of the attached view.
I'm fine with this particular scenario not being properly handled as I have the feeling it might just over-complicate the measuring logic. It also makes sense that the views are placed at the baseline, resulting in the "cascading" effect you mentioned @j-piasecki.
I'd like to hear @NickGerleman opinion on this one, but I believe we can attempt to land this as it is 👍
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@j-piasecki this is failing to build internally with:
[...]\packages\react-native\ReactCommon\react\renderer\components\text\ParagraphShadowNode.cpp(188,30): error: no member named 'getLastBaseline' in 'facebook::react::TextLayoutManager'
return textLayoutManager_->getLastBaseline(
~~~~~~~~~~~~~~~~~~~~^
That's weird, it looks like it's pointing here: https://github.com/facebook/react-native/blob/08c8d8dfffe2240c8a02f07527f03a21cd9c753b/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp#L188-L192 but I already updated it. Moreover, there are no references to getLastBaseline at all in the codebase at the moment. Is it possible that it's caused by a stale cache?
Moreover, there are no references to
getLastBaselineat all in the codebase at the moment. Is it possible that it's caused by a stale cache?
@j-piasecki my bad I copied the error message form a old revision.
The error is still:
[...]\packages\react-native\ReactCommon\react\renderer\components\text\ParagraphShadowNode.cpp(188,30): error: no member named 'baseline' in 'facebook::react::TextLayoutManager'
return textLayoutManager_->baseline(
~~~~~~~~~~~~~~~~~~~~^
The error is still:
[...]\packages\react-native\ReactCommon\react\renderer\components\text\ParagraphShadowNode.cpp(188,30): error: no member named 'baseline' in 'facebook::react::TextLayoutManager' return textLayoutManager_->baseline( ~~~~~~~~~~~~~~~~~~~~^
I don't see why it would fail there. All TextLayoutManager headers (for ios, android, and cxx) have the baseline method declared in the public scope and all of them have the implementation.
One thing I noticed is that I was using float type instead of Float. I think the error message would be different if that was the cause, but I changed it nonetheless.
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
I don't see why it would fail there. All
TextLayoutManagerheaders (for ios, android, and cxx) have thebaselinemethod declared in the public scope and all of them have the implementation.
Ok so the issue is that we do have other TextLayoutManager in our codebase that are not exposed to GitHub. I'll take care of providing a baseline function there and returning 0 so that should unblock us 👍
The one problem left is that the baseline is "cascading" when nesting multiple views and texts together
As mentioned above, this is caused by every attachment being positioned on the baseline, so views containing text are effectively moved up by the descent. This one might be tricky to solve - I think it would need to align with the last baseline of the attached view.
@j-piasecki Sorry, I missed reading this in the last iteration.
In an inline/text context, items are by default aligned so that their baselines mare matched. I think the "cascading" problem is an artifact of always treating the baseline of the inline view as its bottom.
A flexbox container's baseline is set based on its children. So, the baseline of that blue "View", should really match the baseline of the text inside of it.
- https://www.w3.org/TR/css-flexbox-1/#flex-baselines
- https://www.w3.org/TR/css-align-3/#baseline-export
Yoga does implement this algorithm, for determining the baseline of a child when using align-items: baseline, but React Native's text layout doesn't do anything similar for positioning attachments, so nothing will be seen for that. https://github.com/facebook/yoga/blob/5009f5c1accb3dde14e1e18930df70c18c70dc75/yoga/algorithm/Baseline.cpp#L17
Not something for this PR, but would definitely be welcome as followup if you ever feel like doing more around this area.
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
In an inline/text context, items are by default aligned so that their baselines mare matched. I think the "cascading" problem is an artifact of always treating the baseline of the inline view as its bottom.
That's exactly what's happening.
Yoga does implement this algorithm, for determining the baseline of a child when using align-items: baseline, but React Native's text layout doesn't do anything similar for positioning attachments, so nothing will be seen for that. https://github.com/facebook/yoga/blob/5009f5c1accb3dde14e1e18930df70c18c70dc75/yoga/algorithm/Baseline.cpp#L17
Not something for this PR, but would definitely be welcome as followup if you ever feel like doing more around this area.
Sure, I would be up for it, though I don't think it would be contained to the React Native's part of text layout. Every inline view is passed as an attachment which is essentially a rectangle without any information about it's children or baseline.
Moreover, it looks like inline views are aligned on the last baseline instead of the first one: https://jsfiddle.net/tgkdr63s/
Moreover, it looks like inline views are aligned on the last baseline instead of the first one: https://jsfiddle.net/tgkdr63s/
If you change inline-block to inline-flex this changes (baseline determination is different for flex containers vs block containers) See https://github.com/facebook/react-native/pull/45102#discussion_r1651258302
From https://jsfiddle.net/jq49r8hb/ I think the first baseline behavior will also still happen when aligning inline-block element in flex container too. I think the last baseline behavior may be specific to inline-block inside block context?
@cortinico merged this pull request in facebook/react-native@2932c0f71f1882607d9e579e5c09db28e131a4c9.
This pull request was successfully merged by @j-piasecki in 2932c0f71f1882607d9e579e5c09db28e131a4c9.
When will my fix make it into a release? | How to file a pick request?
why is something like this not possible still?
https://jsfiddle.net/yhc43e9w/1/
Aligning the box in the center.