splitwise / TokenAutoComplete

Gmail style MultiAutoCompleteTextView for Android
Apache License 2.0
1.3k stars 384 forks source link

Deleting Text Doesn't Delete Token #370

Closed AEFeinstein closed 5 years ago

AEFeinstein commented 5 years ago

First off, I'm super excited about 3.0.0! I'm really looking forward to synchronous methods.

I did notice a change in behavior. With 2.0.8 if there is a completed token and I go to delete it with the keyboard's delete button, the whole token would be deleted. With 3.0.0-alpha, when I try to delete a token, it is replaced by the token's text. I would like the old behavior restored or made configurable please.

I believe this is due to the commented code in afterTextChanged(), though I haven't had a chance to test that hypothesis.

AEFeinstein commented 5 years ago

Nope, uncommenting that code and using containsTokenTerminator() instead of isSplitChar() didn't fix it. I'm not sure what you changed...

mgod commented 5 years ago

Thanks for checking it out! I used to have the TokenDeleteStyle that would in theory do this, but I found that the underlying replacement behavior depended on the OS and keyboard in a lot of cases and I could not control it. If you run the 2.0.8 version on the exact same device you're seeing the behavior change in, is it actually different between 2.0.8 and 3.0.0?

I would also prefer to have this configurable in the library, but it seemed like it was false advertising as the behavior only sometimes follows the documentation. From the next version feedback, it does look like I should consider making the Clear token delete style implementation the default in the library, which might resolve the issue (it looks like everyone who commented is using Clear) without suggesting to users of the library that this is configurable.

AEFeinstein commented 5 years ago

Yep, the behavior is different on the same device. You can see this with your example app (compiled and attached). With the example from the 2.0.8 release, pressing the delete key with the cursor after the "margaret@example.com" token deletes the entire token. Doing the same with the example from 4a9cf45af7782533e8e3fc234eb7f096c6bddedf replaces the token with "Margaret Smith."

examples.zip

AEFeinstein commented 5 years ago

And yes, in 2.0.8, Clear was the default behavior, set in TokenCompleteTextView()'s init():

        //We had _Parent style during initialization to handle an edge case in the parent
        //now we can switch to Clear, usually the best choice
        setDeletionStyle(TokenDeleteStyle.Clear);

If Clear is the only behavior in 3.0.0, I'd be happy with that.

mgod commented 5 years ago

Thanks. It looks like this was a keyboard issue - the physical keyboard input I was using for testing happened to not have this issue. I think I've figured out where a lot of the behavioral differences were coming from, so hopefully the next version will resolve this (and use the Clear style)

AEFeinstein commented 5 years ago

Glad to hear it, I'll keep an eye on this ticket to test the next version and let you know if I notice anything else.

I know 3.x.x is only in alpha, but the synchronous methods fix at least one bug in my own app, so an alpha2 with Clear behavior is something I'm definitely interested in!

mgod commented 5 years ago

3.0.0-alpha2 should be available and resolves this. I'm still seeing some edge cases where things break in this version, but it's probably pretty close to being ready.

AEFeinstein commented 5 years ago

3.0.0-alpha2 works great! The removal of allowDuplicates() threw me for a little loop, but shouldIgnoreToken() seems so much more flexible.