webscopeio / react-textarea-autocomplete

📝 React component implements configurable GitHub's like textarea autocomplete.
MIT License
451 stars 80 forks source link

Selection not working correctly on touch device #102

Closed diogeneshamilton closed 5 years ago

diogeneshamilton commented 5 years ago

Version: 3.1.1 Video recorded on iOS Simulator (so these clicks are actually Touch events) bug 1

I believe this may have been introduced in this PR: https://github.com/webscopeio/react-textarea-autocomplete/pull/90

It may be alleviated by adding onTouchStart as the same as the OnClick handler.

EDIT: Downgrading to v2.3.5 fixes this issue, so something after that must have introduced it.

jukben commented 5 years ago

Hey Matthew!

Thanks for the ☕️, you are the best! ❤️ I'll take a look on this, I believe it might be an easy fix.

jukben commented 5 years ago

May I ask you which simulator do you use? I've just tried the default project (you can clone the repo and do yarn dev) on iPhone XR 12.1 and everything (I mean, the viewport is not responsive, but) works fine. You can click on the item, the item gets replaced and autocomplete is closed.

EDIT: I'm able to reproduce it, I have to return the trigger token in the item. E.g @ => @bobby.

jukben commented 5 years ago

Hey. @diogeneshamilton You are right onStartTouch solved the issue in my case. Could you try it in your scenario? I've just deployed v3.1.2 🙏

diogeneshamilton commented 5 years ago

Thanks! I've been testing, and unfortunately it doesn't work flawlessly in my case since it's a scrollview. I've been working on finding a fix, but the logic around selecting an item (and what closes the autocomplete) has prevented me from finding the perfect one yet. I can work on this a bit today though. Thanks for your help!

diogeneshamilton commented 5 years ago

Looks like I was able to fix my issues by essentially tracking touches on the list items:

            onTouchStart={(e) => { 
                this.clicked = true
                this.selectItem(e)
            }}
            onTouchEnd={(e) => { 
                if (this.clicked) {
                    onClickHandler(e)
                }
            }}
            onTouchMove={(e) => { 
                this.clicked = false
            }}, 
            onTouchCancel={(e) => { 
                this.clicked = false
            }}

Not sure if this is the most elegant solution, but maybe more complete for all touch cases. For now I'll downgrade to 2.3.5 for my production app (or use a doctored version with this fix), but let me know what you end up going with. I'm happy to submit a PR too if that's easier.

jukben commented 5 years ago

Yeah, I had some cancers regarding the mobile devices - you are probably the first one who is trying to use RTA in such a use case. Your solution seems nice! Definitely open a PR I'll polish it a bit and if it solved your issue, I'm gonna merge it.

Just curious, do you think this is React version related? That's unexpected.

rrikesh commented 5 years ago

I'm also having this issue on iOS Safari on the emoji example, I'm using version 3.1.2

I also get the issue when I try it using the example from yarn dev

jukben commented 5 years ago

Yeah, @rrikesh I guess the solution by @diogeneshamilton might solve it. Are you able to test it and send PR? Otherwise, I might have time for it during the weekend. 🚀

rrikesh commented 5 years ago

I tried @diogeneshamilton's fix but didn't work. Or maybe I am doing something wrong. I'll try to dig in the code again during the day. I'll send a PR if I manage to fix it.

rrikesh commented 5 years ago

I sent a PR based on @diogeneshamilton's fix. I tested on iOS only.

rrikesh commented 5 years ago

Tested on Android too! All good on my side!

diogeneshamilton commented 5 years ago

Absolute win!!! Thank you @rrikesh!!!

jukben commented 5 years ago

Thanks to both of you! Should be ready in 3.1.4 (along with perf fix). 🚀