From 8203cc29d012367ca6aa97660a52f0d743104164 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Leo=20Nikkil=C3=A4?= <hello@lnikki.la>
Date: Thu, 12 Apr 2018 20:18:58 +0300
Subject: [PATCH] Fix view indices with Android LayoutAnimation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes issue #11828 that causes layout animations for removed views to
remove some adjacent views as well. This happens because the animated
views are still present in the ViewGroup, which throws off subsequent
operations that rely on view indices having updated.

This issue was addressed in #11962, which was closed in favour of a more
reliable solution that addresses the issue globally since it’s difficult
to account for animated views everywhere. @janicduplessis recommended[0]
handling the issue through ViewManager.

Since API 11, Android provides `ViewGroup#startViewTransition(View)`
that can be used to keep child views visible even if they have been
removed from the group. ViewGroup keeps an array of these views, which
is only used for drawing. Methods such as `ViewGroup#getChildCount()`
and `ViewGroup#getChildAt(int)` will ignore them.

I believe relying on these framework methods within ViewManager is the
most reliable way to solve this issue because it also works if callers
ignore ViewManager and reach into the native view indices and counts
directly.

[0]: https://github.com/facebook/react-native/pull/11962#pullrequestreview-21862640
---
 .../react/uimanager/NativeViewHierarchyManager.java | 13 ++++++++-----
 .../facebook/react/uimanager/ViewGroupManager.java  |  8 ++++++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java
index a57205b861ebe2..2b72a0a3333088 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java
@@ -373,12 +373,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;
       }
     }
@@ -423,7 +424,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);
             }
           });
diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java
index 017fb5764e7237..c4d5eed429dde7 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java
+++ b/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.