splitwise / TokenAutoComplete

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

Added mode to allow/disallow removal of tokens #161

Closed sweggersen closed 9 years ago

sweggersen commented 9 years ago

These extensions allows the field to have some fixed tokens that cannot be removed by the user. This is useful if you have some tokens that are pre filled, and you only allow the user to make additions of the tokens available.

mgod commented 9 years ago

I'm not a huge fan of this feature because managing input for this library is pretty complex and this kind of change makes testing (which needs to be manually run on several devices with different custom inputs) more time consuming. It also seems like if a user is presented with a field full of tokens, making some them removable and some not is pretty strange. Maybe you should just add tokens to the prefix? What's your use case?

I'm happy to add this with some changes:

For all that I seem skeptical on this, I'm really happy to see this PR! Most of these suggested changes are aimed at reducing the code paths that need testing as much as possible and making maintenance more straightforward. It looks like there are some common patterns like rotating the view, custom input keyboards and moving the cursor to the middle of the field that you haven't tested yet and these changes should fix most issues with those.

sweggersen commented 9 years ago

Thanks for the comment. Great feedback. I'm using the suggested feature for a system that uses the field as a filter system, where some of the tokens are pre populated and should not be removed. The ones that are removable has an x in the view, so the user is able to see which ones that can be removed visually. Iv'e made the changes that you suggested.

mgod commented 9 years ago

Hey, I'm finally getting around to merging this. One thing I'm confused by is setting the keyListener. Is this just so people using the class can also set a keyListener, or is there something specific to this that you're using the keyListener for?

mgod commented 9 years ago

Also, thanks for being responsive about making the changes I asked for :)

sweggersen commented 9 years ago

No problem :) The keyListener is basically there to prevent a backspace key event deleting a token, when you have specified that tokens should not be removed.

mgod commented 9 years ago

Hmmm. I'm seeing both the OnKeyListener.onKey method and onKeyDown methods being called when I do hardware keypresses. I'd rather just use the onKeyDown override and not deal with the OnKeyListener. Are you seeing a case where you needed the OnKeyListener?

sweggersen commented 9 years ago

I see what you mean. I'll make a quick fix.

mgod commented 9 years ago

Thanks! Merged this in 8a4307f843912c8af18295c053877b39a8c3ac74