do not propagate events through portal #1840
Conversation
jluyau
requested review from
dannify,
devongovett and
snowystinger
as code owners
|
Build successful! 🎉 |
|
I wasn't sure if the other events (that I didn't modify) were being triggered in my manual tests. Didn't want to just add the check for every event if it wasn't triggering at all, but I can add to others if needed? |
|
Build successful! 🎉 |
|
Agreed, there are a lot of different interaction paths, however, I think we need to really carefully address all of them. That means doing things like testing the screen reader in FF on windows or whatever else we can think of. Possibly talk to @devongovett about the weirder ones and look through his blog post to track them down as well. |
| @@ -214,7 +214,6 @@ export function usePress(props: PressHookProps): PressResult { | |||
| e.preventDefault(); | |||
There was a problem hiding this comment.
Do we need to do the check for keyboard events as well?
There was a problem hiding this comment.
yeah most likely. i'll see if i can add a good test for that
| if (!e.currentTarget.contains(e.target as HTMLElement)) { | ||
| return; | ||
| } | ||
|
|
||
| if (!state.ignoreEmulatedMouseEvents && e.button === 0) { | ||
| triggerPressUp(e, state.pointerType); | ||
| } |
There was a problem hiding this comment.
Make sure we get all of the events assigned to pressProps. I think maybe touch events are missing? Is there a reason they were not included?
There was a problem hiding this comment.
i think i had placed them all the functions with an event, but i couldn't tell if they were doing anything or not. should i go ahead and place them in all the pressProps events?
There was a problem hiding this comment.
Yeah all of the ones that are on pressProps, not global listeners.
…o prevent-portal-bubbling # Conflicts: # packages/@react-aria/interactions/src/useHover.ts
|
Build successful! 🎉 |
Closes #1810
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project:
Adobe/Quarry