Fix LayoutAnimation iOS delete bug when adding and removing views at …

…the same time

Summary:
Fix a bug that happens when views are added and removed in the same manageChildren block with a delete 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.

To do this I separated `_removeChildren` into 2 different functions, one for animated delete and one for normal delete. The animated one returns an array of `RCTComponent` that are going to be animated. We can then use this array to offset the insert index.

**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.
Closes facebook#7942

Differential Revision: D3417194

Pulled By: nicklockwood

fbshipit-source-id: 790f4ac15a8552323b359e6466cecfa80418c63c

React/Modules/RCTUIManager.m

@@ -210,6 +210,7 @@ @implementation RCTUIManager
 
   // Animation
   RCTLayoutAnimation *_layoutAnimation; // Main thread only
+  NSMutableSet<UIView *> *_viewsToBeDeleted; // Main thread only
 
   NSMutableDictionary<NSNumber *, RCTShadowView *> *_shadowViewRegistry; // RCT thread only
   NSMutableDictionary<NSNumber *, UIView *> *_viewRegistry; // Main thread only
@@ -318,6 +319,8 @@ - (void)setBridge:(RCTBridge *)bridge
 
   _bridgeTransactionListeners = [NSMutableSet new];
 
+  _viewsToBeDeleted = [NSMutableSet new];
+
   // Get view managers from bridge
   NSMutableDictionary *componentDataByName = [NSMutableDictionary new];
   for (Class moduleClass in _bridge.moduleClasses) {
@@ -339,7 +342,6 @@ - (void)setBridge:(RCTBridge *)bridge
                                            selector:@selector(interfaceOrientationWillChange:)
                                                name:UIApplicationWillChangeStatusBarOrientationNotification
                                              object:nil];
-
   [RCTAnimation initializeStatics];
 }
 
@@ -778,53 +780,61 @@ - (void)_amendPendingUIBlocksWithStylePropagationUpdateForShadowView:(RCTShadowV
 
 - (void)_removeChildren:(NSArray<id<RCTComponent>> *)children
           fromContainer:(id<RCTComponent>)container
-              permanent:(BOOL)permanent
 {
-  RCTLayoutAnimation *layoutAnimation = _layoutAnimation;
-  RCTAnimation *deleteAnimation = layoutAnimation.deleteAnimation;
+  for (id<RCTComponent> removedChild in children) {
+    [container removeReactSubview:removedChild];
+  }
+}
 
-  __block NSUInteger completionsCalled = 0;
+/**
+ * Remove subviews from their parent with an animation.
+ */
+- (void)_removeChildren:(NSArray<UIView *> *)children
+          fromContainer:(UIView *)container
+          withAnimation:(RCTLayoutAnimation *)animation
+{
+  RCTAssertMainQueue();
+  RCTAnimation *deleteAnimation = animation.deleteAnimation;
 
-  for (id<RCTComponent> removedChild in children) {
+  __block NSUInteger completionsCalled = 0;
+  for (UIView *removedChild in children) {
 
     void (^completion)(BOOL) = ^(BOOL finished) {
       completionsCalled++;
 
+      [_viewsToBeDeleted removeObject:removedChild];
       [container removeReactSubview:removedChild];
 
-      if (layoutAnimation.callback && completionsCalled == children.count) {
-        layoutAnimation.callback(@[@(finished)]);
+      if (animation.callback && completionsCalled == children.count) {
+        animation.callback(@[@(finished)]);
 
         // It's unsafe to call this callback more than once, so we nil it out here
         // to make sure that doesn't happen.
-        layoutAnimation.callback = nil;
+        animation.callback = nil;
       }
     };
 
-    if (permanent && deleteAnimation && [removedChild isKindOfClass: [UIView class]]) {
-      UIView *view = (UIView *)removedChild;
+    [_viewsToBeDeleted addObject:removedChild];
 
-      // Disable user interaction while the view is animating since JS won't receive
-      // the view events anyway.
-      view.userInteractionEnabled = NO;
+    // Disable user interaction while the view is animating since JS won't receive
+    // the view events anyway.
+    removedChild.userInteractionEnabled = NO;
 
-      NSString *property = deleteAnimation.property;
-      [deleteAnimation performAnimations:^{
-        if ([property isEqualToString:@"scaleXY"]) {
-          view.layer.transform = CATransform3DMakeScale(0, 0, 0);
-        } else if ([property isEqualToString:@"opacity"]) {
-          view.layer.opacity = 0.0;
-        } else {
-          RCTLogError(@"Unsupported layout animation createConfig property %@",
-                      deleteAnimation.property);
-        }
-      } withCompletionBlock:completion];
-    } else {
-      [container removeReactSubview:removedChild];
-    }
+    NSString *property = deleteAnimation.property;
+    [deleteAnimation performAnimations:^{
+      if ([property isEqualToString:@"scaleXY"]) {
+        removedChild.layer.transform = CATransform3DMakeScale(0, 0, 0);
+      } else if ([property isEqualToString:@"opacity"]) {
+        removedChild.layer.opacity = 0.0;
+      } else {
+        RCTLogError(@"Unsupported layout animation createConfig property %@",
+                    deleteAnimation.property);
+      }
+    } withCompletionBlock:completion];
   }
 }
 
+
 RCT_EXPORT_METHOD(removeRootView:(nonnull NSNumber *)rootReactTag)
 {
   RCTShadowView *rootShadowView = _shadowViewRegistry[rootReactTag];
@@ -938,12 +948,18 @@ - (void)_manageChildren:(NSNumber *)containerTag
     [self _childrenToRemoveFromContainer:container atIndices:removeAtIndices];
   NSArray<id<RCTComponent>> *temporarilyRemovedChildren =
     [self _childrenToRemoveFromContainer:container atIndices:moveFromIndices];
-  [self _removeChildren:permanentlyRemovedChildren fromContainer:container permanent:true];
-  [self _removeChildren:temporarilyRemovedChildren fromContainer:container permanent:false];
 
-  [self _purgeChildren:permanentlyRemovedChildren fromRegistry:registry];
+  BOOL isUIViewRegistry = ((id)registry == (id)_viewRegistry);
+  if (isUIViewRegistry && _layoutAnimation.deleteAnimation) {
+    [self _removeChildren:(NSArray<UIView *> *)permanentlyRemovedChildren
+            fromContainer:(UIView *)container
+            withAnimation:_layoutAnimation];
+  } else {
+    [self _removeChildren:permanentlyRemovedChildren fromContainer:container];
+  }
 
-  // TODO (#5906496): optimize all these loops - constantly calling array.count is not efficient
+  [self _removeChildren:temporarilyRemovedChildren fromContainer:container];
+  [self _purgeChildren:permanentlyRemovedChildren fromRegistry:registry];
 
   // Figure out what to insert - merge temporary inserts and adds
   NSMutableDictionary *destinationsToChildrenToAdd = [NSMutableDictionary dictionary];
@@ -960,8 +976,22 @@ - (void)_manageChildren:(NSNumber *)containerTag
   NSArray<NSNumber *> *sortedIndices =
     [destinationsToChildrenToAdd.allKeys sortedArrayUsingSelector:@selector(compare:)];
   for (NSNumber *reactIndex in sortedIndices) {
+    NSInteger insertAtIndex = reactIndex.integerValue;
+
+    // When performing a delete animation, views are not removed immediately
+    // from their container so we need to offset the insertion index if a view
+    // that will be removed appears earlier than the view we are inserting.
+    if (isUIViewRegistry && _viewsToBeDeleted.count > 0) {
+      for (NSInteger index = 0; index < insertAtIndex; index++) {
+        UIView *subview = ((UIView *)container).reactSubviews[index];
+        if ([_viewsToBeDeleted containsObject:subview]) {
+          insertAtIndex++;
+        }
+      }
+    }
+
     [container insertReactSubview:destinationsToChildrenToAdd[reactIndex]
-                          atIndex:reactIndex.integerValue];
+                          atIndex:insertAtIndex];
   }
 }