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

Make EditText selectable when view attached to window #11

Closed
wants to merge 5 commits into from

Conversation

mzorz
Copy link

@mzorz mzorz commented Nov 21, 2018

It's been detected that RN's TextInput components within the list, that are shown offscreen when the list is first loaded, do not allow their text to be selected.

Background

On the technical side, apparently the problem with RN’s TextInput should be fixed in this commit facebook/react-native@89340f1 but we can see it still happens, and for some reason while this code exists for TextView, this modification is not on EditText anymore (see current RN master https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java)
(EDIT: the code has been reverted super shortly after in facebook/react-native@862136a#diff-457fd2ba359cc6fbb0c471516e279696, with the apparent intention to get it back in but never did).

As a workaround, this PR makes sure any EditText that's within a list row view hierarchy gets set to selectable if focusable when it gets attached to the window (that is, when it's about to become visible and hence actionable).

To test:
Before:

  1. run the app
  2. observe the word hello in the TextInput is selectable in the first few rows being shown
  3. scroll the list down (1 screen down is enough, as the problem happens with items that are not visible on first load)
  4. now place the cursor on the TextInput of any of these items and try selecting text. Observe it's not possible.

recyclerv_offscreen_items_not_ok

After:
Repeat the steps above and observe all TextInput is selectable.
recyclerv_offscreen_items_yes_ok

ArrayList<View> allEditTextsInViewGroup = getAllEditTextChildren(holder.itemView);
for (View child : allEditTextsInViewGroup) {
EditText editText = (EditText) child;
// by default, an EditText that is focusable should also be selectable, so

Choose a reason for hiding this comment

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

It seems to me that we're making a strong assumption here. This could work in our case, but I can have an EditText that is focusable but not selectable.
Should we check textIsSelectable before resetting it to true?

Copy link
Author

Choose a reason for hiding this comment

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

I agree it is an assumption. Unfortunately there is no query API for this. However, I can't think of a practical case where it makes sense to use an EditText that is focused but can't be selected. It may make sense for TextView though. We can certainly live without that in this fwiw.

@hypest
Copy link

hypest commented Feb 21, 2020

I think this PR is probably too old by now to consider. WDYT about closing it @mzorz ?

@mzorz
Copy link
Author

mzorz commented Feb 21, 2020

Closing

@mzorz mzorz closed this Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants