Fix view indices with Android LayoutAnimation (attempt 2)

ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java

@@ -409,12 +409,13 @@ public synchronized void manageChildren(
         if (mLayoutAnimationEnabled &&
             mLayoutAnimator.shouldAnimateLayout(viewToRemove) &&
             arrayContains(tagsToDelete, viewToRemove.getId())) {
-          // The view will be removed and dropped by the 'delete' layout animation
-          // instead, so do nothing
-        } else {
-          viewManager.removeViewAt(viewToManage, indexToRemove);
+          // Display the view in the parent after removal for the duration of the layout animation,
+          // but pretend that it doesn't exist when calling other ViewGroup methods.
+          viewManager.startViewTransition(viewToManage, viewToRemove);
         }
 
+        viewManager.removeViewAt(viewToManage, indexToRemove);
+
         lastIndexToRemove = indexToRemove;
       }
     }
@@ -459,7 +460,9 @@ public synchronized void manageChildren(
           mLayoutAnimator.deleteView(viewToDestroy, new LayoutAnimationListener() {
             @Override
             public void onAnimationEnd() {
-              viewManager.removeView(viewToManage, viewToDestroy);
+              // Already removed from the ViewGroup, we can just end the transition here to
+              // release the child.
+              viewManager.endViewTransition(viewToManage, viewToDestroy);
               dropView(viewToDestroy);
             }
           });

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java

@@ -93,6 +93,14 @@ public void removeAllViews(T parent) {
     }
   }
 
+  public void startViewTransition(T parent, View view) {
+    parent.startViewTransition(view);
+  }
+
+  public void endViewTransition(T parent, View view) {
+    parent.endViewTransition(view);
+  }
+
   /**
    * Returns whether this View type needs to handle laying out its own children instead of
    * deferring to the standard css-layout algorithm.

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java

@@ -39,6 +39,8 @@
 import com.facebook.react.uimanager.ViewGroupDrawingOrderHelper;
 import com.facebook.react.uimanager.ViewProps;
 import com.facebook.yoga.YogaConstants;
+import java.util.ArrayList;
+import java.util.List;
 import javax.annotation.Nullable;
 
 /**
@@ -106,6 +108,7 @@ public void onLayoutChange(
   private @Nullable ChildrenLayoutChangeListener mChildrenLayoutChangeListener;
   private @Nullable ReactViewBackgroundDrawable mReactBackgroundDrawable;
   private @Nullable OnInterceptTouchEventListener mOnInterceptTouchEventListener;
+  private @Nullable List<View> mTransitioningViews;
   private boolean mNeedsOffscreenAlphaCompositing = false;
   private final ViewGroupDrawingOrderHelper mDrawingOrderHelper;
   private @Nullable Path mPath;
@@ -334,16 +337,16 @@ public void updateClippingRect() {
 
   private void updateClippingToRect(Rect clippingRect) {
     Assertions.assertNotNull(mAllChildren);
-    int clippedSoFar = 0;
+    int childIndexOffset = 0;
     for (int i = 0; i < mAllChildrenCount; i++) {
-      updateSubviewClipStatus(clippingRect, i, clippedSoFar);
-      if (mAllChildren[i].getParent() == null) {
-        clippedSoFar++;
+      updateSubviewClipStatus(clippingRect, i, childIndexOffset);
+      if (!isChildInViewGroup(mAllChildren[i])) {
+        childIndexOffset++;
       }
     }
   }
 
-  private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFar) {
+  private void updateSubviewClipStatus(Rect clippingRect, int idx, int childIndexOffset) {
     View child = Assertions.assertNotNull(mAllChildren)[idx];
     sHelperRect.set(child.getLeft(), child.getTop(), child.getRight(), child.getBottom());
     boolean intersects = clippingRect
@@ -360,10 +363,10 @@ private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFa
     if (!intersects && child.getParent() != null && !isAnimating) {
       // We can try saving on invalidate call here as the view that we remove is out of visible area
       // therefore invalidation is not necessary.
-      super.removeViewsInLayout(idx - clippedSoFar, 1);
+      super.removeViewsInLayout(idx - childIndexOffset, 1);
       needUpdateClippingRecursive = true;
     } else if (intersects && child.getParent() == null) {
-      super.addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true);
+      super.addViewInLayout(child, idx - childIndexOffset, sDefaultLayoutParam, true);
       invalidate();
       needUpdateClippingRecursive = true;
     } else if (intersects) {
@@ -399,19 +402,25 @@ private void updateSubviewClipStatus(View subview) {
     boolean oldIntersects = (subview.getParent() != null);
 
     if (intersects != oldIntersects) {
-      int clippedSoFar = 0;
+      int childIndexOffset = 0;
       for (int i = 0; i < mAllChildrenCount; i++) {
         if (mAllChildren[i] == subview) {
-          updateSubviewClipStatus(mClippingRect, i, clippedSoFar);
+          updateSubviewClipStatus(mClippingRect, i, childIndexOffset);
           break;
         }
-        if (mAllChildren[i].getParent() == null) {
-          clippedSoFar++;
+        if (!isChildInViewGroup(mAllChildren[i])) {
+          childIndexOffset++;
         }
       }
     }
   }
 
+  private boolean isChildInViewGroup(View view) {
+    // A child is in the group if it's not clipped and it's not transitioning.
+    return view.getParent() != null
+      && (mTransitioningViews == null || !mTransitioningViews.contains(view));
+  }
+
   @Override
   protected void onSizeChanged(int w, int h, int oldw, int oldh) {
     super.onSizeChanged(w, h, oldw, oldh);
@@ -509,13 +518,13 @@ protected void dispatchSetPressed(boolean pressed) {
     addInArray(child, index);
     // we add view as "clipped" and then run {@link #updateSubviewClipStatus} to conditionally
     // attach it
-    int clippedSoFar = 0;
+    int childIndexOffset = 0;
     for (int i = 0; i < index; i++) {
-      if (mAllChildren[i].getParent() == null) {
-        clippedSoFar++;
+      if (!isChildInViewGroup(mAllChildren[i])) {
+        childIndexOffset++;
       }
     }
-    updateSubviewClipStatus(mClippingRect, index, clippedSoFar);
+    updateSubviewClipStatus(mClippingRect, index, childIndexOffset);
     child.addOnLayoutChangeListener(mChildrenLayoutChangeListener);
   }
 
@@ -525,14 +534,14 @@ protected void dispatchSetPressed(boolean pressed) {
     Assertions.assertNotNull(mAllChildren);
     view.removeOnLayoutChangeListener(mChildrenLayoutChangeListener);
     int index = indexOfChildInAllChildren(view);
-    if (mAllChildren[index].getParent() != null) {
-      int clippedSoFar = 0;
+    if (isChildInViewGroup(mAllChildren[index])) {
+      int childIndexOffset = 0;
       for (int i = 0; i < index; i++) {
-        if (mAllChildren[i].getParent() == null) {
-          clippedSoFar++;
+        if (!isChildInViewGroup(mAllChildren[i])) {
+          childIndexOffset++;
         }
       }
-      super.removeViewsInLayout(index - clippedSoFar, 1);
+      super.removeViewsInLayout(index - childIndexOffset, 1);
     }
     removeFromArray(index);
   }
@@ -547,6 +556,26 @@ protected void dispatchSetPressed(boolean pressed) {
     mAllChildrenCount = 0;
   }
 
+  /*package*/ void startViewTransitionWithSubviewClippingEnabled(View view) {
+    // We're mirroring ViewGroup's mTransitioningViews since when a transitioning child is removed,
+    // its parent is not set to null unlike a regular child. Normally this wouldn't be an issue as
+    // ViewGroup pretends the transitioning child doesn't exist when calling any methods that expose
+    // child views, but we keep track of our children directly when subview clipping is enabled and
+    // need to be aware of these.
+    if (mTransitioningViews == null) {
+      mTransitioningViews = new ArrayList<>();
+    }
+    mTransitioningViews.add(view);
+    startViewTransition(view);
+  }
+
+  /*package*/ void endViewTransitionWithSubviewClippingEnabled(View view) {
+    if (mTransitioningViews != null) {
+      mTransitioningViews.remove(view);
+    }
+    endViewTransition(view);
+  }
+
   private int indexOfChildInAllChildren(View child) {
     final int count = mAllChildrenCount;
     final View[] children = Assertions.assertNotNull(mAllChildren);

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java

@@ -297,4 +297,24 @@ public void removeAllViews(ReactViewGroup parent) {
       parent.removeAllViews();
     }
   }
+
+  @Override
+  public void startViewTransition(ReactViewGroup parent, View view) {
+    boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
+    if (removeClippedSubviews) {
+      parent.startViewTransitionWithSubviewClippingEnabled(view);
+    } else {
+      parent.startViewTransition(view);
+    }
+  }
+
+  @Override
+  public void endViewTransition(ReactViewGroup parent, View view) {
+    boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
+    if (removeClippedSubviews) {
+      parent.endViewTransitionWithSubviewClippingEnabled(view);
+    } else {
+      parent.endViewTransition(view);
+    }
+  }
 }