search

splitwise / TokenAutoComplete

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

Fixes for various issues #317

Closed alexcustos closed 5 years ago

mgod commented 7 years ago

Can you split these up into separate commits (or even PRs) for each fix. This whole project has a lot of strange interactions across devices and keyboards, so knowing why a one-line change was made is usually important and I have no idea what you're fixing here.

alexcustos commented 7 years ago

All these changes is a product of long evolution. The history was messed up when I have moved this code out of my project tree and synced it with the actual master branch. Now I'm using my fork via jitpack, so I can't freely rebase or change it. Unfortunately, there is no other way to deliver pull requests. But I have put to this pull request very annoying and critical issues, so may be there are no reasons to split it. Here's the brief of changes, please consider it and let me know what you think. From top to bottom: 1) MultiAutoCompleteTextView was replaced with AppCompatMultiAutoCompleteTextView because I got a compatibility issue on KitKat devices. In any way it's a good practice to use appcompat library. 2) "if (i > 0 && text.charAt(i) == ' ')": here I fought an issue with spaces. When spaces is allowed they look bad at the beginning and at the end of token. 3) I can't remember what requested me to add this dummy CREATOR, but I do not like adding anything without a good reason. Sure I was really forced to do it. 4) "return TextUtils.substring(editable, start, end).trim();": issue with spaces. 5) "if (!hasFocus()) { requestFocus(); }": issue with token selection when the cursor is out of the field. 6) Last changes is about two issues: a) NPE when toString() returns null b) spaces.

mgod commented 5 years ago

I'm pretty sure everything except the hint text issue should be resolved in 3.0.0. If you want to open a PR for just that, I'm happy to merge it in.