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

Exclude trailing whitespace from newline character on measuring text line width #37790

Closed
wants to merge 4 commits into from
Closed

Exclude trailing whitespace from newline character on measuring text line width #37790

wants to merge 4 commits into from

Conversation

bernhardoj
Copy link
Contributor

Summary:

Multiline text in Android shows some extra space. It's easily noticeable when you set the text alignSelf to flex-start. This is because we are using layout.getLineWidth which will include trailing whitespace.
image

Based on Android doc, getLineMax exclude trailing whitespace.
image

Changelog:

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

Test Plan:

After applying the changes:
image

Code snippet:

<Text style={{backgroundColor: 'red', alignSelf: 'flex-start', color: 'white'}}>
    1{'\n'}
</Text>

@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 Jun 9, 2023
@bernhardoj
Copy link
Contributor Author

@NickGerleman here is the new PR.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,740,875 -6,901
android hermes armeabi-v7a 8,052,196 -8,184
android hermes x86 9,231,388 -7,999
android hermes x86_64 9,082,752 -6,437
android jsc arm64-v8a 9,303,774 -6,558
android jsc armeabi-v7a 8,492,427 -7,843
android jsc x86 9,365,161 -7,673
android jsc x86_64 9,620,620 -6,111

Base commit: 5cc8cee
Branch: main

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 12, 2023
@facebook-github-bot
Copy link
Contributor

@NickGerleman merged this pull request in 83d7a48.

kelset pushed a commit that referenced this pull request Jun 29, 2023
Summary:
Multiline text in Android shows some extra space. It's easily noticeable when you set the text `alignSelf` to `flex-start`. This is because we are using `layout.getLineWidth` which will include trailing whitespace.
<img width="300" alt="image" src="https://github.com/facebook/react-native/assets/50919443/8939092b-caef-4ad8-9b34-2ccef5d20968">

Based on Android doc, `getLineMax` exclude trailing whitespace.
<img width="625" alt="image" src="https://github.com/facebook/react-native/assets/50919443/0b32e842-5fab-4fc7-8fd9-299877b9c553">

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

Pull Request resolved: #37790

Test Plan:
After applying the changes:
<img width="300" alt="image" src="https://github.com/facebook/react-native/assets/50919443/bfbf52b0-7e7d-4c89-8958-6af38d8bc1c7">

Code snippet:
```
<Text style={{backgroundColor: 'red', alignSelf: 'flex-start', color: 'white'}}>
    1{'\n'}
</Text>
```

Reviewed By: cortinico

Differential Revision: D46586516

Pulled By: NickGerleman

fbshipit-source-id: 3ea9c150ad92082f9b4d1da453ba0ef04b09ce51
Kudo pushed a commit to expo/react-native that referenced this pull request Jul 3, 2023
Summary:
Multiline text in Android shows some extra space. It's easily noticeable when you set the text `alignSelf` to `flex-start`. This is because we are using `layout.getLineWidth` which will include trailing whitespace.
<img width="300" alt="image" src="https://github.com/facebook/react-native/assets/50919443/8939092b-caef-4ad8-9b34-2ccef5d20968">

Based on Android doc, `getLineMax` exclude trailing whitespace.
<img width="625" alt="image" src="https://github.com/facebook/react-native/assets/50919443/0b32e842-5fab-4fc7-8fd9-299877b9c553">

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - Exclude trailing whitespace from newline character on measuring text line width

Pull Request resolved: facebook#37790

Test Plan:
After applying the changes:
<img width="300" alt="image" src="https://github.com/facebook/react-native/assets/50919443/bfbf52b0-7e7d-4c89-8958-6af38d8bc1c7">

Code snippet:
```
<Text style={{backgroundColor: 'red', alignSelf: 'flex-start', color: 'white'}}>
    1{'\n'}
</Text>
```

Reviewed By: cortinico

Differential Revision: D46586516

Pulled By: NickGerleman

fbshipit-source-id: 3ea9c150ad92082f9b4d1da453ba0ef04b09ce51
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants