-
Notifications
You must be signed in to change notification settings - Fork 25
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
native: simulate pressable event bubbling #4204
Conversation
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.
This was definitely a point of confusion for me when first working with RN. The approach you implemented feels sensible, I think it will be less surprising to have the clicks bubble by default. Curious if others have thoughts!
We should however make sure this doesn't behave strangely anywhere in existing code. Would prefer to avoid merging until after we cut this week's release.
Makes sense to me -- @patosullivan could you take a look? As you've spent time in pressable most recently. |
wanted to add more context that I realized was missing:
|
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.
Makes sense to me!
great – I'll merge this on Friday afternoon after the build review |
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.
Lgtm when you're ready to merge!
To preface: I don't know if this is a great thing to add to the
Pressable
primitive – opening this for discussion. Alternatives include (1) plumbing adisabled
prop toChatMessage
for my one use case, (2) adding a flag toPressable
to opt-in to this behavior, or (3) creating a newPressable
-like component with this behavior (OptionallyPressable
? 🤷 ).I was trying to add a long-press to a container view holding a
ChatMessage
usingPressable
. My handler wouldn't get called – swapping outChatMessage
for a blankView
worked.ChatMessage
has aPressable
which has no bound handlers in this scenario, and I expected events to bubble up the stack like on web – but it looks like RN doesn't have that behavior. Here's a recording of the fixture from this PR demonstrating this – the specific case is whenEnable inner press handler
is off and I'm tapping on the inner view – I expect that the inner view does not activate (because no handler) and the outer view's handler does activate (since I'm tapping over the outer view):passthru-pressable-before.mov
To solve my initial problem:
ChatMessage
'sPressable
, settingpointerEvents="none"
would workdisabled
on the innerPressable
passes events through to ancestors(1) seems violent to do for every use case – I'd prefer a component that can be optionally interactive, and when the component is configured to be non-interactive, events naturally pass through to ancestors.
6847213 implements (2) – setting
disabled={true}
when there are no bound handlers.passthru-pressable-after.mov
I think there's a solution here if we were to move to react-native-gesture-handler instead of the Tamagui pressable mixin, but I don't think it'd be very different in implementation – I think we'd still be disabling interaction when missing handlers. (There are APIs for composing gestures but they require references to the relevant gestures, which would be a lot of plumbing.)