-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
(WIP) Preventing extraneous click on the wrong element when tapping on overlay elements #2942
Conversation
Build successful! 🎉 |
Build successful! 🎉 |
<h1>Hello CodeSandbox</h1> | ||
<p> | ||
When closing a modal using a button using react aria useButton with a | ||
regular button behind, the button behind will immediately be clicked. | ||
</p> |
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.
imma go ahead and remove these and make the story/dialog scrollable
provisional approval for the changes @snowystinger made, can't approve my own PR though haha |
Do we want to do separate testing of just this branch before we merge it? |
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, tested stories in Chrome/FF/Safari, and iPhone Safari.
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
// prevent browsers from firing a delayed click event after touch end | ||
e.target.addEventListener('touchend', (e) => { | ||
e.preventDefault(); | ||
}, {passive: false, once: true}); |
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.
just discovered that something in this PR breaks tapping on checkboxes on my android phone (maybe other components too). Will need to dig to see if there is an alternative
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.
todo: double check if we need this in usePress
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.
@LFDanLu any update on the fix for this issue?
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.
unfortunately no, haven't had time to continue due to other priorities. There is a workaround in the attached issue if you are interested
Hello! Have any progress for this PR? |
@yarastqt Unfortunately haven't been able to pick this back up due to other priorities. If others would like to pick this up in the interim, that would be much appreciated! |
Closing for now until we pick this back up in our priorities. |
Closes #1513
✅ Pull Request Checklist:
📝 Test Instructions:
Follow descriptions in new stories
useModal/issues Click through modals
Date and Time/DatePicker/issues Click through calendar
MenuTrigger menu overlapping buttons
🧢 Your Project:
RSP