Files
react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java
T
Leo Nikkilä d8fcdb9bd7 Fix view indices with Android LayoutAnimation
Summary:
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

I wrote a minimal test app that you can find here:

<https://gist.github.com/lnikkila/87f3825442a5773f17ead433a810d53f>

The expected result is that the red and green squares disappear, a blue
one appears, and the black one stays in place. iOS has this behaviour,
but Android removes the black square as well.

We can see the bug with some breakpoint logging.

Without LayoutAnimation:

```
    NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0
    NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1
    NativeViewHierarchyManager: Removing indices [0] with tags [2]
    RootViewManager: Removing child view at index 0 with tag 2
    NativeViewHierarchyManager: Removing indices [1] with tags [4]
    RootViewManager: Removing child view at index 1 with tag 4
```

With LayoutAnimation tag 3 gets removed when it shouldn’t be:

```
    NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0
    NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1
    NativeViewHierarchyManager: Removing indices [0] with tags [2]
    NativeViewHierarchyManager: Removing indices [1] with tags [4]
->  RootViewManager: Removing child view at index 1 with tag 3
    RootViewManager: Removing child view at index 2 with tag 4

    (Animation listener kicks in here)

    RootViewManager: Removing child view at index 1 with tag 2
```

Here are some GIFs to compare, click to expand:

<details>
  <summary><b>Current master (iOS vs Android)</b></summary>
  <p></p>
  <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695108-06eb73a6-3e94-11e8-867a-b95d7f926ccd.gif" height="400" />
</details><p></p>

<details>
  <summary><b>With this patch (iOS vs Android, fixed)</b></summary>
  <p></p>
  <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695137-1090f782-3e94-11e8-94c8-ce33a5d7ebdb.gif" height="400" />
</details><p></p>

Previously addressed in #11962, which wasn’t merged.

Tangentially related to my other LayoutAnimation PR #18651.

No documentation changes needed.

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

Reviewed By: achen1

Differential Revision: D7612904

Pulled By: mdvacca

fbshipit-source-id: a04cf47ab80e0e813fa043125b1f907e212b1ad4
2018-04-16 14:39:19 -07:00

128 lines
3.6 KiB
Java

/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.uimanager;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.WeakHashMap;
import android.view.View;
import android.view.ViewGroup;
import javax.annotation.Nullable;
/**
* Class providing children management API for view managers of classes extending ViewGroup.
*/
public abstract class ViewGroupManager <T extends ViewGroup>
extends BaseViewManager<T, LayoutShadowNode> {
private static WeakHashMap<View, Integer> mZIndexHash = new WeakHashMap<>();
@Override
public LayoutShadowNode createShadowNodeInstance() {
return new LayoutShadowNode();
}
@Override
public Class<? extends LayoutShadowNode> getShadowNodeClass() {
return LayoutShadowNode.class;
}
@Override
public void updateExtraData(T root, Object extraData) {
}
public void addView(T parent, View child, int index) {
parent.addView(child, index);
}
/**
* Convenience method for batching a set of addView calls
* Note that this adds the views to the beginning of the ViewGroup
*
* @param parent the parent ViewGroup
* @param views the set of views to add
*/
public void addViews(T parent, List<View> views) {
for (int i = 0, size = views.size(); i < size; i++) {
addView(parent, views.get(i), i);
}
}
public static void setViewZIndex(View view, int zIndex) {
mZIndexHash.put(view, zIndex);
}
public static @Nullable Integer getViewZIndex(View view) {
return mZIndexHash.get(view);
}
public int getChildCount(T parent) {
return parent.getChildCount();
}
public View getChildAt(T parent, int index) {
return parent.getChildAt(index);
}
public void removeViewAt(T parent, int index) {
parent.removeViewAt(index);
}
public void removeView(T parent, View view) {
for (int i = 0; i < getChildCount(parent); i++) {
if (getChildAt(parent, i) == view) {
removeViewAt(parent, i);
break;
}
}
}
public void removeAllViews(T parent) {
for (int i = getChildCount(parent) - 1; i >= 0; i--) {
removeViewAt(parent, i);
}
}
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.
* Returns true for the layout to *not* be automatically invoked. Instead onLayout will be
* invoked as normal and it is the View instance's responsibility to properly call layout on its
* children.
* Returns false for the default behavior of automatically laying out children without going
* through the ViewGroup's onLayout method. In that case, onLayout for this View type must *not*
* call layout on its children.
*/
public boolean needsCustomLayoutForChildren() {
return false;
}
/**
* Returns whether or not this View type should promote its grandchildren as Views. This is an
* optimization for Scrollable containers when using Nodes, where instead of having one ViewGroup
* containing a large number of draw commands (and thus being more expensive in the case of
* an invalidate or re-draw), we split them up into several draw commands.
*/
public boolean shouldPromoteGrandchildren() {
return false;
}
}