From f090840f452a0fb8ddb9d7cdd657c419ca627c1c Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Fri, 29 Jun 2018 12:06:54 -0700 Subject: [PATCH] RN: Revert D8666509 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/main/java/com/facebook/react/uimanager/ViewProps.java | 4 ++-- .../java/com/facebook/react/views/view/ReactViewGroup.java | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java index 15ea0c6d40e..f85236d4842 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java @@ -254,8 +254,8 @@ public class ViewProps { 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; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index 74b14bae8cc..1915b6eb48f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -112,7 +112,6 @@ public class ReactViewGroup extends ViewGroup implements public ReactViewGroup(Context context) { super(context); - setClipChildren(false); mDrawingOrderHelper = new ViewGroupDrawingOrderHelper(this); } @@ -639,7 +638,6 @@ public class ReactViewGroup extends ViewGroup implements } public void setOverflow(String overflow) { - setClipChildren(mOverflow == "hidden"); mOverflow = overflow; invalidate(); }