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

[TextInput] Blur event is broken in v0.57.6 #22425

Closed
3 tasks done
ahce opened this issue Nov 27, 2018 · 11 comments
Closed
3 tasks done

[TextInput] Blur event is broken in v0.57.6 #22425

ahce opened this issue Nov 27, 2018 · 11 comments
Labels
Bug Component: Text Component: TextInput Related to the TextInput component. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.

Comments

@ahce
Copy link

ahce commented Nov 27, 2018

Environment

React Native Environment Info:
    System:
      OS: Windows 10
      CPU: (4) x64 Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz
      Memory: 4.29 GB / 11.87 GB
    Binaries:
      Yarn: 1.12.3 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
      npm: 6.4.1 - C:\Program Files\nodejs\npm.CMD
    IDEs:
      Android Studio: Version  3.1.0.0 AI-173.4907809
    npmPackages:
      react: 16.6.1 => 16.6.1
      react-native: 0.57.6 => 0.57.6

Description

The input is not focused after the blur event with the done button.
Blur event was broken with this commit,

Reproducible Demo

  1. Focus the input
  2. Press the done button in keyboard
  3. Try focus the input again
@timwangdev timwangdev added the Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. label Nov 27, 2018
@radeno
Copy link
Contributor

radeno commented Nov 27, 2018

@gazoudoudou can you check that please?

@radko93
Copy link
Contributor

radko93 commented Nov 27, 2018

Happens only on Android for me (I will label it like that), if anyone can reproduce on iOS please write it here.

@kelset
Copy link
Contributor

kelset commented Nov 27, 2018

Hey @ahce thanks for the report, since it's a fairly big issue I'll revert that commit and create a new release asap.

@Minishlink
Copy link
Contributor

Minishlink commented Nov 27, 2018

@kelset Shouldn't we fix the underlying issue? It seems wrong that this "double blur" is needed on Android and not on iOS (and that it is needed at all)

@kelset
Copy link
Contributor

kelset commented Nov 27, 2018

@Minishlink absolutely, the issue should get a proper fix: but that needs to be a PR to master and fix it 'once and for all'.

This revert & new release is to simply prevent people from facing it in 0.57.patch - now that we are aware of the fact of that commit being broken, I'll leave this issue open so that we can properly fix it.

@ahce let me know if 0.57.7 works fine for you now (I just created a test project and was able to repro, and updating to 0.57.7 fixed this)

kelset referenced this issue Nov 27, 2018
Summary:
I noticed that the _onBlur method was not exactly similar to the _onFocus one in the TextInput component.

After digging, I found that the blurTextInput method in the TextInputState.js file was call twice in a raw instead of once when the textinput component should blur.

By removing this line, I fix this unecessary multiple call.
Pull Request resolved: #22156

Reviewed By: TheSavior

Differential Revision: D13105396

Pulled By: RSNara

fbshipit-source-id: 8e83461d8b288d8ee4047bc4a33c4480e193c349
@ahce
Copy link
Author

ahce commented Nov 27, 2018

@kelset works fine in 0.57.7, Thanks!

@sahrens
Copy link
Contributor

sahrens commented Nov 27, 2018

FYI, @RSNara has a fix incoming to master.

@sahrens
Copy link
Contributor

sahrens commented Nov 27, 2018

Fix is to revert #22156

@radko93
Copy link
Contributor

radko93 commented Nov 27, 2018

@sahrens so there's no better fix for that? IMO at least on iOS we should not call it twice.

@sahrens
Copy link
Contributor

sahrens commented Nov 27, 2018

We're reverting for now to be safe. Could potentially add an iOS check if it's causing problems.

@kelset
Copy link
Contributor

kelset commented Mar 19, 2019

Closing since this issue has actually being fixed by the revert.

@kelset kelset closed this as completed Mar 19, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Mar 19, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Mar 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Component: Text Component: TextInput Related to the TextInput component. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

9 participants