From bd3868643d29e93610e19312571a9736df2cbdf8 Mon Sep 17 00:00:00 2001 From: Vojtech Novak Date: Fri, 3 Apr 2020 18:34:15 -0700 Subject: [PATCH] add ripple config object to Pressable (#28156) Summary: Motivation is to support ripple radius just like in TouchableNativeFeedback, plus borderless attribute. See https://github.com/facebook/react-native/pull/28009#issuecomment-589489520 In the current form this means user needs to pass an `android_ripple` prop which is an object of this shape: ``` export type RippleConfig = {| color?: ?ColorValue, borderless?: ?boolean, radius?: ?number, |}; ``` Do we want to add methods that would create such config objects - https://facebook.github.io/react-native/docs/touchablenativefeedback#methods ? ## Changelog [Android] [Added] - support borderless and custom ripple radius on Pressable Pull Request resolved: https://github.com/facebook/react-native/pull/28156 Test Plan: Tested locally in RNTester. I noticed that when some content is rendered after the touchables, the ripple effect is "cut off" by the boundaries of the next view. This is not specific to Pressable, it happens to TouchableNativeFeedback too but I just didn't notice it before in https://github.com/facebook/react-native/pull/28009. As it is an issue of its own, I didn't investigate that. ![pressable](https://user-images.githubusercontent.com/1566403/75098762-785f2200-55ba-11ea-8842-e648317610e3.gif) I changed the Touchable example slightly too (I just moved the "custom ripple radius" up to show the "cutting off" issue), so just for completeness: ![touchable](https://user-images.githubusercontent.com/1566403/75098763-81e88a00-55ba-11ea-9528-e0343d1e054b.gif) Reviewed By: yungsters Differential Revision: D20071021 Pulled By: TheSavior fbshipit-source-id: cb553030934205a52dd50a2a8c8a20da6100e23f --- Libraries/Components/Pressable/Pressable.js | 30 +++++++------ .../Pressable/useAndroidRippleForView.js | 23 +++++++--- Libraries/Components/View/ViewPropTypes.js | 1 + .../js/examples/Pressable/PressableExample.js | 45 ++++++++++++++++++- .../js/examples/Touchable/TouchableExample.js | 20 +++++---- .../react/views/view/ReactDrawableHelper.java | 4 +- .../react/views/view/ReactViewGroup.java | 2 +- 7 files changed, 92 insertions(+), 33 deletions(-) diff --git a/Libraries/Components/Pressable/Pressable.js b/Libraries/Components/Pressable/Pressable.js index f4d7bd0d559..963933d13ca 100644 --- a/Libraries/Components/Pressable/Pressable.js +++ b/Libraries/Components/Pressable/Pressable.js @@ -12,7 +12,9 @@ import * as React from 'react'; import {useMemo, useState, useRef, useImperativeHandle} from 'react'; -import useAndroidRippleForView from './useAndroidRippleForView'; +import useAndroidRippleForView, { + type RippleConfig, +} from './useAndroidRippleForView'; import type { AccessibilityActionEvent, AccessibilityActionInfo, @@ -122,7 +124,7 @@ type Props = $ReadOnly<{| /** * Enables the Android ripple effect and configures its color. */ - android_rippleColor?: ?ColorValue, + android_ripple?: ?RippleConfig, /** * Used only for documentation or testing (e.g. snapshot testing). @@ -138,7 +140,7 @@ function Pressable(props: Props, forwardedRef): React.Node { const { accessible, android_disableSound, - android_rippleColor, + android_ripple, children, delayLongPress, disabled, @@ -156,7 +158,7 @@ function Pressable(props: Props, forwardedRef): React.Node { const viewRef = useRef | null>(null); useImperativeHandle(forwardedRef, () => viewRef.current); - const android_ripple = useAndroidRippleForView(android_rippleColor, viewRef); + const android_rippleConfig = useAndroidRippleForView(android_ripple, viewRef); const [pressed, setPressed] = usePressState(testOnly_pressed === true); @@ -172,18 +174,18 @@ function Pressable(props: Props, forwardedRef): React.Node { onLongPress, onPress, onPressIn(event: PressEvent): void { - if (android_ripple != null) { - android_ripple.onPressIn(event); + if (android_rippleConfig != null) { + android_rippleConfig.onPressIn(event); } setPressed(true); if (onPressIn != null) { onPressIn(event); } }, - onPressMove: android_ripple?.onPressMove, + onPressMove: android_rippleConfig?.onPressMove, onPressOut(event: PressEvent): void { - if (android_ripple != null) { - android_ripple.onPressOut(event); + if (android_rippleConfig != null) { + android_rippleConfig.onPressOut(event); } setPressed(false); if (onPressOut != null) { @@ -193,7 +195,7 @@ function Pressable(props: Props, forwardedRef): React.Node { }), [ android_disableSound, - android_ripple, + android_rippleConfig, delayLongPress, disabled, hitSlop, @@ -211,7 +213,7 @@ function Pressable(props: Props, forwardedRef): React.Node { void] { return [pressed || forcePressed, setPressed]; } -const MemodPressable = React.memo(React.forwardRef(Pressable)); -MemodPressable.displayName = 'Pressable'; +const MemoedPressable = React.memo(React.forwardRef(Pressable)); +MemoedPressable.displayName = 'Pressable'; -export default (MemodPressable: React.AbstractComponent< +export default (MemoedPressable: React.AbstractComponent< Props, React.ElementRef, >); diff --git a/Libraries/Components/Pressable/useAndroidRippleForView.js b/Libraries/Components/Pressable/useAndroidRippleForView.js index 6861e88d1f8..44972fb860f 100644 --- a/Libraries/Components/Pressable/useAndroidRippleForView.js +++ b/Libraries/Components/Pressable/useAndroidRippleForView.js @@ -22,14 +22,21 @@ type NativeBackgroundProp = $ReadOnly<{| type: 'RippleAndroid', color: ?number, borderless: boolean, + rippleRadius: ?number, |}>; +export type RippleConfig = {| + color?: ?ColorValue, + borderless?: ?boolean, + radius?: ?number, +|}; + /** * Provides the event handlers and props for configuring the ripple effect on * supported versions of Android. */ export default function useAndroidRippleForView( - rippleColor: ?ColorValue, + rippleConfig: ?RippleConfig, viewRef: {|current: null | React.ElementRef|}, ): ?$ReadOnly<{| onPressIn: (event: PressEvent) => void, @@ -39,13 +46,16 @@ export default function useAndroidRippleForView( nativeBackgroundAndroid: NativeBackgroundProp, |}>, |}> { + const {color, borderless, radius} = rippleConfig ?? {}; + const normalizedBorderless = borderless === true; + return useMemo(() => { if ( Platform.OS === 'android' && Platform.Version >= 21 && - rippleColor != null + (color != null || normalizedBorderless || radius != null) ) { - const processedColor = processColor(rippleColor); + const processedColor = processColor(color); invariant( processedColor == null || typeof processedColor === 'number', 'Unexpected color given for Ripple color', @@ -53,11 +63,12 @@ export default function useAndroidRippleForView( return { viewProps: { - // Consider supporting `nativeForegroundAndroid` and `borderless`. + // Consider supporting `nativeForegroundAndroid` nativeBackgroundAndroid: { type: 'RippleAndroid', color: processedColor, - borderless: false, + borderless: normalizedBorderless, + rippleRadius: radius, }, }, onPressIn(event: PressEvent): void { @@ -90,5 +101,5 @@ export default function useAndroidRippleForView( }; } return null; - }, [rippleColor, viewRef]); + }, [color, normalizedBorderless, radius, viewRef]); } diff --git a/Libraries/Components/View/ViewPropTypes.js b/Libraries/Components/View/ViewPropTypes.js index 5d3a888f77d..2bff27a0891 100644 --- a/Libraries/Components/View/ViewPropTypes.js +++ b/Libraries/Components/View/ViewPropTypes.js @@ -230,6 +230,7 @@ type AndroidDrawableRipple = $ReadOnly<{| type: 'RippleAndroid', color?: ?number, borderless?: ?boolean, + rippleRadius?: ?number, |}>; type AndroidDrawable = AndroidDrawableThemeAttr | AndroidDrawableRipple; diff --git a/RNTester/js/examples/Pressable/PressableExample.js b/RNTester/js/examples/Pressable/PressableExample.js index d2155ebc48a..1dadac6b1c4 100644 --- a/RNTester/js/examples/Pressable/PressableExample.js +++ b/RNTester/js/examples/Pressable/PressableExample.js @@ -369,13 +369,56 @@ exports.examples = [ }; return ( - + ); }, }, + { + title: 'Pressable with custom Ripple', + description: ("Pressable can specify ripple's radius and borderless params": string), + platform: 'android', + render: function(): React.Node { + const nativeFeedbackButton = { + textAlign: 'center', + margin: 10, + }; + return ( + + + + + radius 30 + + + + + + + + radius 150 + + + + + + + + radius 70, with border + + + + + ); + }, + }, { title: ' with highlight', render: function(): React.Node { diff --git a/RNTester/js/examples/Touchable/TouchableExample.js b/RNTester/js/examples/Touchable/TouchableExample.js index 8389c580543..3d1c4008ac7 100644 --- a/RNTester/js/examples/Touchable/TouchableExample.js +++ b/RNTester/js/examples/Touchable/TouchableExample.js @@ -452,10 +452,12 @@ function CustomRippleRadius() { console.log('custom TNF has been clicked')} - background={TouchableNativeFeedback.SelectableBackgroundBorderless(50)}> + background={TouchableNativeFeedback.SelectableBackgroundBorderless( + 150, + )}> - radius 50 + radius 150 @@ -646,6 +648,13 @@ exports.examples = [ return ; }, }, + { + title: 'Custom Ripple Radius (Android-only)', + description: ('Ripple radius on TouchableNativeFeedback can be controlled': string), + render: function(): React.Element { + return ; + }, + }, { title: 'Disabled Touchable*', description: (' components accept disabled prop which prevents ' + @@ -654,11 +663,4 @@ exports.examples = [ return ; }, }, - { - title: 'Custom Ripple Radius (Android-only)', - description: ('Ripple radius on TouchableNativeFeedback can be controlled': string), - render: function(): React.Element { - return ; - }, - }, ]; 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 6d7dd64bfe6..add5f38812b 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 @@ -69,7 +69,7 @@ public class ReactDrawableHelper { Context context, ReadableMap drawableDescriptionDict) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) { throw new JSApplicationIllegalArgumentException( - "Ripple drawable is not available on " + "android API <21"); + "Ripple drawable is not available on android API <21"); } int color = getColor(context, drawableDescriptionDict); Drawable mask = getMask(drawableDescriptionDict); @@ -101,7 +101,7 @@ public class ReactDrawableHelper { return context.getResources().getColor(sResolveOutValue.resourceId); } else { throw new JSApplicationIllegalArgumentException( - "Attribute colorControlHighlight " + "couldn't be resolved into a drawable"); + "Attribute colorControlHighlight couldn't be resolved into a drawable"); } } } 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 a191dd2cb19..a8f162163a8 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 @@ -186,7 +186,7 @@ public class ReactViewGroup extends ViewGroup 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 + // background to be a layer drawable that contains a drawable that has been setup // as a background previously. This will not work correctly as the drawable callback logic is // messed up in AOSP updateBackgroundDrawable(null);