Wrong adapter swapping when GridLayoutManager and several columns

[anthony-skr]

Hello,

In case of GridLayoutManager, it's possible to move an item from a row to another. If the number of columns is superior to 1, we graphically swap more than one item and this case is not correctly handled by adapter.

Initial state :

A	B
C

Drag&drop A :

B	C
A

Current implementation will just swap A and C, mItems will be equal to :

item[0] = C
item[1] = B
item[2] = A

Which is not good.

It corresponds to receive onItemMove() event with from/to having index distance of 2 or more. In my example: from = 0 and to = 2.

I handled this case like this :

        @Override
        public boolean onItemMove(int fromPosition, int toPosition) {
            if (fromPosition < toPosition) {
                for (int i = fromPosition; i < toPosition; i++) {
                    Collections.swap(mItems, i, i + 1);
                    notifyItemMoved(i, i + 1);
                }
            } else {
                for (int i = fromPosition; i > toPosition; i--) {
                    Collections.swap(mItems, i, i - 1);
                    notifyItemMoved(i, i - 1);
                }
            }
            return true;
        }

I'm not fully sure it covers correctly all cases, other eyes will be appreciated.

Anthony Skrzypczyk.

[iPaulPro]

As mentioned in the article, onItemMove is called every time an item index changes. This means that for your example, it will call

onItemMove(A, B) then onItemMove(B, C)

This is clearly demonstrated and working in the "Grid - Basic Swipe" example RecyclerGridFragment.

[anthony-skr]

Hello,

Here the proof it is not working as you say :

    @Override
    public boolean onItemMove(int fromPosition, int toPosition) {
        for (int i = 0; i < mItems.size(); i++) {
            Log.d("DBG", "before item " + i + " = " + mItems.get(i));
        }

        Log.d("DBG", "move " + fromPosition + " to " + toPosition);
        Collections.swap(mItems, fromPosition, toPosition);
        notifyItemMoved(fromPosition, toPosition);

        for (int i = 0; i < mItems.size(); i++) {
            Log.d("DBG", "after item " + i + " = " + mItems.get(i));
        }

        return true;
    }

Before moving first item to the second line :

08-06 16:58:57.509: D/DBG(13376): before item 0 = One
08-06 16:58:57.509: D/DBG(13376): before item 1 = Two
08-06 16:58:57.509: D/DBG(13376): before item 2 = Three
08-06 16:58:57.509: D/DBG(13376): before item 3 = Four
08-06 16:58:57.509: D/DBG(13376): before item 4 = Five
08-06 16:58:57.509: D/DBG(13376): before item 5 = Six
08-06 16:58:57.509: D/DBG(13376): before item 6 = Seven
08-06 16:58:57.509: D/DBG(13376): before item 7 = Eight
08-06 16:58:57.509: D/DBG(13376): before item 8 = Nine
08-06 16:58:57.509: D/DBG(13376): before item 9 = Ten

08-06 17:02:39.719: D/DBG(13376): move 0 to 2

After :

08-06 17:02:39.719: D/DBG(13376): after item 0 = Three
08-06 17:02:39.719: D/DBG(13376): after item 1 = Two
08-06 17:02:39.719: D/DBG(13376): after item 2 = One
08-06 17:02:39.719: D/DBG(13376): after item 3 = Four
08-06 17:02:39.719: D/DBG(13376): after item 4 = Five
08-06 17:02:39.719: D/DBG(13376): after item 5 = Six
08-06 17:02:39.719: D/DBG(13376): after item 6 = Seven
08-06 17:02:39.719: D/DBG(13376): after item 7 = Eight
08-06 17:02:39.719: D/DBG(13376): after item 8 = Nine
08-06 17:02:39.719: D/DBG(13376): after item 9 = Ten

Which is wrong because the first item is "Two" and not "Three" :

[iPaulPro]

onItemMove would never be called from with 0 as fromPosition and 2 as toPosition. As I explained above, an actual drag from 0 to 2 would call

onItemMove(0,1)
onItemMove(1,2)

I have tested this on supported devices, and it works as expected. Are you actually performing a drag, or just debugging this with manual parameters?

[iPaulPro]

Ah! You're saying that the items show properly, but the underlying data is not properly reflecting the change.

Sorry, I had misread your issue. Re-opening.

[anthony-skr]

Yes it's all about mItems indexes :sweat_smile:

UI works like a charm but I'm saving in cache each item position for user and I needed data reflecting exactly the UI.

[iPaulPro]

I will be updating the code and article to make this correction. Thanks!

Note, you can simplify to this:

@Override
public boolean onItemMove(int fromPosition, int toPosition) {
    if (fromPosition < toPosition) {
        for (int i = fromPosition; i < toPosition; i++) {
            Collections.swap(mItems, i, i + 1);
        }
    } else {
        for (int i = fromPosition; i > toPosition; i--) {
            Collections.swap(mItems, i, i - 1);
        }
    }
    notifyItemMoved(fromPosition, toPosition);
    return true;
}

Since notifyItemMoved takes care of both shifts (hence my error).

[callmepeanut]

I solve this by: String tmp = mItems.get(fromPosition) mItems.remove(fromPosition); mItems.add(toPosition, tmp); much shorter.