-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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: added ref support to button and touchable ripple component. #3992
Conversation
Hey @sangameshsomawar, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
3900fc9
to
9c4373e
Compare
@lukewalczak Let me know if I can do something more to help reviewing/testing code? |
|
||
type ButtonRef = React.ElementRef<typeof TouchableRipple>; | ||
|
||
const Button: React.ForwardRefRenderFunction<ButtonRef, Props> = ( |
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.
Have you tried something like this?
const Button: React.ForwardRefRenderFunction<ButtonRef, Props> = ( | |
const Button = forwardRef({ |
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.
@DimitarNestorov I could not understand your point.
forwardRef
is already added in the last line export default forwardRef(Button);
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.
I would prefer if we do not explicitly set the type of Button
.
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.
@DimitarNestorov We cannot use forwardRef
from src/utils/forwardRef.tsx
because it does not support mentioning component level attribute, In our case TouchableRipple.supported = true;
is a component level attribute.
Fixes: #3988
Summary
Added ref support for Button and TouchableRipple component.
Test plan
I tried creating ref for button and touchable ripples, it worked.