autocomplete: clear active item after selection #84
Conversation
| @@ -47,3 +47,6 @@ module.exports = class KDAutoCompleteListView extends KDListView | |||
| active.index = i | |||
| break | |||
| active | |||
|
|
|||
| setActiveItem: (target) -> | |||
| item.active = item is target for item in @items | |||
There was a problem hiding this comment.
I think itemDataPath option should play a part here if it is set to a non-null value.
There was a problem hiding this comment.
The implementation is designed to be symmetrical to the getter. Can you explain your idea?
There was a problem hiding this comment.
Identity of a list item is determined by itemDataPath when an object is passed as an argument to a method of list view. I thought keeping the same behavior in similar views would be consistent semantically. Equality operator is not always enough to determine same object.
There was a problem hiding this comment.
@sinan can you advise about this? I am not sure I understand about the item data path. In the case of autocomplete, i believe that the is operator will be sufficient to compare list items.
There was a problem hiding this comment.
If you have two different copies of the very same object then is operator returns false and comparing argument of the function against list data misses the item you want to be processed.
This is what we have experienced with removing an item from list view. Data was fetched from two different sources. Identical two objects stored in different places in the memory. is operator was returning false. So, implementation improved to take data path into account if it's provided to compare identity of the argument and item in the list.
This is the case I wanted to be covered.
There was a problem hiding this comment.
OK, but I don't think that can happen for this method, which is just going to set the active item for the autocomplete view, and not for list views in general. If you want me to make this change, can you please open a PR against my fork that implements your suggestion?
There was a problem hiding this comment.
If I can find time to do that then I will open another pull request. This shouldn't hold off the pull request. I'll be merging this.
There was a problem hiding this comment.
Sorry, I would make the change myself, but I wasn't able to grok your suggestion. I don't understand how I should check the identity of the object by the item data path, which is just a string.
|
@szkl can we merge this please? |
|
Right away. |
autocomplete: clear active item after selection
After an item is chosen, we should clear the selected item in the list view. The implementation is a little weird. I noticed that code was pretty old that I was editing :). This fixes a bug where the same item will be selected multiple times when it shouldn't be.