Skip to content
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

Avoid setting a lineHeight lower of the fontSize lineHeight with Text and TextInput #44614

Closed
wants to merge 3 commits into from

Conversation

fabOnReact
Copy link
Contributor

@fabOnReact fabOnReact commented May 20, 2024

Summary:

Avoids setting a lineHeight lower of the fontSize lineHeight with Text and TextInput.
The check was previously introduced on the Paper Renderer with PR #37465.

fixes Expensify/App#14445 fixes #29507
related #38359 and Expensify/react-native-live-markdown#350

Changelog:

[IOS] [FIXED] - Avoid setting a lineHeight lower of the fontSize lineHeight with Text and TextInput

Test Plan:

CLICK TO OPEN SOURCE CODE

/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @format
 * @flow
 */

import type {RNTesterModuleInfo} from './types/RNTesterTypes';

import * as React from 'react';
import {StyleSheet, Text, TextInput, View} from 'react-native';

// RNTester App currently uses in memory storage for storing navigation state

const RNTesterApp = ({
  testList,
}: {
  testList?: {
    components?: Array<RNTesterModuleInfo>,
    apis?: Array<RNTesterModuleInfo>,
  },
}): React.Node => {
  return (
    <View style={styles.container}>
      <Text>This is some Text</Text>
      <Text style={styles.text}>
        <Text>jjjj</Text>
        {'\n'}
        <Text>💀</Text>
      </Text>
      <Text>This is some TextInput</Text>
      <TextInput multiline style={styles.input} />
    </View>
  );
};

export default RNTesterApp;

const styles = StyleSheet.create({
  container: {
    marginTop: 250,
  },
  text: {
    lineHeight: 14,
    borderWidth: 1,
  },
  input: {
    lineHeight: 14,
    borderWidth: 1,
  },
});

Before After

Expensify

Before After

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 20, 2024
@fabOnReact fabOnReact changed the title Add check line height Avoid setting a lineHeight lower than the fontSize lineHeight on Text and TextInput May 20, 2024
@fabOnReact fabOnReact changed the title Avoid setting a lineHeight lower than the fontSize lineHeight on Text and TextInput Avoid setting a lineHeight lower of the fontSize lineHeight with Text and TextInput May 20, 2024
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,542,749 -4
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,912,802 -15
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 93c079b
Branch: main

@fabOnReact
Copy link
Contributor Author

fabOnReact commented May 21, 2024

RN-Tester - lineHeight examples

There is a difference in the RNTester test results.

  • I believe this PR introduces the correct behavior
  • The changes were previously merged and approved with PR #37465
  • PR 37465 was reverted for the slight difference in the text baseline, but no issues were reported with the changes in the lineHeight of the text/textinput.
Before After
Before After

@facebook-github-bot
Copy link
Contributor

@sammy-SC has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

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 react-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 2, 2024
@react-native-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
4 participants