Add support for continuePropagation to usePress #4683
Conversation
|
Build successful! 🎉 |
| return event.shouldStopPropagation; | ||
| } | ||
|
|
||
| return true; |
There was a problem hiding this comment.
Why do we have return values for these now?
There was a problem hiding this comment.
These are called below and the return value is used to determine whether to stop propagation or not
There was a problem hiding this comment.
LGTM, tested locally and confirmed that longPress properly propagates the press to parent handlers on tap but not on actual long press. Only opinion is that it would be good to document the need to continuePropagation on pressStart and every subsequent press handler to actually propagate a press event, even if just in an "Advanced" section of the docs.
|
Build successful! 🎉 |
|
## API Changes
unknown top level export { type: 'identifier', name: 'Column' } |
6 participants
Closes #2100
This adds a
continuePropagation()method toPressEvent, like we have for keyboard events. This allows anonPressStarthandler to decide that it won't handle a particular event (e.g. maybe a certain press handler only wants to handle touch events or something), and allow it to continue propagating to a parent element.A use case for this is in
useLongPress, which shouldn't prevent a press event from occurring on a parent element if there is a long press handler on a child. If the user actually long presses, the pointer event will be canceled so anonPresswon't be called, but if the user doesn't press long enough, then parent press handlers should still work.One sorta weird thing about this is that
continuePropagationonly on an event likeonPresswon't work, because by that point propagation will already have been stopped duringonPressStartso the press event won't actually reach the parent. So you'd kinda need to continuePropagation on all of them. We could maybe have a new prop to allow propagation, but then it would be harder to do it conditionally for only certain events (which feels like the main use case). I guess continuing propagation is already an advanced use case, so maybe this is fine? It matches our other hooks, and doesn't require adding any new props to lots of components.