facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.37k stars 24.25k forks source link

TouchableNativeFeedback's ripples aren't affected by borderRadius on Android 9 #6480

Closed jeffchienzabinet closed 3 years ago

jeffchienzabinet commented 8 years ago

This is a bit of a problem when creating buttons with borderRadius and TouchableNativeFeedback, and the ripples extend out. I'm not an expert but I suppose setNativeBackground needs to get the corner radius when making the mask for the RippleDrawable and setBorderRadius may need to remake the background drawable somehow.

jonathanpalma commented 8 years ago

Attached the following images to represent graphically what is proposed

before after

Images were taken from Michael Evans Blog - Android Ripples With Rounded Corners

antoinerousseau commented 8 years ago

It's supposed to be implemented in https://github.com/facebook/react-native/pull/6515 but I can't make it work :/

antoinerousseau commented 8 years ago

I realize that PR got reverted

reyesr commented 8 years ago

FWIW, as of 0.30, the ripple effect works with respect to the borderRadius of its parent , but it requires its borderless argument to be set to true. Eg:

<View style={{borderRadius: 24, backgroundColor: '#F66', width: 48,height: 48}}> <TouchableNativeFeedback background={TouchableNativeFeedback.Ripple('#AAF', true)}> [snip] </TouchableNativeFeedback> </View> If this is the intented effect, it should probably be made explicit, and added to the documentation. It it's a side effect (no pun intented), #6515 still makes sense.

tituswoo commented 8 years ago

@reyesr OMG thank you so much for this! 👍

jonathanpalma commented 8 years ago

@reyesr IT WORKS!!! Thank you so much buddy!! :raised_hands:

ericvicenti commented 7 years ago

Looks like this is resolved. Feel free to open a new issue if you're still having troubles with it

satya164 commented 6 years ago

Re-opening since this issue has re-surfaced on Android P.

ferrannp commented 6 years ago

Just to illustrate from Android P:

screenshot_20180820-124834_2

lalosh commented 6 years ago

@reyesr thank you..yet it worked for me after I added a View inside TochableNativeFeedback with the same parent style .. like:

        <View style={{ borderRadius: 20, backgroundColor: '#2f2', width: 80, height: 80 }}>
            <TouchableNativeFeedback background={TouchableNativeFeedback.Ripple('#AAF', true)}>
                <View style={{ borderRadius: 20, backgroundColor: '#2f2', width: 80, height: 80 }}>
                    <Text> test button </Text>
                </View>
             </TouchableNativeFeedback> 
         </View>
yaronlevi commented 5 years ago

0.55 + Android Pie = The ripple is not affected by the borderRadius. Tried all the variations (View from outside, View from inside, both).

aMarCruz commented 5 years ago

This issue was never solved, the solution given in a comment above is a hack.

yaronlevi commented 5 years ago

Any updates on the issue?

balthazar commented 5 years ago
<View style={{ borderRadius: 5, overflow: 'hidden' }}>
  <TouchableNativeFeedback onPress={() => {}} background={TouchableNativeFeedback.Ripple('red')}>
    {'CLICK CLICK'}
  </TouchableNativeFeedback>
</View>

You guys should try to wrap your TouchableNativeFeedback in a view with a border-radius and an hidden overflow.

aMarCruz commented 5 years ago

@balthazar , wrapping the touchable hides the effect of borderless ripple. This is undesired in Floating Action Buttons components.

balthazar commented 5 years ago

@aMarCruz I'm not talking about borderless ripples here, as my example shows.

ferrannp commented 5 years ago

I've been trying the button issue with https://github.com/kmagiera/react-native-gesture-handler. Looks like it works correctly using the buttons from the library on Android P:

ripple-p

RichardLindhout commented 5 years ago

For me it works when I set in TouchableRipple.js the useForeground true + borderless false

  disabled={disabled}
          useForeground={true}
          background={
            background != null
              ? background
              : TouchableNativeFeedback.Ripple(calculatedRippleColor, false)
          }

Also on e.g. the FAB its needs an overflow: 'hidden' style. Works for me on both iOS and Android!

  touchable: {
    borderRadius: 28,
    overflow: 'hidden',
  },

I'm running the latest Android (9 Pie)

rickhanlonii commented 5 years ago

Thanks @RichardLindhout

If anyone is still experiencing this on the latest release please submit a new issue

satya164 commented 5 years ago

@rickhanlonii That's a workaround which we can't use if oveflow: hidden is not desirable. The behavior is still different between Android versions and buggy.

rickhanlonii commented 5 years ago

@satya164 do you think we should keep this open so contributors need to read though the whole thread from 2016 to understand what the current status is or should we file a new issue with the current state of the bug and no superfluous/outdated information?

satya164 commented 5 years ago

@rickhanlonii someone can open a new issue, but this issue lists some workarounds which are useful to people coming from google, which is better than a new empty issue imo, given that this thread is quite short. there are also people subscribed to this issue who won't get the notifications for the new issue.

tsvetan-ganev commented 5 years ago

It's 2019 and I can't create a touchable element with border radius which works on both platforms without using dirty hacks.

nhunzaker commented 5 years ago

I poked around with this a bit. One solution is to change the mask on the ripple drawable to be a ShapeDrawable with a corner radius. So instead of:

https://github.com/facebook/react-native/blob/cbf1b39c662eeede356214ba276a105604b75bb2/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java#L78-L86

ColorDrawable is just a rectangle, but it can be replaced with a ShapeDrawable that uses a RoundRectShape. So you could do:

private static Drawable getRippleMask() {
    // Border radii
    float[] outerRadii = new float[] { 12f, 12f, 12f, 12f, 12f, 12f, 12f, 12f };

    RoundRectShape r = new RoundRectShape(outerRadii, null, null);

    ShapeDrawable shapeDrawable = new ShapeDrawable(r);
    shapeDrawable.getPaint().setColor(Color.WHITE);

    return shapeDrawable;
  }

  public static Drawable createDrawableFromJSDescription(Context context, ReadableMap drawableDescriptionDict) {
  // Later on
  return new RippleDrawable(colorStateList, null, mask);
}

This works!

Screenshot_1561043846

However it needs to:

  1. Account for border-width (the mask should only apply to the edge of the border)
  2. Account for the actual border radius

So I think I might have a fix for this! I just need to figure out how to pass the border and corner radius fields ReactDrawableHelper.

nhunzaker commented 5 years ago

I sent out a possible fix for this here: https://github.com/facebook/react-native/pull/25342

stale[bot] commented 5 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

satya164 commented 5 years ago

Fixed in https://github.com/facebook/react-native/commit/14b455f69a30d128db384749347f41b03b9a6000

grabbou commented 4 years ago

Unfortunately, the commit that was supposed to fix this issue introduced a major regression and we had to roll it back.

I am reopening this issue as it's still the case. Hopefully, prior work by @nhunzaker can be helpful to work on a fix once again.

InceptSolutions commented 4 years ago

My fix to this was to add overflow:'hidden' and the needed borderRadius to the parent view containing the TouchableNativeFeedback and then add useForeground={true} to the TouchableNativeFeedback :)

RichardLindhout commented 4 years ago

For me it works when I set in TouchableRipple.js the useForeground true + borderless false

  disabled={disabled}
          useForeground={true}
          background={
            background != null
              ? background
              : TouchableNativeFeedback.Ripple(calculatedRippleColor, false)
          }

Also on e.g. the FAB its needs an overflow: 'hidden' style. Works for me on both iOS and Android!

  touchable: {
    borderRadius: 28,
    overflow: 'hidden',
  },

I'm running the latest Android (9 Pie)

I was first :-P

InceptSolutions commented 4 years ago

@RichardLindhout Hahahaha, nice one :D

Kleinhummel commented 4 years ago

FWIW, as of 0.30, the ripple effect works with respect to the borderRadius of its parent , but it requires its borderless argument to be set to true. Eg:

<View style={{borderRadius: 24, backgroundColor: '#F66', width: 48,height: 48}}> <TouchableNativeFeedback background={TouchableNativeFeedback.Ripple('#AAF', true)}> [snip] </TouchableNativeFeedback> </View> If this is the intented effect, it should probably be made explicit, and added to the documentation. It it's a side effect (no pun intented), #6515 still makes sense.

This causes the ripple to completely ignore the size of the button:

Button

My fix to this was to add overflow:'hidden' and the needed borderRadius to the parent view containing the TouchableNativeFeedback and then add useForeground={true} to the TouchableNativeFeedback :)

This was also the working solution for me. But what if I want the ripples in the background and not in the foreground??

stale[bot] commented 4 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

likrot commented 4 years ago

FWIW, as of 0.30, the ripple effect works with respect to the borderRadius of its parent , but it requires its borderless argument to be set to true. Eg: <View style={{borderRadius: 24, backgroundColor: '#F66', width: 48,height: 48}}> <TouchableNativeFeedback background={TouchableNativeFeedback.Ripple('#AAF', true)}> [snip] </TouchableNativeFeedback> </View> If this is the intented effect, it should probably be made explicit, and added to the documentation. It it's a side effect (no pun intented), #6515 still makes sense.

This causes the ripple to completely ignore the size of the button:

Button

My fix to this was to add overflow:'hidden' and the needed borderRadius to the parent view containing the TouchableNativeFeedback and then add useForeground={true} to the TouchableNativeFeedback :)

This was also the working solution for me. But what if I want the ripples in the background and not in the foreground??

Hi,

I don't know if do you have still a problem, but this is how I'm using it ( "react-native": "0.62.2",):

    buttonShape: {
        borderRadius: 50,
        overflow: 'hidden',
    },
    button: {
        backgroundColor:  'green',
        height: 80,
        width: 80,
    },

[...]

            <View style={styles.buttonShape}>
                <TouchableFeedback>
                    <View style={styles.button}>
                        <Text>
                            Floating btn
                        </Text>
                    </View>
                </TouchableFeedback>
            </View>
Bardiamist commented 4 years ago

Same problem in Pressable =\

tpakhoa1996 commented 4 years ago

I tried every possible ways to make it work on Pressable and this problem still persist. Are there any solutions for 0.63.2 yet?

RichardLindhout commented 4 years ago

For me it works when I set in TouchableRipple.js the useForeground true + borderless false

  disabled={disabled}
          useForeground={true}
          background={
            background != null
              ? background
              : TouchableNativeFeedback.Ripple(calculatedRippleColor, false)
          }

Also on e.g. the FAB its needs an overflow: 'hidden' style. Works for me on both iOS and Android!

  touchable: {
    borderRadius: 28,
    overflow: 'hidden',
  },

I'm running the latest Android (9 Pie)

This still works but it's in the hidden comments now..

mrousavy commented 3 years ago

The <Pressable> doesn't have a useForeground prop, how are we supposed to show ripple effects with buttons that have backgrounds (such as a LinearGradient) in a Pressable?

nurhusni commented 3 years ago

Is there any update and better solution to this? I've already tried each solution like wrapping the TouchableNativeFeedback in View, setForeground to true, overflow hidden, etc but none of them works. I run it on my phone on Android Pie.

RichardLindhout commented 3 years ago

Is there any update and better solution to this? I've already tried each solution like wrapping the TouchableNativeFeedback in View, setForeground to true, overflow hidden, etc but none of them works. I run it on my phone on Android Pie.

Have you explicitly set the borderless={false} + adding a view around it with overflow:'hidden' and the same borderRadius as the ripple?

nurhusni commented 3 years ago

Have you explicitly set the borderless={false} + adding a view around it with overflow:'hidden' and the same borderRadius as the ripple?

I did. But in this case, the ripple doesn't appear at all. There's no feedback when pressing the button.

This is the code:

    <View
      style={{ backgroundColor, borderRadius: 16, overflow: "hidden" }}
    >
      <TouchableNativeFeedback
        onPress={onPress}
        useForeground={true}
        background={TouchableNativeFeedback.Ripple(rippleColor, false, 16)}
      >
        {children}
      </TouchableNativeFeedback>
    </View>
RichardLindhout commented 3 years ago

Ok I found some code in the hidden comments from when I did have problems with this.

For me it works when I set in TouchableRipple.js the useForeground true + borderless false

  disabled={disabled}
          useForeground={true}
          background={
            background != null
              ? background
              : TouchableNativeFeedback.Ripple(calculatedRippleColor, false)
          }

Also on e.g. the FAB its needs an overflow: 'hidden' style. Works for me on both iOS and Android!

  touchable: {
    borderRadius: 28,
    overflow: 'hidden',
  },

I'm running the latest Android (9 Pie)

This did work for me

I think in your example it should be something like

<View
      style={{borderRadius: 16, overflow: "hidden" }}
    >
      <TouchableNativeFeedback
        onPress={onPress}
        style={{borderRadius: 16 }}
         useForeground={true}
         borderless={false}
        background={TouchableNativeFeedback.Ripple(rippleColor, false, 16)}
      >
        {children}
      </TouchableNativeFeedback>
    </View>

Maybe also consider using the Pressable component instead of the TouchableNativeFeedback

nurhusni commented 3 years ago

This one works for me! Instead of wrapping the Touchable inside View, I wrap the View inside the Touchable.

    <TouchableNativeFeedback
      onPress={onPress}
      style={{borderRadius: 16 }}
      background={TouchableNativeFeedback.Ripple(rippleColor, false)}
      useForeground={true}
    >
      <View
        style={[
          { overflow: "hidden" },
        ]}
      >
        {children}
      </View>
    </TouchableNativeFeedback>
vasilestefirta commented 3 years ago

@panjiahnh's suggestion worked for me. Here's my full Button.js component if anyone is interested. It's using TouchableOpacity on iOS and TouchableNativeFeedback on Android.

{
  "react-native": "0.63.4"
}
import React from 'react';
import PropTypes from 'prop-types';
import {
  StyleSheet,
  TouchableOpacity,
  TouchableNativeFeedback,
  Platform,
  Text,
  View,
  ViewPropTypes,
} from 'react-native';
import { colors, fonts } from '../../config/styles';

const Button = (props) => {
  const { style, type, children, disabled, onPress } = props;
  const isPrimary = type === 'primary';
  const isSecondary = type === 'secondary';
  const isTransparent = type === 'transparent';

  const buttonText = (
    <Text style={[styles.buttonText, !isPrimary && styles.buttonTextDark]}>{children}</Text>
  );

  if (Platform.OS === 'ios') {
    return (
      <TouchableOpacity
        style={[
          styles.button,
          isPrimary && styles.buttonPrimary,
          isSecondary && styles.buttonSecondary,
          style,
          disabled && styles.buttonDisabled,
        ]}
        onPress={onPress}
        disabled={disabled}
      >
        {buttonText}
      </TouchableOpacity>
    );
  }

  return (
    <TouchableNativeFeedback
      onPress={onPress}
      disabled={disabled}
      background={
        Platform.Version >= 21
          ? TouchableNativeFeedback.Ripple(
              isPrimary || isSecondary ? colors.whiteBg : colors.blueBg,
              false
            )
          : TouchableNativeFeedback.SelectableBackground(5)
      }
      useForeground={isTransparent}
    >
      <View
        style={[
          styles.button,
          isPrimary && styles.buttonPrimary,
          isSecondary && styles.buttonSecondary,
          isTransparent && { overflow: 'hidden' },
          style,
          disabled && styles.buttonDisabled,
        ]}
      >
        {buttonText}
      </View>
    </TouchableNativeFeedback>
  );
};

Button.propTypes = {
  onPress: PropTypes.func,
  style: ViewPropTypes.style,
  type: PropTypes.oneOf(['primary', 'secondary', 'transparent']),
  children: PropTypes.oneOfType([PropTypes.string, PropTypes.array]),
  disabled: PropTypes.bool,
};

Button.defaultProps = {
  onPress: (f) => f,
  style: {},
  type: 'primary',
  children: '',
  disabled: false,
};

const styles = StyleSheet.create({
  button: {
    borderRadius: 27,
    height: 45,
    paddingHorizontal: 30,
    justifyContent: 'center',
    alignItems: 'center',
  },
  buttonPrimary: {
    backgroundColor: colors.blueBg,
  },
  buttonSecondary: {
    backgroundColor: colors.greenBg,
  },
  buttonText: {
    color: '#FFF',
    fontSize: 16,
    fontFamily: fonts.regular,
    letterSpacing: 1.15,
    textAlign: 'center',
  },
  buttonTextDark: {
    color: colors.darkBlueText,
  },
  buttonDisabled: {
    opacity: 0.3,
  },
});

export default Button;
iOS 14 Android 10 (Pixel 4 API 29)
ios-device android-device
SaeedZhiany commented 3 years ago

@vasilestefirta

using overflow: 'hidden' hides elevation effect on android in my case. I didn't test, but maybe it hides shadows on iOS too.

safaiyeh commented 3 years ago

Closing as @panjiahnh suggestion works. Feel free to open a new issue if this is still happening.

SaeedZhiany commented 3 years ago

Closing as @panjiahnh suggestion works. Feel free to open a new issue if this is still happening.

@safaiyeh did you read my comment? @panjiahnh suggestion is just a workaround and doesn't fix the problem correctly. please reopen this issue.

dsoffiantini commented 2 years ago

@panjiahnh's suggestion worked for me. Here's my full Button.js component if anyone is interested. It's using TouchableOpacity on iOS and TouchableNativeFeedback on Android.

{
  "react-native": "0.63.4"
}
import React from 'react';
import PropTypes from 'prop-types';
import {
  StyleSheet,
  TouchableOpacity,
  TouchableNativeFeedback,
  Platform,
  Text,
  View,
  ViewPropTypes,
} from 'react-native';
import { colors, fonts } from '../../config/styles';

const Button = (props) => {
  const { style, type, children, disabled, onPress } = props;
  const isPrimary = type === 'primary';
  const isSecondary = type === 'secondary';
  const isTransparent = type === 'transparent';

  const buttonText = (
    <Text style={[styles.buttonText, !isPrimary && styles.buttonTextDark]}>{children}</Text>
  );

  if (Platform.OS === 'ios') {
    return (
      <TouchableOpacity
        style={[
          styles.button,
          isPrimary && styles.buttonPrimary,
          isSecondary && styles.buttonSecondary,
          style,
          disabled && styles.buttonDisabled,
        ]}
        onPress={onPress}
        disabled={disabled}
      >
        {buttonText}
      </TouchableOpacity>
    );
  }

  return (
    <TouchableNativeFeedback
      onPress={onPress}
      disabled={disabled}
      background={
        Platform.Version >= 21
          ? TouchableNativeFeedback.Ripple(
              isPrimary || isSecondary ? colors.whiteBg : colors.blueBg,
              false
            )
          : TouchableNativeFeedback.SelectableBackground(5)
      }
      useForeground={isTransparent}
    >
      <View
        style={[
          styles.button,
          isPrimary && styles.buttonPrimary,
          isSecondary && styles.buttonSecondary,
          isTransparent && { overflow: 'hidden' },
          style,
          disabled && styles.buttonDisabled,
        ]}
      >
        {buttonText}
      </View>
    </TouchableNativeFeedback>
  );
};

Button.propTypes = {
  onPress: PropTypes.func,
  style: ViewPropTypes.style,
  type: PropTypes.oneOf(['primary', 'secondary', 'transparent']),
  children: PropTypes.oneOfType([PropTypes.string, PropTypes.array]),
  disabled: PropTypes.bool,
};

Button.defaultProps = {
  onPress: (f) => f,
  style: {},
  type: 'primary',
  children: '',
  disabled: false,
};

const styles = StyleSheet.create({
  button: {
    borderRadius: 27,
    height: 45,
    paddingHorizontal: 30,
    justifyContent: 'center',
    alignItems: 'center',
  },
  buttonPrimary: {
    backgroundColor: colors.blueBg,
  },
  buttonSecondary: {
    backgroundColor: colors.greenBg,
  },
  buttonText: {
    color: '#FFF',
    fontSize: 16,
    fontFamily: fonts.regular,
    letterSpacing: 1.15,
    textAlign: 'center',
  },
  buttonTextDark: {
    color: colors.darkBlueText,
  },
  buttonDisabled: {
    opacity: 0.3,
  },
});

export default Button;

This solution works for the transparent button, but it does not work for the colored buttons. It looks like it does since the effect is white, but if you try changing it to black or a color, you see the square surrounding the button still.

HeadJk commented 2 years ago

I have tried these solutions, and the ripple effect is working, but the touchable is responding to presses outside of the border radius.

The red area is where it should respond to presses, and the black area is where it is actually responding to presses. image

gaganbiswas commented 2 years ago

So, I tried a solution and it works as expected. The ripple effect works behind the background and it doesn't require useForeground={true}

<View style={{ borderRadius: 30, overflow: "hidden"}}>
    <TouchableNativeFeedback
          background={TouchableNativeFeedback.Ripple("#1D4ED8", false)}
          onPress={() => {}}
    >
          <View style={{ padding: 8, borderRadius: 30 }}>
              <Ionicons name="call-outline" size={24} color={"white"} />
          </View>
    </TouchableNativeFeedback>
</View>

Try this out and let me know if it works for you 👯. P.S. Change the Ripple color to suit your needs.