Wrong item on click
Opened this issue · 6 comments
Hi, found a bug.
There is a repro here. Just click the item twice, first time is fine, second time is incorrect.
Alright I think that this is a good explanation of what happens but I'm not sure how to fix it:
- We pass [ Item(id= 1, name = "Test")!01 ] to Groupie.
- Groupie sets that to its internal list.
- DiffUtils tells Groupie that a change has happened and Groupie creates a ViewHolder.
- The ViewModel is bound with the current Item.
- We force refresh. (Reload data from the network and create new items)
- We pass [ Item(id= 1, name = "Test")!02] to Groupie
- Groupie sets that to its internal list.
- DiffUtils finds no changes and says noting about an update. (This is what we want, kind of)
- We click the view (item in the list) its ViewHolder which has the click listener gets called. Since it's not been bound after the last update it still has the old Item (Item(id= 1, name = "Test")!01 ) so it returns it.
- We try to find the item in the Adapter (Groupie) but since it has updated to our new list the item is not found thus returns
-1
.
Not sure if related, but I'm experiencing something very similar. I'm using ItemTouchHelper
to reorder views.
(adapter as GroupAdapter).getAdapterPosition(item)
will give me the original position of the item, and not the updated one. even though I called adapter.notifyItemMoved(to, from)
when the item was moved
@Meisolsson , I was able to reproduce this in the example project by recreating your example. Thank you so much for the repo.
It's being caused because Item.hasSameContentAs()
is returning true since you overrode equals
; as you observed, there is no bind
called in the ViewHolder, which is appropriate, but the ViewHolder is not internally updated with the new Item. This is wrong (an oversight in the library).
I'm working on a fix for this now.
This "bug" seems to occur when the list is mutable, but the elements contained are immutable. For example in a social media feed, I can't edit the posts from people I follow but I can delete posts that I consider to be spam or unamusing. If I want to delete a post, when groupAdapter.update()
is called, the element is successfully removed, but bind()
is not called for the remaining elements as the haven't changed (remember they are immutable). Any indexes attached to UI event listeners won't be updated, which can be a HUGE problem.
This is definitely a pain point, but Groupie's behavior makes sense to me. I would advise people to not use the position
parameter in bind()
when using mutable lists. In fact I don't think that this parameter is ever useful since all of the elements data should be stored in the BindableItem
instance. There's no need to use an index lookup.
The same problem. So i clear() first and then addAll().
@lisawray any progress?
I don't see why mutating an item and making it the same from old state to new state thus consuming the change event is supposedly a bug in Groupie.
I think it's a developer error in expecting a DiffUtil to evaluate a change where there is no difference the provided old and new state. 🤔