Skip to content

Conversation

@etoledom
Copy link
Contributor

This PR fixes the html text input that stoped responding to changes on Android.

On iOS:
If the html string is stored on the state and it changes, the whole TextInput is recreated, making jump on certain circumstances. (ref: #115)

On Android:
If the html string is not stored on the component's state, the state of the TextInput won't change even when the user tries. In other words, #115 broke Android's html TextInput.

This solution implements a different way to hold and get the html string for iOS and Android.

  • I'm inclined to create an HTMLTextInput component that handles this problem internally, but I prefer to ask for suggestions before implementing it.

This helps the html TextInput on Android to be editable and change its state properly.
@etoledom etoledom added the [Type] Bug Something isn't working label Sep 18, 2018
@etoledom etoledom self-assigned this Sep 18, 2018
@etoledom etoledom requested a review from hypest September 18, 2018 22:19
@hypest
Copy link
Contributor

hypest commented Sep 19, 2018

I'm inclined to create anHTMLTextInput component that handles this problem internally, but I prefer to ask for suggestions before implementing it.

Yes, that would actually be useful since there are moving parts all over the place in BlockManager, especially now that it seems the different mobile platforms demand different implementations.

You think you can work on that in this PR? If not, happy to have it in a separate PR.

Otherwise, I tested this PR until this point and I can confirm it fixes the regression on Android. iOS continues to work as well.

@etoledom
Copy link
Contributor Author

Thank you @hypest !

I'll merge this to fix the issue on master.
Right after that I'll send a PR with that improvement.

@etoledom etoledom merged commit f36fd5e into master Sep 19, 2018
@etoledom etoledom deleted the fix/android-html-text-input branch September 19, 2018 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants