Move animation for StaggeredGridLayoutManager is broken when using DefaultItemAnimator
This looks like a bug in StaggeredGridLayoutManager. I have taken your app and made a few modifications to it for the following demo. Consider the following that makes this point:
As you can see, initially everything works as expected: Items move and the animations are as they should be when the screen is not full. However, when we add item "M", everything goes haywire as you have presented. My conclusion is that this is a bug. It looks like a boundary problem related to gap management but that is just my guess.
I have further confirmed this as a layout issue and can demonstrate that while the order of the children of the RecyclerView is correct, StaggeredGridLayout fails to layout the children in the same order. When you scroll, you are forcing the layout manager to take another look at the layout and then it gets it right although there are no changes to the adapter.
I will add that this is not an issue with the item animator. If you set the animation off (recyclerView.setItemAnimator(null);) the problem persists.
One workaround is to customize ListUpdateCallback to capture the problem situation and do something different. In the code below, the onMoved() method of MyListUpdateCallback looks for a move to position 0 and call notifyItemChanged on the from the to positions; otherwise, processing proceeds as normal.
Here is the app with the fix applied:
While this solution addresses the issues with the MCVE that you offered, it may not address the issue in the real app exactly as written. If it doesn't, I believe that this approach can be adapted to work.
MainActivity.java
This is the updated code for the demo. Set the boolean mDoTheFix to true to apply the fix. If mDoTheFix is false, the app will exhibit the broken behavior.
public class MainActivity extends AppCompatActivity implements View.OnClickListener {
private RecyclerView recyclerView;
private List<Data> datas = new ArrayList<>();
private Adapter adapter;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
recyclerView = findViewById(R.id.recycler_view);
StaggeredGridLayoutManager staggeredGridLayoutManager =
new StaggeredGridLayoutManager(2, StaggeredGridLayoutManager.VERTICAL) {
@Override
public void onLayoutCompleted(RecyclerView.State state) {
// super.onLayoutCompleted(state);
}
};
// recyclerView.setItemAnimator(null);
// staggeredGridLayoutManager.setGapStrategy(StaggeredGridLayoutManager.GAP_HANDLING_NONE);
this.recyclerView.setLayoutManager(staggeredGridLayoutManager);
datas.add(new Data(0, "A title", "A body"));
datas.add(new Data(1, "B title", "B body"));
datas.add(new Data(2, "C title", "C body"));
datas.add(new Data(3, "D title", "D body"));
datas.add(new Data(4, "E title", "E body"));
datas.add(new Data(5, "F title", "F body"));
datas.add(new Data(6, "G title", "G body"));
datas.add(new Data(7, "H title", "H body"));
datas.add(new Data(8, "I title", "I body"));
datas.add(new Data(9, "J title", "J body"));
datas.add(new Data(10, "K title", "K body"));
datas.add(new Data(11, "L title", "L body"));
datas.add(new Data(12, "M title", "M body"));
// datas.add(new Data(13, "N title", "N body"));
// datas.add(new Data(14, "O title", "O body"));
// datas.add(new Data(11, "P title", "P body"));
// datas.add(new Data(12, "Q title", "Q body"));
// datas.add(new Data(13, "R title", "R body"));
// datas.add(new Data(14, "S title", "S body"));
adapter = new Adapter(datas, this);
recyclerView.setAdapter(adapter);
}
@Override
public boolean onCreateOptionsMenu(Menu menu) {
MenuInflater inflater = getMenuInflater();
inflater.inflate(R.menu.menu, menu);
return true;
}
@Override
public boolean onOptionsItemSelected(MenuItem item) {
// Handle item selection
switch (item.getItemId()) {
case R.id.debug: {
String s = (String) ((TextView) (((LinearLayout) ((FrameLayout) recyclerView.getChildAt(0)).getChildAt(0)).getChildAt(0)))
.getText();
Data data0 = datas.get(0);
Data data1 = datas.get(1);
datas.set(0, data1);
datas.set(1, data0);
new MyListUpdateCallback().onMoved(1, 0);
return true;
}
case R.id.debug2:
Data data2 = datas.get(2);
Data data3 = datas.get(3);
datas.set(2, data3);
datas.set(3, data2);
adapter.notifyItemMoved(2, 3);
return true;
case R.id.debug3: {
List<Data> oldDatas = new ArrayList<>(datas);
Data data0 = datas.get(0);
Data data1 = datas.get(1);
datas.set(0, data1);
datas.set(1, data0);
MyNoteDiffUtilCallback noteDiffUtilCallback = new MyNoteDiffUtilCallback(datas, oldDatas);
DiffUtil.calculateDiff(noteDiffUtilCallback).dispatchUpdatesTo(new MyListUpdateCallback());
return true;
}
case R.id.debug4:
String ABC = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
int i = datas.size();
if (i < 26) {
String ch = ABC.substring(i, i + 1);
datas.add(new Data(12, ch + " title", ch + " body"));
adapter.notifyItemInserted(i - 1);
}
return true;
case R.id.debug5:
int toRemove = datas.size() - 1;
datas.remove(toRemove);
adapter.notifyItemRemoved(toRemove);
return true;
default:
return super.onOptionsItemSelected(item);
}
}
@Override
public void onClick(View v) {
int oldPos = recyclerView.getLayoutManager().getPosition(v);
int newPos = oldPos - 1;
if (newPos < 0) {
return;
}
Data data0 = datas.get(oldPos);
Data data1 = datas.get(newPos);
datas.set(oldPos, data1);
datas.set(newPos, data0);
new MyListUpdateCallback().onMoved(oldPos, newPos);
}
public class MyNoteDiffUtilCallback extends DiffUtil.Callback {
private List<Data> newsDatas;
private List<Data> oldDatas;
public MyNoteDiffUtilCallback(List<Data> newsDatas, List<Data> oldDatas) {
this.newsDatas = newsDatas;
this.oldDatas = oldDatas;
}
@Override
public int getOldListSize() {
return oldDatas.size();
}
@Override
public int getNewListSize() {
return newsDatas.size();
}
@Override
public boolean areItemsTheSame(int oldItemPosition, int newItemPosition) {
return oldDatas.get(oldItemPosition).id == newsDatas.get(newItemPosition).id;
}
@Override
public boolean areContentsTheSame(int oldItemPosition, int newItemPosition) {
return oldDatas.get(oldItemPosition).equals(newsDatas.get(newItemPosition));
}
}
boolean mDoTheFix = true;
private class MyListUpdateCallback implements ListUpdateCallback {
@Override
public void onMoved(int fromPosition, int toPosition) {
if (mDoTheFix && toPosition == 0) {
adapter.notifyItemChanged(fromPosition);
adapter.notifyItemChanged(toPosition);
} else {
adapter.notifyItemMoved(fromPosition, toPosition);
}
}
public void onInserted(int position, int count) {
adapter.notifyItemRangeInserted(position, count);
}
@Override
public void onRemoved(int position, int count) {
adapter.notifyItemRangeRemoved(position, count);
}
@Override
public void onChanged(int position, int count, Object payload) {
adapter.notifyItemRangeChanged(position, count, payload);
}
}
}
menu.xml
<menu xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:myApp="http://schemas.android.com/apk/res-auto">
<item android:id="@+id/debug"
android:title="0->1"
myApp:showAsAction="always" />
<item android:id="@+id/debug2"
android:title="2->3"
myApp:showAsAction="always" />
<item android:id="@+id/debug3"
android:title="DiffUtil"
myApp:showAsAction="always" />
<item android:id="@+id/debug4"
android:title="Add"
myApp:showAsAction="always" />
<item android:id="@+id/debug5"
android:title="Del"
myApp:showAsAction="always" />
</menu>
I did some analysis for the StaggeredGridLayoutManager by scanning grep code. Looks like there is an issue with calculation when itemMoved is at index 0. You can refer this method which will be called when notifyItemMoved() is called implicitly by the RecyclerView's adapter.
It works for other layouts because, other layouts simply call invalidateSpanCache() to perform correct recalculation in next layout which will be cased by notifyItemMoved(), since its a structural change event.
So the solution/workaround which I found are as follows, which only focuses on dealing with position movement of index 0:
Option 1: Call notifyDataSetChanged
For your case, in order to enforce StaggeredGridLayoutManager to react correctly for item moved at index 0, you need to enforce the StaggeredGridLayoutManager fully rebind and relayout all visible views. This can be achieved by calling notifyDataSetChanged() as follows:
case R.id.debug: {
Data data0 = datas.get(0);
Data data1 = datas.get(1);
datas.set(0, data1);
datas.set(1, data0);
adapter.notifyDataSetChanged();
return true;
}
This triggers your expected animation. But the drawback is this operation can be expensive when you are dealing with huge data
Option 2: Call onItemMoved directly on layout
This method somehow performs correct calculation and views are not messed up. You need to maintain reference for the layout object:
case R.id.debug:{
Data data0=datas.get(0);
Data data1=datas.get(1);
datas.set(0,data1);
datas.set(1,data0);
staggeredGridLayoutManager.requestSimpleAnimationsInNextLayout();
staggeredGridLayoutManager.onItemsMoved(recyclerView,0,1,1);
return true;
}
Drawback here is, it applies different animation (fade in/fade out). This will also not call any data change observers since StaggeredGridLayoutManager doesnot propogate the call to RecyclerView. You might need to create your animation to have same effect.
Option 3: Notify notifyItemChanged
This is the option you don't like for obvious valid reasons:
case R.id.debug: {
Collections.swap(datas, 0, 1);
adapter.notifyItemChanged(0);
adapter.notifyItemChanged(1);
return true;
}
To conclude, based on the nature of code, unclear documentation and nature of usage StaggeredGridLayoutManager development looks to be in inchoate stage and would mature in near future. A good solution will only be possible, when the issues with the layout has been fixed. Till then we have to use some workarounds with different trade offs.
i tried that code and it works great
(instead of using notifyItemMoved I used notifyItemRangeChanged that is the correct notify function that you need)
you can see it also here https://jumpshare.com/v/fQklba8yFs4YTqkxoyMH
@Override
public boolean onOptionsItemSelected(MenuItem item) {
// Handle item selection
switch (item.getItemId()) {
case R.id.debug:
Data data0 = datas.get(0);
Data data1 = datas.get(1);
datas.set(0, data1);
datas.set(1, data0);
adapter.notifyItemRangeChanged(0,2);
return true;
case R.id.debug2:
Data data2 = datas.get(2);
Data data3 = datas.get(3);
datas.set(2, data3);
datas.set(3, data2);
adapter.notifyItemRangeChanged(2,2);
return true;
default:
return super.onOptionsItemSelected(item);
}
}
Update
it is wrong to use notifyItemMoved for swap.
You have to use 2 notifyItemChanged or a notifyItemRangeChanged or for all the dataset.
2nd working code
@Override
public boolean onOptionsItemSelected(MenuItem item) {
// Handle item selection
switch (item.getItemId()) {
case R.id.debug:
Data data0 = datas.get(0);
Data data1 = datas.get(1);
datas.set(0, data1);
datas.set(1, data0);
adapter.notifyItemChanged(0);
adapter.notifyItemChanged(1);
return true;
case R.id.debug2:
Data data2 = datas.get(2);
Data data3 = datas.get(3);
datas.set(2, data3);
datas.set(3, data2);
adapter.notifyItemChanged(2);
adapter.notifyItemChanged(3);
return true;
default:
return super.onOptionsItemSelected(item);
}
}