fix: select text on auto focus for TextInput#45004
fix: select text on auto focus for TextInput#45004kunalchavhan wants to merge 4 commits intofacebook:mainfrom kunalchavhan:fix/textinput-selecttext-autofocus
Conversation
facebook-github-bot
added
CLA Signed
Base commit: 47261bf |
|
@cortinico Can you please help in reviewing this PR? Thank you for your time. |
cortinico
left a comment
cortinico
left a comment
There was a problem hiding this comment.
@kunalchavhan thanks for sending this over
Can I ask you to add a sample for RN-Tester (the one you're showing in your screenshots)
Sure, will add it. |
|
@cortinico I have added sample in RN-Tester as you suggested. Took the liberty to combine |
| module.exports = ([ | ||
| { | ||
| title: 'Auto-focus', | ||
| title: 'Auto-focus & select text on focus', |
There was a problem hiding this comment.
Why haven't you just added selectTextOnFocus here but instead you also added onChangeText?
There was a problem hiding this comment.
To show selected text I have added value prop with initial value set to Hello World. Earlier the auto-focus sample allowed user input. After adding value prop user input was getting overridden, I have added onChangeText to change value to user's input.
There was a problem hiding this comment.
@cortinico do you want me to remove onChangeText?
There was a problem hiding this comment.
nope, thanks for the clarification 👍
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cortinico One of the checks failed can't get more details on it. Please let me know if anythings needs to be done from my side. |
I'll take care of it. I'm discussing with one of my colleague if we want to merge this or not, I'll get back to you if I need further input |
| if (mSelectTextOnFocus) { | ||
| // Explicitly call this method to select text when layout is drawn | ||
| selectAll(); | ||
| // Prevent text on being selected for next layout pass | ||
| mSelectTextOnFocus = false; | ||
| } |
There was a problem hiding this comment.
Shouldn't we only select text when the text is actually focused?
Making the text selected even when not focused doesn't seem to be right by this propname.
There was a problem hiding this comment.
Thanks for your input @javache. I will add a check isFocused along with mSelectTextOnFocus.
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cortinico merged this pull request in 18d6028. |
|
This pull request was successfully merged by @kunalchavhan in 18d6028. When will my fix make it into a release? | How to file a pick request? |
5 participants
Summary:
Fixes: #43413
This pull request addresses an issue on Android where the text selection was not working when both
selectTextOnFocusandautoFocuswere set to true on TextInput.ReactTextInputManagerwas callingsetSelectAllOnFocusonReactEditTextbefore its onLayout is called causing text selection to not work on auto focus.Changes Made
On the first layout pass, the code now explicitly calls selectAll() to select the text.
Impact
This change ensures that text selection is properly triggered when selectTextOnFocus and autoFocus are both enabled, improving the user experience and making text input behavior consistent and reliable.
Changelog:
[ANDROID] [FIXED]: fixed select text on auto focus for TextInput
Test Plan: