From 475f1b57967e44d067223131fe920ea98f97ea09 Mon Sep 17 00:00:00 2001 From: Jeff Chien Date: Sun, 24 Apr 2016 15:44:17 -0700 Subject: [PATCH] Reverted commit D3126513 Summary:Using TouchableNativeFeedback has been a problem for me because the ripples it makes don't follow the child view's border radii so the ripples stick out of the child view's rounded corners. This PR should fix this problem with a minor caveat: this only works for TouchableNativeFeedback.Ripple and not TouchableNativeFeedback.SelectableBackground. I searched how I could apply corner radius to selectableItemBackground and it doesn't seem to be possible (the prevalent advice is to create the ripple manually which is equivalent to using TNF.Ripple in our case), though I could be wrong. I added [an example to UIExplorer (TouchableExample)](http://i.imgur.com/CHY9xjW.png). This is my first PR to this repo so let me know if something's wrong. Cheers! Closes https://github.com/facebook/react-native/pull/6515 Differential Revision: D3126513 Pulled By: AaaChiuuu fb-gh-sync-id: 4a00e7177ee4ffd8dffeca143f4f43f08c99b5a1 fbshipit-source-id: 4a00e7177ee4ffd8dffeca143f4f43f08c99b5a1 --- Examples/UIExplorer/TouchableExample.js | 33 ------------ .../TouchableNativeFeedback.android.js | 6 --- .../react/views/view/ReactDrawableHelper.java | 18 ++----- .../view/ReactViewBackgroundDrawable.java | 54 +++++++++---------- .../react/views/view/ReactViewGroup.java | 20 +------ .../react/views/view/ReactViewManager.java | 3 +- 6 files changed, 31 insertions(+), 103 deletions(-) diff --git a/Examples/UIExplorer/TouchableExample.js b/Examples/UIExplorer/TouchableExample.js index 6c19c4086..5e7243c8d 100644 --- a/Examples/UIExplorer/TouchableExample.js +++ b/Examples/UIExplorer/TouchableExample.js @@ -110,14 +110,6 @@ exports.examples = [ render: function(): ReactElement { return ; }, - }, { - title: 'TouchableNativeFeedback with Border Radius', - description: 'The ripples from should follow borderRadius. ' + - 'This only works with a non-borderless background from TouchableNativeFeedback.Ripple', - platform: 'android', - render: function(): ReactElement { - return ; - }, }]; var TextOnPressBox = React.createClass({ @@ -376,25 +368,6 @@ var TouchableDisabled = React.createClass({ } }); -var TouchableNativeFeedbackBorderRadius = React.createClass({ - render: function() { - return ( - - console.log('TouchableNativeFeedback Ripple with Border Radius has been clicked')} - background={TouchableNativeFeedback.Ripple('#ccc')}> - - - TouchableNativeFeedback.Ripple with Border Radius - - - - - ); - } -}); - var heartImage = {uri: 'https://pbs.twimg.com/media/BlXBfT3CQAA6cVZ.png:small'}; var styles = StyleSheet.create({ @@ -468,10 +441,4 @@ var styles = StyleSheet.create({ fontWeight: '500', color: 'blue', }, - tnfBorderRadiusView: { - borderTopLeftRadius: 10, - borderTopRightRadius: 5, - borderBottomLeftRadius: 0, - borderBottomRightRadius: 20 - } }); diff --git a/Libraries/Components/Touchable/TouchableNativeFeedback.android.js b/Libraries/Components/Touchable/TouchableNativeFeedback.android.js index af158fdf2..4fe522732 100644 --- a/Libraries/Components/Touchable/TouchableNativeFeedback.android.js +++ b/Libraries/Components/Touchable/TouchableNativeFeedback.android.js @@ -92,9 +92,6 @@ var TouchableNativeFeedback = React.createClass({ /** * Creates an object that represents android theme's default background for * selectable elements (?android:attr/selectableItemBackground). - * - * The backgrounds generated by this method DO NOT follow the child view's - * border radii styles. To clip the background with border radii, use Ripple. */ SelectableBackground: function() { return {type: 'ThemeAttrAndroid', attribute: 'selectableItemBackground'}; @@ -114,9 +111,6 @@ var TouchableNativeFeedback = React.createClass({ * example of that behavior). This background type is available on Android * API level 21+. * - * The non-borderless backgrounds generated by this method follows the child - * view's border radii styles (e.g. the ripples are clipped by the border radii). - * * @param color The ripple color * @param borderless If the ripple can render outside it's bounds */ diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java index 4e6ce5e2a..a4dda0d9d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java @@ -9,12 +9,10 @@ package com.facebook.react.views.view; -import javax.annotation.Nullable; - import android.content.Context; import android.content.res.ColorStateList; import android.graphics.Color; -import android.graphics.drawable.PaintDrawable; +import android.graphics.drawable.ColorDrawable; import android.graphics.drawable.Drawable; import android.graphics.drawable.RippleDrawable; import android.os.Build; @@ -36,13 +34,6 @@ public class ReactDrawableHelper { public static Drawable createDrawableFromJSDescription( Context context, ReadableMap drawableDescriptionDict) { - return createDrawableFromJSDescription(context, drawableDescriptionDict, null); - } - - public static Drawable createDrawableFromJSDescription( - Context context, - ReadableMap drawableDescriptionDict, - @Nullable float[] cornerRadii) { String type = drawableDescriptionDict.getString("type"); if ("ThemeAttrAndroid".equals(type)) { String attr = drawableDescriptionDict.getString("attribute"); @@ -84,14 +75,11 @@ public class ReactDrawableHelper { "couldn't be resolved into a drawable"); } } - PaintDrawable mask = null; + Drawable mask = null; if (!drawableDescriptionDict.hasKey("borderless") || drawableDescriptionDict.isNull("borderless") || !drawableDescriptionDict.getBoolean("borderless")) { - mask = new PaintDrawable(Color.WHITE); - if (cornerRadii != null) { - mask.setCornerRadii(cornerRadii); - } + mask = new ColorDrawable(Color.WHITE); } ColorStateList colorStateList = new ColorStateList( new int[][] {new int[]{}}, diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java index d2b65797f..fd8961cb8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java @@ -236,25 +236,6 @@ import com.facebook.csslayout.Spacing; } } - /* package */ float[] getBorderRadii() { - float defaultBorderRadius = !CSSConstants.isUndefined(mBorderRadius) ? mBorderRadius : 0; - float topLeftRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[0]) ? mBorderCornerRadii[0] : defaultBorderRadius; - float topRightRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[1]) ? mBorderCornerRadii[1] : defaultBorderRadius; - float bottomRightRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[2]) ? mBorderCornerRadii[2] : defaultBorderRadius; - float bottomLeftRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[3]) ? mBorderCornerRadii[3] : defaultBorderRadius; - - return new float[] { - topLeftRadius, - topLeftRadius, - topRightRadius, - topRightRadius, - bottomRightRadius, - bottomRightRadius, - bottomLeftRadius, - bottomLeftRadius - }; - } - private void updatePath() { if (!mNeedUpdatePathForBorderRadius) { return; @@ -277,10 +258,25 @@ import com.facebook.csslayout.Spacing; mTempRectForBorderRadius.inset(fullBorderWidth * 0.5f, fullBorderWidth * 0.5f); } - float[] borderRadii = getBorderRadii(); + float defaultBorderRadius = !CSSConstants.isUndefined(mBorderRadius) ? mBorderRadius : 0; + float topLeftRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[0]) ? mBorderCornerRadii[0] : defaultBorderRadius; + float topRightRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[1]) ? mBorderCornerRadii[1] : defaultBorderRadius; + float bottomRightRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[2]) ? mBorderCornerRadii[2] : defaultBorderRadius; + float bottomLeftRadius = mBorderCornerRadii != null && !CSSConstants.isUndefined(mBorderCornerRadii[3]) ? mBorderCornerRadii[3] : defaultBorderRadius; + + mPathForBorderRadius.addRoundRect( mTempRectForBorderRadius, - borderRadii, + new float[] { + topLeftRadius, + topLeftRadius, + topRightRadius, + topRightRadius, + bottomRightRadius, + bottomRightRadius, + bottomLeftRadius, + bottomLeftRadius + }, Path.Direction.CW); float extraRadiusForOutline = 0; @@ -292,14 +288,14 @@ import com.facebook.csslayout.Spacing; mPathForBorderRadiusOutline.addRoundRect( mTempRectForBorderRadiusOutline, new float[] { - borderRadii[0] + extraRadiusForOutline, - borderRadii[1] + extraRadiusForOutline, - borderRadii[2] + extraRadiusForOutline, - borderRadii[3] + extraRadiusForOutline, - borderRadii[4] + extraRadiusForOutline, - borderRadii[5] + extraRadiusForOutline, - borderRadii[6] + extraRadiusForOutline, - borderRadii[7] + extraRadiusForOutline + topLeftRadius + extraRadiusForOutline, + topLeftRadius + extraRadiusForOutline, + topRightRadius + extraRadiusForOutline, + topRightRadius + extraRadiusForOutline, + bottomRightRadius + extraRadiusForOutline, + bottomRightRadius + extraRadiusForOutline, + bottomLeftRadius + extraRadiusForOutline, + bottomLeftRadius + extraRadiusForOutline }, Path.Direction.CW); 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 5ace0a417..63ceac761 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 @@ -22,7 +22,6 @@ import android.view.View; import android.view.ViewGroup; import com.facebook.infer.annotation.Assertions; -import com.facebook.react.bridge.ReadableMap; import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.react.touch.ReactHitSlopView; import com.facebook.react.touch.ReactInterceptingViewGroup; @@ -95,7 +94,6 @@ public class ReactViewGroup extends ViewGroup implements private @Nullable ReactViewBackgroundDrawable mReactBackgroundDrawable; private @Nullable OnInterceptTouchEventListener mOnInterceptTouchEventListener; private boolean mNeedsOffscreenAlphaCompositing = false; - private @Nullable ReadableMap mNativeBackground; public ReactViewGroup(Context context) { super(context); @@ -136,21 +134,7 @@ public class ReactViewGroup extends ViewGroup implements "This method is not supported for ReactViewGroup instances"); } - public void setNativeBackground(@Nullable ReadableMap nativeBackground) { - mNativeBackground = nativeBackground; - refreshTranslucentBackgroundDrawable(); - } - - private void refreshTranslucentBackgroundDrawable() { - Drawable background = null; - if (mNativeBackground != null) { - float[] cornerRadii = null; - if (mReactBackgroundDrawable != null) { - cornerRadii = mReactBackgroundDrawable.getBorderRadii(); - } - background = ReactDrawableHelper.createDrawableFromJSDescription(getContext(), mNativeBackground, cornerRadii); - } - + public void setTranslucentBackgroundDrawable(@Nullable Drawable background) { // it's required to call setBackground to null, as in some of the cases we may set new // background to be a layer drawable that contains a drawable that has been previously setup // as a background previously. This will not work correctly as the drawable callback logic is @@ -223,12 +207,10 @@ public class ReactViewGroup extends ViewGroup implements public void setBorderRadius(float borderRadius) { getOrCreateReactViewBackground().setRadius(borderRadius); - refreshTranslucentBackgroundDrawable(); } public void setBorderRadius(float borderRadius, int position) { getOrCreateReactViewBackground().setRadius(borderRadius, position); - refreshTranslucentBackgroundDrawable(); } public void setBorderStyle(@Nullable String style) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java index d20eee96f..6a4bd5d26 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java @@ -101,7 +101,8 @@ public class ReactViewManager extends ViewGroupManager { @ReactProp(name = "nativeBackgroundAndroid") public void setNativeBackground(ReactViewGroup view, @Nullable ReadableMap bg) { - view.setNativeBackground(bg); + view.setTranslucentBackgroundDrawable(bg == null ? + null : ReactDrawableHelper.createDrawableFromJSDescription(view.getContext(), bg)); } @ReactProp(name = ReactClippingViewGroupHelper.PROP_REMOVE_CLIPPED_SUBVIEWS)