-
Notifications
You must be signed in to change notification settings - Fork 24.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LineHeight is working weird on specific fonts (Android Only) #29232
Comments
source code here. |
No. It doesn't reproduce only on Galaxy S9+. It happens on emulator, too. I tested on Nexus 5 API 28, Nexus 5 API 29 Emulator. and I changed System language(English(United States) to 한국어(대한민국)) |
Same issue here, even it was fixed according to #7546 (comment) , I am using react-native 0.63.2, line height of the first line of the ttf font I am using differs from all the other liens. And it is only on Android. I tested on Pixel 2 API R. |
The same question,Have you solved it |
I believe @fabriziobertoglio1987 is working on a PR similar to this? Can you confirm and link the PR/issues? |
Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions. |
I am also running into this issue. Any solutions? |
Reproducible example with the font |
Unfortunately the issue still persists for me in Starting Point
Strange workaroundAdd an empty nested
SummaryI created a component that does the job:
|
@emin93 Thanks for the workaround! I noticed that It looks like literally any non-zero difference to the parent line height also fixes it, e.g. |
@markdalgleish Ah that's good to know, thanks! |
Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions. |
Not stale. |
Thanks @emin93 , it's working with your suggested changes. |
The issue still persists for me in 0.64.3. However, the strange workaround from @emin93 works. |
Reporting back after a while - this issue is still present. While the provided workaround might solve the issue with the first two lines, it unfortunately is not a "one size fits all" solution. When applied on single line texts it causes the vertical alignment to be off. |
I'm also seeing this issue when trying to correctly align the placeholder text inside the Android-specific |
Can confirm, this is still a bug (using GrotaRounded custom font for instance) on Android. |
I believe this is caused by There is some complex logic that tries to center the font in an appropriate way if the line size is below a certain amount. It can happen if the font has unusual metadata, like the capheight being very tall. I don't think this happens on IOS or web, at least I don't see it happening in practice nor can I find any code that does it. Adding magic offsets to line height will only make it work for that specific font, at that specific size and line height. |
This file has moved to https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLineHeightSpan.java
|
This issue still persists. I tried the workaround suggested by @emin93, but it didn't work. To fix it, I added a marginTop specifically for Android. However, it feels somewhat dirty.
|
I'm facing this problem too (version: react-native 0.70.9) |
For most of the fonts you should apply ascender/descender metrics to make it render correctly at first and last lines. Though there are broken fonts (made by font designers) where you should add workarounds 😄 based on ascender/descender data you can dynamically calculate your padding top Though maybe someone has better approach, for me nothing worked except doing all calculations on my own |
Thank you so much! I ran into this issue with custom font after switching to target SDK 33 and this app was immensely useful in showing what to fix in the font to make it behave 'normally'. I ended up rebuilding the font with proper location of glyphs. |
The solution from @emin93 is nice! But I found that solution can't fix the paragragh problem. The lines still stick together after a newline. If you are facing same issue like me, I have another solution that is based on @emin93's solution. export const Text = ({ style, children, ...rest }) => (
<RNText style={style} {...rest}>
{children}
{Platform.OS === 'android' && <RNText style={{ lineHeight: style.lineHeight + 0.001 }} />}
</RNText>
); const ParagraphText = ({paragraph}): JSX.Element => {
const textStrings = useMemo(() => paragraph?.split(/\r?\n/), [paragraph]);
const textStringsComponents = useMemo(
() =>
textStrings?.map((text, index) => (
<>
<Text>
{text}
</Text>
{!!textStrings && index !== textStrings.length - 1 && (
<Text>
{'\n'}
</Text>
)}
</>
)),
[color, content, textStrings, variant, weight]
);
const transformedChildren = useMemo(
() => (Platform.OS === 'android' ? textStringsComponents : paragraph),
[paragraph, textStringsComponents]
);
return (
<Text>
{transformedChildren}
</Text>
); |
Do you still experience this issue? If yes, I will publish the fix in the react-native-improved package https://github.com/fabriziobertoglio1987/react-native-improved. Thanks a lot |
This is just crazy that this issue still a problem. @fabOnReact Can it be fixed without patches?) |
My workaround. Is not breaking ref, works as drop in replacement pnpm i react-fast-hoc import React from 'react';
import { transformProps } from 'react-fast-hoc';
import * as RN from 'react-native';
// Fix of android lineHeight problems https://github.com/facebook/react-native/issues/29232#issuecomment-1721080348
if (RN.Platform.OS === 'android') {
// Clone text since we modify it
const OriginalText = Object.assign({}, RN.Text);
Object.assign(
RN.Text,
transformProps(RN.Text, (props) => {
const style = RN.StyleSheet.flatten(props.style);
if (!style?.lineHeight) {
return props;
}
const children = [
...React.Children.toArray(props.children),
React.createElement(OriginalText, {
key: '__text_fix',
style: {
lineHeight: style.lineHeight + 0.0001,
},
}),
];
props.children = children;
return props;
}),
);
} |
@XantreGodlike I believe this should be fixed in rn-core. If the developer needs:
We have 1000+ issues, the dev would end up not being productive and better develop his app with the Android Sdk or iOS SDK. My patch does not fix 1 issue. I plan to fix 1000-2000 issues. I don't think it is even worth documenting this. distributing 1 patch with react-native-improved and fixing the issue at the source is the solution. |
I will text this solution and implement it in rn-core. If you are interested, you can send PR to https://github.com/fabriziobertoglio1987/react-native-improved. Thanks |
I am sceptical about patching, it can be complicated and unsafe. But maybe you're doing it in the correct way. |
The issue says that it fixes this bug on new arch |
The issue does not reproduce when you don't set the lineHeight? because every text has by default a lineHeight, which is calculated on their font ascent and descent. React Native often does not work well with custom font, because custom font has different font ascent and descent. As react native over-rides onMeasure in ReactShadowTextView, it calculates the font lineHeight internally based on their font ascent and descent to calculate the Text height. but maybe this does not work with custom fonts. The PR from MD Vacca only sets the lineHeight, but when no lineHeight is set, do we really take in account that the Text onMeasure function correctly calculates height based on the custom font Text ascent and descent? |
Maybe the issue is not fixed on nested text? The current implementation maybe does not support nested text. I believe the change now sets 1 lineHeight for the entire Text, so in this case, they would all have lineHeight 20. Previously you could have 2 text with different lineHeight. for ex. <Text style={{lineHeight: 20}}>
Line height 20
<Text style={{lineHeight: 30}}>Line Height 30</Text>
</Text> I worked on something similar in the past and I can easily implement it in text. |
I think the only solution with this workaround is to use lineHeight explicitly. Because we cannot calc lineHeight from font size |
This issue 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. |
Bump |
Im also facing this issue |
Please provide all the information requested. Issues that do not follow this format are likely to stall.
Description
Multiline texts don't have same line height on specific fonts. (NotoSansCJK= Source Hans Sans, NotoSerifCJK=Source Hans Serif)
It happens on first line.
React Native version:
Steps To Reproduce
Expected Results
Every lines have same line height.
like this:
Snack, code example, screenshot, or link to a repository:
Android Native(No React Native) is fine. This font is NotoSansKR-Regular :
iOS is fine:
Text Style:
The text was updated successfully, but these errors were encountered: