facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.3k stars 24.35k forks source link

Line height is distributed unevenly when lineHeight <= fontSize #29507

Open andymatuschak opened 4 years ago

andymatuschak commented 4 years ago

Description

When lineHeight is less than the fontSize, Text shrinks the line's bounding box only by removing space from above the text, rather than distributing the space evenly above and below, as is done with extra line height. Actually, the issue manifests when lineHeight is less than fontSize plus some small amount (perhaps the font bounding box height?)—e.g. <= 45px for 40px Helvetica.

This leads to clipped glyphs and produces differences in text rendering between react-native-web and react-native (https://github.com/necolas/react-native-web/issues/1687). After some discussion with @necolas, my inclination is that RNW's behavior is the more reasonable one, so I thought I'd open an issue here.

This issue reproduces across a variety of fonts.

image

Unfortunately,I worry that fixing this issue will create subtle and surprising source compatibility issues for existing clients.

React Native version:

System:
    OS: macOS 10.15.5
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 337.02 MB / 32.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 10.21.0 - ~/.nvm/versions/node/v10.21.0/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v10.21.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.9.3 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 13.6, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
    Android SDK: Not Found
  IDEs:
    Android Studio: 3.5 AI-191.8026.42.35.5900203
    Xcode: 11.6/11E708 - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.2 - /usr/bin/javac
    Python: 2.7.16 - /usr/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: Not Found
    react-native: 0.63.2 => 0.63.2 
  npmGlobalPackages:
    *react-native*: Not Found

Steps To Reproduce

Run example app here: https://snack.expo.io/W51F2OAqD

Expected Results

The extra line spacing should be removed evenly from both above and below the type.

Snack, code example, screenshot, or link to a repository:

https://snack.expo.io/W51F2OAqD

andymatuschak commented 4 years ago

A workaround for readers who stumble on this issue: if you'd like to use smaller line heights without glyph clipping, you'll need to add top padding to the Text element, which you must then compensate for by shifting the element up (e.g. via top plus relative positioning, or a negative marginTop).

ravirajn22 commented 4 years ago

Does adding this prop help textAlignVertical='center'?

andymatuschak commented 4 years ago

@ravirajn22: Nope, that doesn't change the output, neither on Android nor on iOS.

stale[bot] commented 3 years ago

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.

andymatuschak commented 3 years ago

Nope, the issue has not been fixed; it still requires attention. I don't seem to have permission to add it to the backlog.

Dinkelborg commented 3 years ago

I can confirm that this is still an issue

Thanks for the tip with the workaround @andymatuschak

SpookyUmi commented 3 years ago

Yep, still an issue

navaneeth7067 commented 3 years ago

Still an issue

kbrandwijk commented 3 years ago

Agreed, still an issue, bot.

aschultz commented 2 years ago

On Windows, XAML has LineStackingStrategy and TextLineBounds for fine-tuning the text positioning. On web, there's a proposal for "leading-trim".

Do iOS or Android have anything similar that could be used or exposed?

v0vchansky commented 2 years ago

Still an issue

sungsong88 commented 1 year ago

Same issue here. Basically, any characters that were not part of the font package were causing this issue for me.

elencho commented 1 year ago

https://github.com/facebook/react-native/issues/7687

lexi-stein commented 1 year ago

Definitely still an issue and the workaround is not great if you ever have Text within Text, they no longer align when they both have some kind of top offset

HaakonSvane commented 1 year ago

Would be great to bring some attention to this issue. The workaround mentioned in #7687 is hacky and will not work properly with special capital characters (e.g. Å, Á, È) unless you manually tweak paddings to "just about right".

Mr-Fullstack commented 1 year ago

try just verticalAlign:"bottom" or verticalAlign:"bottom" lineHeight:1, height: 24 => here you define the height you want for your text

jeonghoonkim commented 11 months ago

Still an issue

fabOnReact commented 10 months ago

it is caused by the baseline of the Text. The Text and TextInput implementations are similar, you can find relevant explanations of how baseline is set on react native in this prs https://github.com/facebook/react-native/pull/38359 https://github.com/facebook/react-native/pull/37465. I'll follow up

oliviercperrier commented 9 months ago

Still an issue

OlacodesLA commented 5 months ago

Still an issue

keithpickering commented 5 months ago

Hey @fabOnReact, it seems like you have the best understanding of this bug. Certain fonts have different ascents/descents and iOS doesn't adjust the baseline based on this difference. This presents unnoticeably on many fonts, but for some other fonts the difference can be drastic.

The fix you linked to involves the TextInput component. Do you know if there's a similar fix for Text? I'm seeing the incorrect line height issue on both.

lsarni commented 4 months ago

It is also a problem when using special characters even if the lineHeight > fontSize as they get cut off. The hacky padding workaround works but is not ideal https://github.com/facebook/react-native/issues/29507#issuecomment-1529425022

Here is lineHeight: 37 and fontSize: 32 Android image Web image

https://snack.expo.dev/@lsarni/petrified-kiwi

ManuLpz4 commented 3 months ago

This is crazy, just to follow up it

mellyeliu commented 1 month ago

fyi: we've added a fix for this here: https://github.com/facebook/react-native/pull/46362, although we are still testing before rollout

cortinico commented 1 month ago

Closing as the PR mentioned by @mellyeliu has been merged and will be shipped with React Native 0.76

DimitarNestorov commented 1 month ago

@mellyeliu @cortinico The issue seems to affect both Android and iOS and the PR only introduces a solution for Android. Can we reopen until iOS is also addressed?

mellyeliu commented 1 month ago

Reopening until iOS fix for tracking purposes

ArekChr commented 1 month ago

@mellyeliu I have created a PR with a fix for iOS. Please take a moment to review It. Your feedback would be greatly appreciated!