Fix LayoutAnimation Android bug when adding and removing views at the same time

[janicduplessis]

Similar to #7942 but for android

Fix a bug that happens when views are added during a delete layout animation. What happens is that the inserted view is not inserted at the proper index if the deleted view index is smaller than the inserted one. This is because the view is not immediately removed from the subviews array so we need to offset the insert index for each view that is going to be deleted with an animation and is before the inserted view.

Test plan (required)
Tested that this fixed the bug in an app where I noticed it, also tested the UIExplorer example to make sure LayoutAnimations still worked properly.

[ghost]

By analyzing the blame information on this pull request, we identified @dmmiller and @korDen to be potential reviewers.

[janicduplessis]

cc @astreet

[dmmiller]

This comment probably needs to be updated. Since do nothing now looks like doing something.

[astreet]

Kinda unfortunate we have to do this, but I don't see another easy way. Would be amazing if we could add an integration test for this case as well layout animation in general since it seems somewhat easy to break!

[janicduplessis]

I found other issues that this doesn't fix so I'll have to debug this more and find a better solution. We may want to consider disabling delete animations for now.

[astreet]

Oli committed a couple fixes for this, not sure if they're relevant to what you were thinking (I'm guessing not, but you might want to rebase?):

bb9ed2d
c0ac0d5

[janicduplessis]

Yes I saw those commit but the problem is that views get added at the wrong index when doing some more complex animations. I managed to fix some of the issues by removing views based on their tags instead of their index but this doesn't work for adding views. I'll have to figure out why the index passed to manageChildren are not good.

I'm using this modified UIExplorer example to reproduce this issue https://gist.github.com/janicduplessis/211d78c5408ab17dee7ad2e381648a48 and also increasing the LayoutAnimation duration to make it more obvious.

[ghost]

@janicduplessis do you have any updates for this pull request? It's been a while since the last update so wanted to check in and see if you've looked at the requested changes.

[robclouth]

Any news on this? I'm getting

com.facebook.react.uimanager.IllegalViewOperationException: Trying to remove a view index above child count 0 view tag: 999

When using LayoutAnimation on Android when removing and adding items to a wrapped row.

[robclouth]

Occasionally this too:

java.lang.IndexOutOfBoundsException: index=23 count=22
                                                            at android.view.ViewGroup.addInArray(ViewGroup.java:4027)
                                                            at android.view.ViewGroup.addViewInner(ViewGroup.java:3958)
                                                            at android.view.ViewGroup.addView(ViewGroup.java:3786)
                                                            at android.view.ViewGroup.addView(ViewGroup.java:3727)
                                                            at com.facebook.react.views.view.ReactViewManager.addView(ReactViewManager.java:196)
                                                            at com.facebook.react.views.view.ReactViewManager.addView(ReactViewManager.java:39)
                                                            at com.facebook.react.uimanager.NativeViewHierarchyManager.manageChildren(NativeViewHierarchyManager.java:387)

[hramos]

@janicduplessis still planning to ship this PR?

[janicduplessis]

I don't think I'll have time to work on this any time soon, sadly this PR didn't completely fix the issue and I couldn't figure out a way to have a proper fix.

[lacker]

OK, I am going to close this pull request because it seems like everyone is stalled, but if someone wants to continue working on this then please feel free to spin it back up again!