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

Add a workaround for text being measured incorrectly when ending with an empty line #42331

Closed
wants to merge 1 commit into from

Conversation

j-piasecki
Copy link
Collaborator

Summary:

It looks like StaticLayout used for measuring multiline text on the new architecture returns wrong metrics for the last line if it's empty, however, if it contains anything, then the returned values are correct.

Similar approach with measuring I was already used in ReactEditText:

// If we don't have text, make sure we have *something* to measure.
// Hint has the same dimensions - the only thing that's different is background or foreground
// color
if (!haveText) {
if (getHint() != null && getHint().length() > 0) {
sb.append(getHint());
} else {
// Measure something so we have correct height, even if there's no string.
sb.append("I");
}
}

Changelog:

[ANDROID] [FIXED] - Fixed text being measured incorrectly when ending with an empty line on the new architecture

Test Plan:

I tested the change on this simple code on React Native 0.73.2 (with the new arch enabled)
import React from 'react';
import {useState} from 'react';
import {SafeAreaView, TextInput} from 'react-native';

export default function App() {
  const [text, setText] = useState('ABC\nDEF\nGHI\n');

  return (
    <SafeAreaView style={{flex: 1}}>
      <TextInput
        value={text}
        onChangeText={setText}
        style={{borderWidth: 1, margin: 20, padding: 30}}
        multiline
      />
    </SafeAreaView>
  );
}

Before:

Screen.Recording.2024-01-17.at.13.10.09.mov

After:

Screen.Recording.2024-01-17.at.13.07.46.mov

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Software Mansion Partner: Software Mansion Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 17, 2024
@sammy-SC sammy-SC requested a review from mdvacca January 17, 2024 12:55
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,798,999 +7
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,190,313 -5
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 6c8dfc8
Branch: main

Copy link

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.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jul 16, 2024
@j-piasecki
Copy link
Collaborator Author

Not stale

@react-native-bot react-native-bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Jul 19, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in bd32392.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 21, 2024
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @j-piasecki in bd32392

When will my fix make it into a release? | How to file a pick request?

@NickGerleman
Copy link
Contributor

NickGerleman commented Sep 24, 2024

FYI that #46613 replaces this logic with a proper fix (we were not correctly propagating settings to TextPaint used for layout)

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. Merged This PR has been merged. p: Software Mansion Partner: Software Mansion Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants