Skip to content

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Examples/UIExplorer/js/ListViewPagingExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class ListViewPagingExample extends React.Component {
initialListSize={10}
pageSize={4}
scrollRenderAheadDistance={500}
stickySectionHeadersEnabled
/>
);
}
Expand Down
6 changes: 5 additions & 1 deletion Examples/UIExplorer/js/SectionListExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const VIEWABILITY_CONFIG = {
};

const renderSectionHeader = ({section}) => (
<View>
<View style={styles.header}>
<Text style={styles.headerText}>SECTION HEADER: {section.key}</Text>
<SeparatorComponent />
</View>
Expand Down Expand Up @@ -144,6 +144,7 @@ class SectionListExample extends React.PureComponent {
refreshing={false}
renderItem={this._renderItemComponent}
renderSectionHeader={renderSectionHeader}
stickySectionHeadersEnabled
sections={[
{renderItem: renderStackedItem, key: 's1', data: [
{title: 'Item In Header Section', text: 'Section s1', key: '0'},
Expand Down Expand Up @@ -186,6 +187,9 @@ class SectionListExample extends React.PureComponent {
}

const styles = StyleSheet.create({
header: {
backgroundColor: '#e9eaed',
},
headerText: {
padding: 4,
},
Expand Down
4 changes: 1 addition & 3 deletions Libraries/Components/ScrollView/ScrollView.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,9 +650,7 @@ const ScrollView = React.createClass({
{...contentSizeChangeProps}
ref={this._setInnerViewRef}
style={contentContainerStyle}
removeClippedSubviews={
hasStickyHeaders && Platform.OS === 'android' ? false : this.props.removeClippedSubviews
}
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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 subviews

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@nihgwu nihgwu May 16, 2017

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 on SectionList
image
You can see the sticked sectionHeader is overlapped by the others, works perfectly after reverted

Copy link
Contributor Author

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.

Copy link
Contributor

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

removeClippedSubviews={this.props.removeClippedSubviews}
collapsable={false}>
{children}
</ScrollContentContainerViewClass>;
Expand Down
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
Expand Up @@ -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() {
Expand All @@ -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);
}

/**
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return 0 if null

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another option is to change the increment/decrement condition to ReactViewManager.getViewZIndex(view) != 0

}

public int getChildCount(T parent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.facebook.react.uimanager.ReactClippingViewGroup;
import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
import com.facebook.react.uimanager.ReactPointerEventsView;
import com.facebook.react.uimanager.ViewGroupDrawingOrderHelper;

/**
* Backing for a React View. Has support for borders, but since borders aren't common, lazy
Expand Down Expand Up @@ -96,9 +97,12 @@ public void onLayoutChange(
private @Nullable ReactViewBackgroundDrawable mReactBackgroundDrawable;
private @Nullable OnInterceptTouchEventListener mOnInterceptTouchEventListener;
private boolean mNeedsOffscreenAlphaCompositing = false;
private final ViewGroupDrawingOrderHelper mDrawingOrderHelper;

public ReactViewGroup(Context context) {
super(context);

mDrawingOrderHelper = new ViewGroupDrawingOrderHelper(this);
}

@Override
Expand Down Expand Up @@ -374,6 +378,36 @@ protected void onAttachedToWindow() {
}
}

@Override
public void addView(View child, int index, LayoutParams params) {
// This will get called for every overload of addView so there is not need to override every method.
mDrawingOrderHelper.handleAddView(child);
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());

super.addView(child, index, params);
}

@Override
public void removeView(View view) {
mDrawingOrderHelper.handleRemoveView(view);
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());

super.removeView(view);
}

@Override
public void removeViewAt(int index) {
mDrawingOrderHelper.handleRemoveView(getChildAt(index));
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());

super.removeViewAt(index);
}

@Override
protected int getChildDrawingOrder(int childCount, int index) {
return mDrawingOrderHelper.getChildDrawingOrder(childCount, index);
}

@Override
public PointerEvents getPointerEvents() {
return mPointerEvents;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ public void addView(ReactViewGroup parent, View child, int index) {
} else {
parent.addView(child, index);
}
reorderChildrenByZIndex(parent);
}

@Override
Expand Down