Ghost after drag and drop

[tsuharesu]

I was excited of this because I implemented the same itemTouchStopDrag method locally to clear visuals after I dropped the item, however after this change my list is having a ghost effect.

In my itemTouchDropped method I update my DB and that triggers a new list being set to FastAdapter with itemAdapter.set(newList). According to my logs, the list is being set before itemTouchStopDrag is called.

D/AccountListFragment: ITEM TOUCH ON MOVE
D/AccountListFragment: ITEM TOUCH ON MOVE
D/AccountListFragment: ITEM TOUCH DROPPED
D/AccountListFragment$bindViewModel: SET NEW LIST
D/AccountListFragment: ITEM TOUCH STOP DRAG

Here is a video of what is happening:
https://user-images.githubusercontent.com/808267/114277375-cafbd780-9a22-11eb-9052-54f21f0b17a8.mp4

[mikepenz]

Thank you so much for the PR @tsuharesu

@cketti could you possibly also have a look at this change? Would be super interested if this will also positively impact your implementation.

[cketti]

This looks very similar to the initial implementation. My commit message contains some explanation on why I changed it: ffdc702

Change the way ItemTouchCallback.itemTouchDropped() is called

When using ItemTouchHelper.startDrag() it's possible to get into a state where SimpleDragCallback.clearView() of the dropped item is called after the drag for the next item has already started. By this time the 'from' and 'to' values no longer refer to the item that was just dropped. So the wrong values are delivered to ItemTouchCallback.itemTouchDropped().
To clear the visual highlight from a dropped item a new callback method ItemTouchCallback.itemTouchStopDrag() is introduced.

Unfortunately, it doesn't say how you get into that state. I believe I was dropping an item and then started dragging another one before the animation of the first item was finished.

The behavior in the video looks very strange to me. Maybe using stable IDs will fix the issue?

We're also using itemTouchDropped() to persist the state and that will trigger repopulating the list. In my tests I noticed the animation being slightly bouncy, but nothing serious like what the video shows. We are using stable IDs, though.
I ended up adding a delay so the new state will be saved when the animation has hopefully ended. See thunderbird/thunderbird-android#5243.

[tsuharesu]

@cketti yes, I read all the commits and that's why I reverted only this piece. I couldn't reproduce what you mentioned in the commit as well.

I noticed in your reference that you are using setNewList, while I use set. I'll try later with the other method and see if it works correctly. I don't think creating a delay is the way to go.
You are right, I am not using stableIds, because my items have String UUIDs as IDs.

• try with setNewList

[cketti]

Not using stable IDs probably means the adapter shouldn't be updated while any of the items are being dragged or are in the middle of an animation. I don't see how duplicate items could be avoided otherwise.

Mapping UUIDs to somewhat stable IDs is probably less effort than trying to make this work without stable IDs.

[tsuharesu]

Using setNewList made it worse. Using stableIds fixed. Thank you @cketti and @mikepenz. I'm going to close this PR then.