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 f4d7bd0d559b1a..963933d13ca11b 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 6861e88d1f82c6..44972fb860f669 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 5d3a888f77dd49..2bff27a0891f61 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 d2155ebc48a36f..1dadac6b1c4c13 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 8389c580543a31..3d1c4008ac731c 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 @@ -647,18 +649,18 @@ exports.examples = [ }, }, { - title: 'Disabled Touchable*', - description: (' components accept disabled prop which prevents ' + - 'any interaction with component': string), + title: 'Custom Ripple Radius (Android-only)', + description: ('Ripple radius on TouchableNativeFeedback can be controlled': string), render: function(): React.Element { - return ; + return ; }, }, { - title: 'Custom Ripple Radius (Android-only)', - description: ('Ripple radius on TouchableNativeFeedback can be controlled': string), + title: 'Disabled Touchable*', + description: (' components accept disabled prop which prevents ' + + 'any interaction with component': string), render: function(): React.Element { - return ; + 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 6d7dd64bfe62bc..add5f38812baa0 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 @@ private static RippleDrawable getRippleDrawable( 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 @@ private static int getColor(Context context, ReadableMap drawableDescriptionDict 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 a191dd2cb19b11..a8f162163a8562 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 void setBackground(Drawable drawable) { 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);