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

[Android] Fix TextInput Cursor jumping to the right when the placeholder null #28995

Closed

Conversation

fabOnReact
Copy link
Contributor

@fabOnReact fabOnReact commented May 28, 2020

Summary

This issue fixes #28794 fixes #27658
Flow type ?Stringish allows to set the placeholder value to null. The null value causes the cursor to jump to the right in a TextInput. The fix replaces the placeholder null value with an empty string which avoid calling setHint(null) as causes the placeholder to jump to the right.

Changelog

[Android] [Fixed] - avoid calling setHint with a null parameter causing cursor to jump to the right

Test Plan

CLICK TO OPEN TESTS RESULTS (28 May 2020 20a9473)

More videos and information included in issue #28794
The below test cases are from the following repository

BEFORE
AFTER

Extensive testing on RNTester did not identify any regression.

AFTER

CLICK TO OPEN TESTS RESULTS (15 February 2021 20a9473)

BEFORE
2021-02-15.15-48-54.mp4
AFTER
2021-02-15.15-44-55.mp4

@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 28, 2020
@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels May 28, 2020
@analysis-bot
Copy link

analysis-bot commented May 28, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,891,047 -5,897
android hermes armeabi-v7a 8,394,773 +5,429
android hermes x86 9,380,311 -5,140
android hermes x86_64 9,323,030 -5,111
android jsc arm64-v8a 10,341,051 -289,994
android jsc armeabi-v7a 9,827,876 -278,630
android jsc x86 10,391,813 -289,236
android jsc x86_64 10,974,791 -289,203

Base commit: 7b09eb5

@analysis-bot
Copy link

analysis-bot commented May 28, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 7b09eb5

Copy link

@ShivamJoker ShivamJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this code and it works perfect please merge it

@devinjameson
Copy link

Why has this not been merged yet?

@safaiyeh
Copy link
Contributor

cc @JoshuaGross

@Elijen
Copy link

Elijen commented Sep 25, 2020

@hramos @rickhanlonii 🙏

@fabOnReact
Copy link
Contributor Author

@Haris-Shahid i'll test again, but it worked when I wrote this pr...

@Yandamuri
Copy link

when will this be merged?

@sosunny
Copy link

sosunny commented Jan 21, 2021

@Haris-Shahid i'll test again, but it worked when I wrote this pr...

Did you test it works by any chance? It didn't fix the issue for me either with RN v0.63.4 :/

@fabOnReact

This comment has been minimized.

@fabOnReact fabOnReact marked this pull request as draft February 5, 2021 09:59
@fabOnReact fabOnReact marked this pull request as ready for review February 23, 2021 15:02
@sreuter
Copy link

sreuter commented Feb 25, 2021

@fabriziobertoglio1987 I've noticed you just updated this PR recently. Any chance of this getting merged soon. This also just seems to happen when textAlign is set to center, correct?

@lunaleaps lunaleaps self-assigned this Aug 4, 2021
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@lunaleaps merged this pull request in 3560753.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 12, 2021
@ShivamJoker
Copy link

Wow finally after 2 years, folks at facebook merged this PR 😳🤦

@alichampion
Copy link

In which version of react-native it will work fine?

@lunaleaps
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 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. Platform: Android Android applications.
Projects
None yet