Graeme Graeme - 4 months ago 142
Android Question

RecyclerView corrupts view using notifyItemMoved()

I'm having a problem with using the

notifyItemMoved()
method. It seems to be incorrectly displaying unmoved views.

My list has 4 element in it. What I want to do is animate a swap between item 1 and item 3. Items 1 and 3 swap correctly, but item 2 displays what was at item 3!

So the list starts off looking something like this:

Item 0
Item 1
Item 2
Item 3


And ends like this:

Item 0
Item 3
Item 3 <!-- What the heck has this changed for?
Item 1


My adapter is backed by a
List mProductList
. I call the following code:

public void sortBackingListUsingSortingList(List<ProductWrapper> newProductItems) {
Log.e("", "Before:");
for(ProductWrapper wrapper : mProductItems) wrapper.log();
for(int i = 0; i < newProductItems.size(); i++) {
ProductWrapper currentItem = mProductItems.get(i);
ProductWrapper correctItem = newProductItems.get(i);

if(!currentItem.equals(correctItem)) {
// Item in wrong place
int indexOfCorrectItem = getIndexOfItemInList(mProductItems, correctItem);
Collections.swap(mProductItems, i, indexOfCorrectItem);
notifyItemMoved(i, indexOfCorrectItem);
Log.e("", "notifyItemMoved(" + i + ", " + indexOfCorrectItem+")");
Log.e("", "After:");
for(ProductWrapper wrapper : mProductItems) wrapper.log();
}
}
}


I've also added logging to
onBindViewHolder
to check if my view logic is being called:

@Override
public void onBindViewHolder(HolderBasic holder, int position) {
Log.e("", "onBindViewHolder(holder, " + position + ")");
holder.fill(mProductItems.get(position));
}


My logs look like this:

09-02 14:39:17.853 ﹕ Before:
09-02 14:39:17.853 : Item 0
09-02 14:39:17.853 : Item 1
09-02 14:39:17.853 : Item 2
09-02 14:39:17.853 : Item 3

09-02 14:39:17.854 ﹕ notifyItemMoved(1, 3)

09-02 14:39:17.854 ﹕ After:
09-02 14:39:17.854 : Item 0
09-02 14:39:17.854 : Item 3
09-02 14:39:17.854 : Item 2
09-02 14:39:17.854 : Item 1

09-02 14:39:17.867 ﹕ onBindViewHolder(holder, 1)
09-02 14:39:17.874 ﹕ onBindViewHolder(holder, 3)


As you can see, no reason for Item 2 to have change it's display at all - and yet, it does. Anybody know why?

EDIT

I can get around above by looping through the entire adapter and calling
notifyItemChanged()
on every item. Inefficient and not a good solution, but is invisible to the user.

Answer

Thank you to @david.mihola for leading me to what I'm doing wrong.

This took so long to figure out as the symptom didn't make the problem obvious!

I was doing this:

Collections.swap(mProductItems, i, indexOfCorrectItem);
notifyItemMoved(i, indexOfCorrectItem)

But, I obviously didn't think through what notifyItemMoved() was actually doing. It is only notifying the adapter that item i has moved to indexOfCorrectItem it isn't telling the adapter that indexOfCorrectItem has also moved to i.

Under the covers it was doing the following:

  1. Move item 1 to 3
  2. Move what was at 2 to 1 to fill the gap
  3. Move what was at 3 to 2 to fill the gap
  4. notifyItemChanged(1);
  5. notifyItemChanged(3);

The above of course leaves item 3 moved down to item 2 without a refreshed view! It was steps 4 and 5 which were hiding the problem by making item1 and item3 display correctly and leaving item2 incorrect!

As soon as I realised this I tried the following code:

notifyItemMoved(indexOfCorrectItem, i);
notifyItemMoved(i, indexOfCorrectItem);

This left the list in the correct order, but it short circuited the animation.

So, instead, I dumped swapping altogether:

mProductItems.remove(indexOfCorrectItem);
mProductItems.add(i, correctItem);
notifyItemMoved(indexOfCorrectItem, i);