facebook / react-native

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

TextInput lineHeight handling is incorrect #45268

Closed yard closed 2 days ago

yard commented 2 days ago

Description

Due to inconsistencies of NSLayoutManager configuration across RN, <Text /> and <TextInput /> components do handle line height in a slightly different way. More specifically, the resulting line heights of a <Text /> component seem to be correct, while <TextInput />s one is off by some fraction.

The offending places seem to be https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.mm#L52 and https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.mm#L274 – notice both are missing setting usesFontLeading to NO (contrary, it gets set in other places, e.g. https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Text/Text/RCTTextShadowView.mm#L223).

The reproduction is really trivial – please see the snack attached. Notice how the two columns of texts are perfectly aligned on the web and is not really aligned on iOS, although both elements share the same style attribute.

The same issue happens on Android, but it is probably caused by something different, as the amount by which the texts are off is smaller.

Steps to reproduce

  1. Set custom line height to a <TextInput /> component
  2. Measure the actual line height (using a screenshot)
  3. Notice the difference

React Native Version

0.73.6

Affected Platforms

Runtime - iOS

Output of npx react-native info

System:
  OS: macOS 14.3
  CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Memory: 47.86 MB / 32.00 GB
  Shell:
    version: 3.2.57
    path: /bin/bash
Binaries:
  Node:
    version: 20.11.1
    path: ~/.nvm/versions/node/v20.11.1/bin/node
  Yarn:
    version: 4.1.1
    path: /usr/local/bin/yarn
  npm:
    version: 10.2.4
    path: ~/.nvm/versions/node/v20.11.1/bin/npm
  Watchman:
    version: 2024.05.06.00
    path: /usr/local/bin/watchman
Managers:
  CocoaPods: Not Found
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2023.2 AI-232.10300.40.2321.11567975
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 13.0.1
    path: /usr/bin/javac
  Ruby:
    version: 3.3.0
    path: /Users/anton/.rvm/rubies/ruby-3.3.0/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.6
    wanted: 0.73.6
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

N/A

Reproducer

https://snack.expo.dev/YI-C2DqB4yUmxW9Eb0cWW

Screenshots and Videos

Screenshot 2024-07-03 at 19 12 15
github-actions[bot] commented 2 days ago
:warning: Newer Version of React Native is Available!
:information_source: You are on a supported minor version, but it looks like there's a newer patch available - 0.73.8. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.
cortinico commented 2 days ago

Basically a duplicate of https://github.com/facebook/react-native/issues/39145

yard commented 2 days ago

Basically a duplicate of #39145

Oh yeah, very much looks like it is. I can prepare a PR disabling the font leading everywhere – we currently have a patch applied to our app and it seems to resolve the issue.

cortinico commented 2 days ago

Closing this to keep the conversation focused on the same place. Let's move over to #39145 ok? 👍