Skip to content

Commit

Permalink
RN: Revert D8666509
Browse files Browse the repository at this point in the history
Summary:
Reverts D8666509. Unfortunately, I misunderstood `setClipChildren` on Android.

When set on a `ViewGroup`, `setClipChildren` configures whether its //children// — not itself — are clipped to their bounds. This is unlike `overflow` (as it behaves on iOS) which configures whether the view itself is clipped to its bounds.

But they are definitely related. In theory, I think we could implement `overflow` using `setClipChildren` by:

- Setting `setClipChildren(false)` by default. (This part, I got right.)
- When `overflow` is set to `hidden` on a `View`, we create an extra `ViewGroup` (child) within the normal `ViewGroup` (parent). Then, we can set `setClipChildren(true)` on the parent `ViewGroup` which will cause the child `ViewGroup` to be clipped to its bounds.

However, I think the tricky thing will be to create the child `ViewGroup` without incurring unintentional side effects.

I need to decide whether or not this is worth trying. The alternative is to add a new `clipChildren` boolean prop that is Android-only, but I really hate further bifurcating the platform. But for now, I am reverting my mistake.

Reviewed By: achen1

Differential Revision: D8690551

fbshipit-source-id: 1ba3bbcc5458ffbd5c475430ea0382b3fd0916b2
  • Loading branch information
yungsters authored and facebook-github-bot committed Jun 29, 2018
1 parent d9fa1d7 commit f090840
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ public static boolean isLayoutOnly(ReadableMap map, String prop) {
return map.isNull(BORDER_RIGHT_WIDTH) || map.getDouble(BORDER_RIGHT_WIDTH) == 0d;
case BORDER_BOTTOM_WIDTH:
return map.isNull(BORDER_BOTTOM_WIDTH) || map.getDouble(BORDER_BOTTOM_WIDTH) == 0d;
case OVERFLOW:
return map.isNull(OVERFLOW) || map.getString(OVERFLOW) == "visible";
case OVERFLOW: // We do nothing with this right now.
return true;
default:
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ public void onLayoutChange(

public ReactViewGroup(Context context) {
super(context);
setClipChildren(false);
mDrawingOrderHelper = new ViewGroupDrawingOrderHelper(this);
}

Expand Down Expand Up @@ -639,7 +638,6 @@ public void setHitSlopRect(@Nullable Rect rect) {
}

public void setOverflow(String overflow) {
setClipChildren(mOverflow == "hidden");
mOverflow = overflow;
invalidate();
}
Expand Down

0 comments on commit f090840

Please sign in to comment.