-
-
Notifications
You must be signed in to change notification settings - Fork 256
[popover] Fix swiping or scrolling on nested popup dismissing popover on touch #3011
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
base: master
Are you sure you want to change the base?
Conversation
commit: |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Bundle size report
|
1e49be7 to
f45a406
Compare
23548eb to
a7806f2
Compare
a7806f2 to
039fb7e
Compare
Signed-off-by: atomiks <[email protected]>
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.
Csb with the fix: https://codesandbox.io/p/devbox/brave-butterfly-rm8zjs?workspaceId=ws_HE9JRzFW36Y2kpBoSwdvZ5
Can you elaborate why we are removing the way to customize the capture of the keyboard event using escape? What's the correlation to the bug?
| /** | ||
| * Determines whether to use capture phase event listeners. | ||
| */ | ||
| capture?: boolean | { escapeKey?: boolean; outsidePress?: boolean }; |
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.
Don't we still need this? Why was it removed?
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.
No, it's an option from Floating UI config, but we don't use it anywhere in our components, so it can be removed.
Signed-off-by: atomiks <[email protected]>
Fixes #3008
This ensures pressing on nested children (in the React tree) doesn't dismiss on
pointermove(swiping/scrolling) by using the capture event trick, which is already done for the other event handlersAlso refactored from pointer to touch events because (at least on non-iOS/on Chrome DevTools),
pointermovestops firing once scroll starts, whiletouchmovealways fires which ensures the popover always dismisses while scrolling awayNote: #2978 refactors away the timeout for the
handleCaptureInsidelogic - I will rebase one of the PRs depending on which is merged first