Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Facebook#1077 - Fix for input bug on Android - space or a special char clears the input #1500

Closed
wants to merge 13 commits into from

Conversation

ashevtcov
Copy link

@ashevtcov ashevtcov commented Nov 9, 2017

Summary

Fix for input bug on Android - typing space or a special char clears the input
#1077

Test Plan

  1. Add the component to your React website (test website)
  2. Run Android emulator
  3. Open your test website
  4. Type a word into the component (for example, "All") and push space
  5. Type another word (for example, "right") and push space

Expected behavior:
Text is not cleared, autocomplete is applied as expected

Wrong behavior fixed by this change:
Text is cleared or wrong symbols are added

@mxstbr
Copy link

mxstbr commented Nov 16, 2017

This works perfectly in our project and fixes typing on my german Android keyboard.

mxstbr added a commit to withspectrum/spectrum that referenced this pull request Nov 16, 2017
Somebody submitted 5 line patch upstream to DraftJS which (should) fix
DraftJS on Android. (facebookarchive/draft-js#1500)

I tested this locally and it works super well on my phone, so I pulled
the repo locally and deployed a quick fork. This PR just sets the
package `draft-js` to install the fork based on that PR instead.

Note that this isn't any kind of long-term solution, but it finally
fixes the problem and let's me respond to messages from my phone, so I
say fuck long-term and let's ship this.
@mxstbr
Copy link

mxstbr commented Nov 24, 2017

Just to report back, we've been running this patch in production for a week now and have gotten 0 complaints about typing on Android anymore. 🎉

@mxstbr
Copy link

mxstbr commented Nov 29, 2017

Update: This doesn't work perfectly, while it makes it possible to type on a german keyboard there's still bugs when autocorrecting/in some other cases that I haven't been able to track down. At least it doesn't crash the editor though...

@arypurnomoz
Copy link

I found this works better with my keyboard

var composedChars = textInputData.trim() ? textInputData : compositionInputData + textInputData

due sometimes the value of textInputData is " " (a space character)

@ayyazdaniaryan
Copy link

Seeing this issue recently on draft-js using Gboard only.

@jrserani
Copy link

Also confirmed that this is popping up on draft-js using Gboard.

@Nantris
Copy link

Nantris commented Mar 12, 2018

Just tested this on Android and it 100% does not work. Unlike other editors, you can't even trick Draft into marginally working.

@aminland
Copy link

This seems to be broken when the caret is placed in the middle of another word. Typing any character causes the full gboard suggestion to be inserted.

@Nantris
Copy link

Nantris commented Mar 13, 2018

@aminland I can't get anywhere close to that level of functionality with Gboard.

@aminland
Copy link

Sorry I meant that's the behaviour with this PR applied.

@fabiomcosta
Copy link
Contributor

@flarnie any way we could get this test and merged for any future release?
As you can see from the comments it's not the perfect fix, but at least people can type something when on Gboard. Currently it's completely broken.
I understand you might be very busy, we just want to know if this will be considered for a future version or not.
Thx for keeping this project going!

@mxstbr
Copy link

mxstbr commented May 26, 2018

@sophiebits just to note, as I said above the typing experience still sucks on Android with this patch applied. Autocorrect messes the text up really really bad.

The only thing this patch adds is that the editor doesn't crash on Android, but imo this isn't anywhere ready for production usage with real users.

@pb-expa
Copy link

pb-expa commented May 26, 2018

We got draft js working on android, but ended up serving a custom textarea component and plugging into/building the editorstate object ourselves on submit.

@mxstbr
Copy link

mxstbr commented May 26, 2018

@pb-expa that's exactly what we did too! 👍 See withspectrum/spectrum#2868, specifically our custom draft-js-plugins-editor wrapper:

https://github.com/withspectrum/spectrum/blob/e7d02da6d03348a54cbbb9c132512a9d8b477657/src/components/draft-js-plugins-editor/index.js#L80-L86

@sophiebits
Copy link
Contributor

Please stop posting +1. I'm going to delete all the (many) +1 comments here. The way this gets merged is if we find answers to the questions I posted above.

@facebookarchive facebookarchive deleted a comment from bchmn May 31, 2018
@facebookarchive facebookarchive deleted a comment from alandardic May 31, 2018
@facebookarchive facebookarchive deleted a comment from dorraba May 31, 2018
@facebookarchive facebookarchive deleted a comment from shaulgo May 31, 2018
@facebookarchive facebookarchive deleted a comment from gilatwix May 31, 2018
@facebookarchive facebookarchive deleted a comment from enterline May 31, 2018
@facebookarchive facebookarchive deleted a comment from eriksape May 31, 2018
@facebookarchive facebookarchive deleted a comment from dariusshop May 31, 2018
@facebookarchive facebookarchive deleted a comment from inifaisal May 31, 2018
@facebookarchive facebookarchive deleted a comment from dfeinzeig May 31, 2018
@facebookarchive facebookarchive deleted a comment from mikesheetal May 31, 2018
@facebookarchive facebookarchive deleted a comment from jonescr3 May 31, 2018
@facebookarchive facebookarchive deleted a comment from pb-expa May 31, 2018
@facebookarchive facebookarchive deleted a comment from charlespeach May 31, 2018
@facebookarchive facebookarchive deleted a comment from ducnvhn May 31, 2018
@facebookarchive facebookarchive deleted a comment from tanat May 31, 2018
Copy link
Contributor

@niveditc niveditc left a comment

Choose a reason for hiding this comment

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

Requesting changes until the questions that @sophiebits asked above are answered.

@Nantris
Copy link

Nantris commented Sep 10, 2018

Lots of information related to those questions here: ianstormtaylor/slate#2062

As per @thesunny's research, as I understand it, there is no beforeinput before Android API 27, which corresponds with Android Oreo 8.1.0.

@sandorvasas
Copy link

sandorvasas commented Feb 4, 2019

@sophiebits You should just merge it, even a partial fix is a better fix than leaving it bugging. We heavily invested in integrating this component and turns out it's unusable on Android devices. It's bad that it's not working, but it's a real shame FB that you're even halting the patches to be merged.

@thesunny
Copy link

thesunny commented Feb 4, 2019

I wrote the Android compatibility for SlateJS. Thought I'd pop in and give some hints.

To get Android to work, here are a few key things that you will need for Android:

  • Every version of Android is different. You cannot target one version and have it work on all versions. Be prepared to commit to making a few different versions of your compatibility layer.
  • You cannot know, by looking at a single event, what the user did (it may work in some versions but not others). You will have to use a combination of looking at multiple events and/or doing DOM analysis.
  • You cannot always intercept an event. For example, you can't look for an enter and stop it and handle it on your own. By the time you know that it's enter, you can't call preventDefault to stop it either because (a) you know after the event happened that it was an enter because there are no properties on the event that tell you so or (b) the browser simply doesn't respond to the preventDefault.
  • You will need a library to revert the DOM to an earlier safe state in order to work with React. For example, if you hit enter, Android will split the DOM in its default way and you will need to revert it or else React will get confused and throw errors. I have a DomSnapshot module inside Slate that you are free to use inside my PR. I usually take the Snapshot on a keydown or compositionEnd and then after Android does its thing, I revert the DOM to that state.

Good luck guys. It took several solid weeks of work for me but if you look at the code, you can probably save time.

@thesunny
Copy link

thesunny commented Feb 4, 2019

Here's the issue which contains the most info:
ianstormtaylor/slate#2062

Here's the new PR:
ianstormtaylor/slate#2565

Here an older PR with more details:
ianstormtaylor/slate#2553

@niveditc
Copy link
Contributor

niveditc commented Feb 4, 2019

@thesunny - thanks so much for the detailed response above! I'm sure it will help inform further attempts for improving Android Web support in Draft.js.

I also wanted to mention that @flarnie has compiled a very thorough list of issues + explanation of the state of Android Web support on #1895 .

@ashevtcov
Copy link
Author

Ok, my friends, I am closing this PR. First, it was never a fix for the full problem, I covered only limited number of issues, listed in the description - at that point in time it was enough. Since I issued this fix, Google implemented a lot of changes that make this fix just utterly insufficient: so many other issues appeared and fixing just this side effect doesn't change the game significantly at all.

@ashevtcov ashevtcov closed this Feb 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.