-
Notifications
You must be signed in to change notification settings - Fork 24.7k
Improve z-index implementation on Android #13105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
64806c6
a116aed
70d3d9e
fc4e614
b26b236
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
package com.facebook.react.uimanager; | ||
|
||
import android.view.View; | ||
import android.view.ViewGroup; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.Comparator; | ||
|
||
import javax.annotation.Nullable; | ||
|
||
/** | ||
* Helper to handle implementing ViewGroups with custom drawing order based on z-index. | ||
*/ | ||
public class ViewGroupDrawingOrderHelper { | ||
private final ViewGroup mViewGroup; | ||
private int mNumberOfChildrenWithZIndex = 0; | ||
private @Nullable int[] mDrawingOrderIndices; | ||
|
||
public ViewGroupDrawingOrderHelper(ViewGroup viewGroup) { | ||
mViewGroup = viewGroup; | ||
} | ||
|
||
/** | ||
* This should be called every time a view is added to the ViewGroup in {@link ViewGroup#addView}. | ||
* @param view The view that is being added | ||
*/ | ||
public void handleAddView(View view) { | ||
if (ViewGroupManager.getViewZIndex(view) != null) { | ||
mNumberOfChildrenWithZIndex++; | ||
} | ||
|
||
mDrawingOrderIndices = null; | ||
} | ||
|
||
/** | ||
* This should be called every time a view is removed from the ViewGroup in {@link ViewGroup#removeView} | ||
* and {@link ViewGroup#removeViewAt}. | ||
* @param view The view that is being removed. | ||
*/ | ||
public void handleRemoveView(View view) { | ||
if (ViewGroupManager.getViewZIndex(view) != null) { | ||
mNumberOfChildrenWithZIndex--; | ||
} | ||
|
||
mDrawingOrderIndices = null; | ||
} | ||
|
||
/** | ||
* If the ViewGroup should enable drawing order. ViewGroups should call | ||
* {@link ViewGroup#setChildrenDrawingOrderEnabled} with the value returned from this method when | ||
* a view is added or removed. | ||
*/ | ||
public boolean shouldEnableCustomDrawingOrder() { | ||
return mNumberOfChildrenWithZIndex > 0; | ||
} | ||
|
||
/** | ||
* The index of the child view that should be drawn. This should be used in | ||
* {@link ViewGroup#getChildDrawingOrder}. | ||
*/ | ||
public int getChildDrawingOrder(int childCount, int index) { | ||
if (mDrawingOrderIndices == null) { | ||
ArrayList<View> viewsToSort = new ArrayList<>(); | ||
for (int i = 0; i < childCount; i++) { | ||
viewsToSort.add(mViewGroup.getChildAt(i)); | ||
} | ||
// Sort the views by zIndex | ||
Collections.sort(viewsToSort, new Comparator<View>() { | ||
@Override | ||
public int compare(View view1, View view2) { | ||
Integer view1ZIndex = ViewGroupManager.getViewZIndex(view1); | ||
if (view1ZIndex == null) { | ||
view1ZIndex = 0; | ||
} | ||
|
||
Integer view2ZIndex = ViewGroupManager.getViewZIndex(view2); | ||
if (view2ZIndex == null) { | ||
view2ZIndex = 0; | ||
} | ||
|
||
return view1ZIndex - view2ZIndex; | ||
} | ||
}); | ||
|
||
mDrawingOrderIndices = new int[childCount]; | ||
for (int i = 0; i < childCount; i++) { | ||
View child = viewsToSort.get(i); | ||
mDrawingOrderIndices[i] = mViewGroup.indexOfChild(child); | ||
} | ||
} | ||
return mDrawingOrderIndices[index]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,13 +19,15 @@ | |
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> { | ||
|
||
public static WeakHashMap<View, Integer> mZIndexHash = new WeakHashMap<>(); | ||
private static WeakHashMap<View, Integer> mZIndexHash = new WeakHashMap<>(); | ||
|
||
@Override | ||
public LayoutShadowNode createShadowNodeInstance() { | ||
|
@@ -43,7 +45,6 @@ public void updateExtraData(T root, Object extraData) { | |
|
||
public void addView(T parent, View child, int index) { | ||
parent.addView(child, index); | ||
reorderChildrenByZIndex(parent); | ||
} | ||
|
||
/** | ||
|
@@ -61,54 +62,10 @@ public void addViews(T parent, List<View> views) { | |
|
||
public static void setViewZIndex(View view, int zIndex) { | ||
mZIndexHash.put(view, zIndex); | ||
// zIndex prop gets set BEFORE the view is added, so parent may be null. | ||
ViewGroup parent = (ViewGroup) view.getParent(); | ||
if (parent != null) { | ||
reorderChildrenByZIndex(parent); | ||
} | ||
} | ||
|
||
public static void reorderChildrenByZIndex(ViewGroup view) { | ||
// Optimization: loop through the zIndexHash to test if there are any non-zero zIndexes | ||
// If there aren't any, we can just return out | ||
Collection<Integer> zIndexes = mZIndexHash.values(); | ||
boolean containsZIndexedElement = false; | ||
for (Integer zIndex : zIndexes) { | ||
if (zIndex != 0) { | ||
containsZIndexedElement = true; | ||
break; | ||
} | ||
} | ||
if (!containsZIndexedElement) { | ||
return; | ||
} | ||
|
||
// Add all children to a sortable ArrayList | ||
ArrayList<View> viewsToSort = new ArrayList<>(); | ||
for (int i = 0; i < view.getChildCount(); i++) { | ||
viewsToSort.add(view.getChildAt(i)); | ||
} | ||
// Sort the views by zIndex | ||
Collections.sort(viewsToSort, new Comparator<View>() { | ||
@Override | ||
public int compare(View view1, View view2) { | ||
Integer view1ZIndex = mZIndexHash.get(view1); | ||
if (view1ZIndex == null) { | ||
view1ZIndex = 0; | ||
} | ||
|
||
Integer view2ZIndex = mZIndexHash.get(view2); | ||
if (view2ZIndex == null) { | ||
view2ZIndex = 0; | ||
} | ||
return view1ZIndex - view2ZIndex; | ||
} | ||
}); | ||
// Call .bringToFront on the sorted list of views | ||
for (int i = 0; i < viewsToSort.size(); i++) { | ||
viewsToSort.get(i).bringToFront(); | ||
} | ||
view.invalidate(); | ||
public static @Nullable Integer getViewZIndex(View view) { | ||
return mZIndexHash.get(view); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: return 0 if null There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now I'm using null as a way to know that the view doesn't have z-index set so we can avoid using custom view drawing when no child has z-index set. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another option is to change the increment/decrement condition to |
||
} | ||
|
||
public int getChildCount(T parent) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is causing the clipped subviews tests to fail. is this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added in #11315 as a workaround for bugs with the old z-index implementation. Tested that it isn't needed anymore, maybe the test failure in unrelated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be
removeClippedSubviews={this.props.removeClippedSubviews}
otherwise ScrollView will never remove clipped subviewsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I should have actually checked the diff -_- https://github.com/facebook/react-native/pull/11315/files#diff-f8f8b220fc0d1573e4f8b78c84415075L478
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janicduplessis

stickySectionHeader on
ListView
is not work as expected after this change on both master branch and 0.45.0-rc.0, while it's OK onSectionList
You can see the sticked sectionHeader is overlapped by the others, works perfectly after reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've actually hit a few issues with sticky sections headers when updating Expo to 0.44. I've fixed then and will make some PRs soon. Hopefuly it also fixes your issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janicduplessis Great, BTW, can you help to review #13885