react-native icon indicating copy to clipboard operation
react-native copied to clipboard

fix: select text on auto focus for TextInput

Open kunalchavhan opened this issue 1 year ago • 7 comments

Summary:

Fixes: #43413

This pull request addresses an issue on Android where the text selection was not working when both selectTextOnFocus and autoFocus were set to true on TextInput. ReactTextInputManager was calling setSelectAllOnFocus on ReactEditText before its onLayout is called causing text selection to not work on auto focus.

Changes Made

  • Added logic to wait for the ReactEditText view's layout to be drawn before attempting to select the text. On the first layout pass, the code now explicitly calls selectAll() to select the text.
  • Implemented a check to ensure selectAll() is only called during the first layout pass, avoiding unnecessary calls on subsequent layout passes.

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:

Before After
before after

kunalchavhan avatar Jun 17 '24 09:06 kunalchavhan

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 20,366,365 -901,491
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 23,562,945 -902,029
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 47261bfb791a70e60de6e514fcfbbb6bff2cee09 Branch: main

analysis-bot avatar Jun 17 '24 10:06 analysis-bot

@cortinico Can you please help in reviewing this PR? Thank you for your time.

kunalchavhan avatar Jul 01 '24 07:07 kunalchavhan

@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.

kunalchavhan avatar Jul 04 '24 07:07 kunalchavhan

@cortinico I have added sample in RN-Tester as you suggested. Took the liberty to combine autoFocus and selectTextOnFocus in one sample instead creating separate one to avoid focus conflict hope this is fine.

kunalchavhan avatar Jul 04 '24 11:07 kunalchavhan

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

facebook-github-bot avatar Jul 08 '24 10:07 facebook-github-bot

@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.

kunalchavhan avatar Jul 10 '24 06:07 kunalchavhan

@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

cortinico avatar Jul 10 '24 12:07 cortinico

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

facebook-github-bot avatar Jul 12 '24 14:07 facebook-github-bot

@cortinico merged this pull request in facebook/react-native@18d6028ff908eef99aae363d8ee9a6789d264284.

facebook-github-bot avatar Jul 12 '24 17:07 facebook-github-bot

This pull request was successfully merged by @kunalchavhan in 18d6028ff908eef99aae363d8ee9a6789d264284.

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

github-actions[bot] avatar Jul 12 '24 17:07 github-actions[bot]