Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buttons(iOS | Android): BorderRadius issue with styled components #2594

Closed
hirbod opened this issue Sep 19, 2023 · 1 comment · Fixed by #2691
Closed

Buttons(iOS | Android): BorderRadius issue with styled components #2594

hirbod opened this issue Sep 19, 2023 · 1 comment · Fixed by #2691
Labels
Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@hirbod
Copy link

hirbod commented Sep 19, 2023

Description

We're attempting to port all instances of Pressable from react-native to RNGH Buttons (e.g., RectButton). We utilize NativeWind (akin to Tailwind) and convert them into styled components as follows:

import { RectButton } from "react-native-gesture-handler";
const StyledRectButton = styled(RectButton);

Setting a borderRadius is straightforward:

<StyledRectButton tw="rounded-full" />

This approach works seamlessly for every component. However, it encounters issues with the Buttons exported by RNGH. I've identified the root of the problem: the Buttons incorporate an Animated.View as a child within the Button and derive the borderRadius from the flattened Stylesheet.

While passing borderRadius via the style prop to the StyledComponent does work, it complicates the process. You can see the relevant code here:

const resolvedStyle = StyleSheet.flatten(style ?? {});
return (
<BaseButton
{...rest}
style={resolvedStyle}
onActiveStateChange={this.onActiveStateChange}>
<Animated.View
style={[
btnStyles.underlay,
{
opacity: this.opacity,
backgroundColor: this.props.underlayColor,
borderRadius: resolvedStyle.borderRadius,
borderTopLeftRadius: resolvedStyle.borderTopLeftRadius,
borderTopRightRadius: resolvedStyle.borderTopRightRadius,
borderBottomLeftRadius: resolvedStyle.borderBottomLeftRadius,
borderBottomRightRadius: resolvedStyle.borderBottomRightRadius,
},
]}
/>
{children}
</BaseButton>
);
}
}

Ideally, we'd like to pass the child component (turning it into an AnimatedStyledView) and be able to add tw/className props to it, without the need to wrap it in an outer view. At present, migration is unfeasible for us because wrapping the RNGH Button with a view disrupts a significant amount of our styling.

If I recall correctly, we could modify the children's styles in the past. However, this wouldn't be particularly helpful now, as we need the ability to pass our styled props from Tailwind.

Steps to reproduce

See the Snack
https://snack.expo.dev/@hirbod/rngh-button-nativewind?platform=ios

Works fine on web, but does not work on iOS. On iOS, it does apply the borderRadius correctly for the view with the underlayColor (see the red color) but it fails for the main view.

On Android, it fails completely and doesn't even add a color or anything at all.

Snack or a link to a repository

https://snack.expo.dev/@hirbod/rngh-button-nativewind?&platform=ios

Gesture Handler version

2.9.0

React Native version

0.72.3

Platforms

Android, iOS

JavaScript runtime

None

Workflow

Expo bare workflow

Architecture

Paper (Old Architecture)

Build type

None

Device

None

Device model

No response

Acknowledgements

Yes

@github-actions github-actions bot added Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided labels Sep 19, 2023
@hirbod hirbod changed the title Buttons: BorderRadius issue with styled components Buttons(iOS | Android): BorderRadius issue with styled components Sep 19, 2023
@kacperkapusciak
Copy link
Member

We triaged the issue and we've found out that this has nothing to do with tailwind or styled components (the provided repro isn't minimal).

className="rounded-full" in the end applies "{borderBottomLeftRadius": 9999, "borderBottomRightRadius": 9999, "borderTopLeftRadius": 9999, "borderTopRightRadius": 9999} style

And we've found that our RectButton on iOS uses UIControl that doesn't have borderBottomLeftRadius, borderTopLeftRadius, borderTopRightRadius etc. implemented at all. We haven't tested Android yet but we think the issue is most probably similar.

The minimal repro for this would be something like:
      <RectButton
        style={{
          backgroundColor: 'red',
          borderBottomLeftRadius: 24,
          borderBottomRightRadius: 24,
          borderTopLeftRadius: 24,
          borderTopRightRadius: 24,
        }}>
        <Text>My button</Text>
      </RectButton>

I believe this issue applies to every touchable component in RNGH.

For a proposed solution: Handling these border radii props on the native side is implemented in React Native on a View component.

One solution would be to copy-over the border radii code from RCTView to our RectButton. That would add a lot of legacy debt, two separate implementations on iOS and Android - a big no no.

or... just embed a View somewhere deep on the Gesture Handler side and carefully apply only the required styles to it similarly what was proposed in #1617 more than 2 years ago

The second one is more managable but I also worry it'll have some layout glitches for flex layout types

TLDR

We know what is broken but we don't have a good solution yet

cc @j-piasecki

j-piasecki added a commit that referenced this issue Jan 11, 2024
# Description


https://github.com/software-mansion/react-native-gesture-handler/assets/39658211/4ebc5761-c571-4c9a-a9c0-c2e0b7ac0018


This PR adds support for border radius style props like:
`borderTopLeftRadius`, `borderTopRightRadius`, `borderBottomLeftRadius`,
`borderBottomRightRadius` to RectButton.

It implements the border radii by wrapping a RawButton with two Views -
the outer one curves the native RawButton and the inner acts like a mask
cutting the corners of the button. The radius of the inner view had to
be adjusted by subtracting the borderWidth from it - [see this
tweet](https://twitter.com/lilykonings/status/1567317037126680576).

FIxes
#2594

---------

Co-authored-by: Tomasz Żelawski <[email protected]>
Co-authored-by: Jakub Piasecki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants