-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix: Button and IconButton TouchableRipple borderRadius #4278
base: main
Are you sure you want to change the base?
fix: Button and IconButton TouchableRipple borderRadius #4278
Conversation
Hey @jahirfiquitiva, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
| 'borderRadius' | ||
> | ||
>; | ||
export const getButtonTouchableRippleStyle = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a unit test covering that functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukewalczak for this specific function? or for the Button
component? Because the previous tests were still passing, meaning the functionality did not affect their styles. The only test that changed was for the outlined button, where the result inner border radius is 19 (20 - 1 (border))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jahirfiquitiva Perfectly for both cases
@lukewalczak I have added a test for an outlined button with custom radius. There were already tests for non-outlined buttons with custom radius |
@lukewalczak would you mind taking another look, please? |
4afd5ed
to
0e0efdf
Compare
Motivation
Resolve issue #4266
Description
Removes the
borderRadius
from theIconButton
innerTouchableRipple
.Using
overflow: hidden
is enough for the inner views to fit the parent view, even taking care of the parent viewborderRadius
.After the changes, we can even set custom border radius for each corner of
IconButton
and theTouchableRipple
will fill the button properlyShot.2024-01-15.at.20.28.01.mp4
Updates the
Button
innerTouchableRipple
props to make the ripple fill theButton
surface.The
overflow: hidden
trick could not be applied toButton
as itsSurface
can be elevated, and when usingoverflow: hidden
the elevation shadow will not be displayed correctly.Adds
contentStyle
prop toIconButton
, similar to the one inButton
Previously there was no way to customize the
IconButton
padding, as if we set it to thestyle
prop, it would cause the innerTouchableRipple
to not fill theIconButton
. Now,contentStyle
allows doing so by setting the styles to the innerTouchableRipple
view.Test plan
Run
yarn example web
to run the example project and view the updated examples forButton
andIconButton