From e8c7cb1c04c583219c99c13657d3038776e968c6 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Mon, 27 Aug 2018 20:13:18 -0700 Subject: [PATCH] Revert D9105838: [react-native][PR] Fix view indices with Android LayoutAnimation (attempt 2) Differential Revision: D9105838 Original commit changeset: 5ccb0957d1f4 fbshipit-source-id: f679eceac47c95d9138f1a91a77cc20a74e64e04 --- .../uimanager/NativeViewHierarchyManager.java | 13 ++-- .../react/uimanager/ViewGroupManager.java | 8 --- .../react/views/view/ReactViewGroup.java | 69 ++++++------------- .../react/views/view/ReactViewManager.java | 20 ------ 4 files changed, 25 insertions(+), 85 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 7d694e52c..3e7d56a5e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -409,13 +409,12 @@ public class NativeViewHierarchyManager { if (mLayoutAnimationEnabled && mLayoutAnimator.shouldAnimateLayout(viewToRemove) && arrayContains(tagsToDelete, viewToRemove.getId())) { - // 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); + // The view will be removed and dropped by the 'delete' layout animation + // instead, so do nothing + } else { + viewManager.removeViewAt(viewToManage, indexToRemove); } - viewManager.removeViewAt(viewToManage, indexToRemove); - lastIndexToRemove = indexToRemove; } } @@ -460,9 +459,7 @@ public class NativeViewHierarchyManager { mLayoutAnimator.deleteView(viewToDestroy, new LayoutAnimationListener() { @Override public void onAnimationEnd() { - // Already removed from the ViewGroup, we can just end the transition here to - // release the child. - viewManager.endViewTransition(viewToManage, viewToDestroy); + viewManager.removeView(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 c4d5eed42..017fb5764 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java @@ -93,14 +93,6 @@ public abstract class ViewGroupManager } } - 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. diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index 6cd9462ce..ce2fa9528 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -39,8 +39,6 @@ import com.facebook.react.uimanager.RootViewUtil; 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; /** @@ -108,7 +106,6 @@ public class ReactViewGroup extends ViewGroup implements private @Nullable ChildrenLayoutChangeListener mChildrenLayoutChangeListener; private @Nullable ReactViewBackgroundDrawable mReactBackgroundDrawable; private @Nullable OnInterceptTouchEventListener mOnInterceptTouchEventListener; - private @Nullable List mTransitioningViews; private boolean mNeedsOffscreenAlphaCompositing = false; private final ViewGroupDrawingOrderHelper mDrawingOrderHelper; private @Nullable Path mPath; @@ -337,16 +334,16 @@ public class ReactViewGroup extends ViewGroup implements private void updateClippingToRect(Rect clippingRect) { Assertions.assertNotNull(mAllChildren); - int childIndexOffset = 0; + int clippedSoFar = 0; for (int i = 0; i < mAllChildrenCount; i++) { - updateSubviewClipStatus(clippingRect, i, childIndexOffset); - if (!isChildInViewGroup(mAllChildren[i])) { - childIndexOffset++; + updateSubviewClipStatus(clippingRect, i, clippedSoFar); + if (mAllChildren[i].getParent() == null) { + clippedSoFar++; } } } - private void updateSubviewClipStatus(Rect clippingRect, int idx, int childIndexOffset) { + private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFar) { View child = Assertions.assertNotNull(mAllChildren)[idx]; sHelperRect.set(child.getLeft(), child.getTop(), child.getRight(), child.getBottom()); boolean intersects = clippingRect @@ -363,10 +360,10 @@ public class ReactViewGroup extends ViewGroup implements 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 - childIndexOffset, 1); + super.removeViewsInLayout(idx - clippedSoFar, 1); needUpdateClippingRecursive = true; } else if (intersects && child.getParent() == null) { - super.addViewInLayout(child, idx - childIndexOffset, sDefaultLayoutParam, true); + super.addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true); invalidate(); needUpdateClippingRecursive = true; } else if (intersects) { @@ -402,25 +399,19 @@ public class ReactViewGroup extends ViewGroup implements boolean oldIntersects = (subview.getParent() != null); if (intersects != oldIntersects) { - int childIndexOffset = 0; + int clippedSoFar = 0; for (int i = 0; i < mAllChildrenCount; i++) { if (mAllChildren[i] == subview) { - updateSubviewClipStatus(mClippingRect, i, childIndexOffset); + updateSubviewClipStatus(mClippingRect, i, clippedSoFar); break; } - if (!isChildInViewGroup(mAllChildren[i])) { - childIndexOffset++; + if (mAllChildren[i].getParent() == null) { + clippedSoFar++; } } } } - 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); @@ -518,13 +509,13 @@ public class ReactViewGroup extends ViewGroup implements addInArray(child, index); // we add view as "clipped" and then run {@link #updateSubviewClipStatus} to conditionally // attach it - int childIndexOffset = 0; + int clippedSoFar = 0; for (int i = 0; i < index; i++) { - if (!isChildInViewGroup(mAllChildren[i])) { - childIndexOffset++; + if (mAllChildren[i].getParent() == null) { + clippedSoFar++; } } - updateSubviewClipStatus(mClippingRect, index, childIndexOffset); + updateSubviewClipStatus(mClippingRect, index, clippedSoFar); child.addOnLayoutChangeListener(mChildrenLayoutChangeListener); } @@ -534,14 +525,14 @@ public class ReactViewGroup extends ViewGroup implements Assertions.assertNotNull(mAllChildren); view.removeOnLayoutChangeListener(mChildrenLayoutChangeListener); int index = indexOfChildInAllChildren(view); - if (isChildInViewGroup(mAllChildren[index])) { - int childIndexOffset = 0; + if (mAllChildren[index].getParent() != null) { + int clippedSoFar = 0; for (int i = 0; i < index; i++) { - if (!isChildInViewGroup(mAllChildren[i])) { - childIndexOffset++; + if (mAllChildren[i].getParent() == null) { + clippedSoFar++; } } - super.removeViewsInLayout(index - childIndexOffset, 1); + super.removeViewsInLayout(index - clippedSoFar, 1); } removeFromArray(index); } @@ -556,26 +547,6 @@ public class ReactViewGroup extends ViewGroup implements 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); diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java index 31ff9d075..b4eb58704 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java @@ -297,24 +297,4 @@ public class ReactViewManager extends ViewGroupManager { 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); - } - } }