status-im
/
react-native
mirror of https://github.com/status-im/react-native.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix view indices with Android LayoutAnimation (attempt 2) (#19775) 

Summary:
/cc janicduplessis mdvacca

This addresses the same issue as #18830 which was reverted since it didn’t handle `removeClippedSubviews` properly.

When subview clipping is on, ReactViewGroup keeps track of its own children directly and accounts for the offset introduced by clipped views when calling ViewGroup methods that modify children.

Instead of accounting for just clipped children (views with no parent), it should account for any children that aren’t in the ViewGroup which also includes children that are being transitioned. If you look at the ViewGroup source code, [it explicitly retains the view parent until the transition finishes](https://github.com/aosp-mirror/platform_frameworks_base/blob/oreo-release/core/java/android/view/ViewGroup.java#L5034-L5036) which caused the `getParent()` checks to pass, even though those views should be ignored. I added a new `isChildInViewGroup` method that handles both clipped and transitioning views to fix this.

I reproduced the [earlier crash](https://github.com/facebook/react-native/pull/18830#issuecomment-382798628) by enabling clipping in [this test app](https://github.com/facebook/react-native/pull/18830#pullrequestreview-111758886) and adding a “clear views” button that resets the state to an empty items array with an animation.

- #18830

[ANDROID] [BUGFIX] [LayoutAnimation] - Removal LayoutAnimations no longer remove adjacent views as well in certain cases.
Pull Request resolved: https://github.com/facebook/react-native/pull/19775

Differential Revision: D9105838

Pulled By: hramos

fbshipit-source-id: 5ccb0957d1f46c36add960c0e4ef2a545cb03cbe
This commit is contained in:
Leo Nikkilä 2018-08-02 06:49:14 -07:00 committed by Facebook Github Bot
parent cbfe159d44
commit 1f88a7111d
4 changed files with 85 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java
View File

@ -409,12 +409,13 @@ public class NativeViewHierarchyManager {
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 class NativeViewHierarchyManager {
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);
}
});

8
ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java
View File

@ -93,6 +93,14 @@ public abstract class ViewGroupManager <T extends ViewGroup>
}
}
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.

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

@ -39,6 +39,8 @@ 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;
/**
@ -106,6 +108,7 @@ public class ReactViewGroup extends ViewGroup implements
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 class ReactViewGroup extends ViewGroup implements
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 @@ 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 - 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 @@ public class ReactViewGroup extends ViewGroup implements
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 @@ public class ReactViewGroup extends ViewGroup implements
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 @@ public class ReactViewGroup extends ViewGroup implements
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 @@ 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);

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

@ -297,4 +297,24 @@ public class ReactViewManager extends ViewGroupManager<ReactViewGroup> {
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);
}
}
}