-
Notifications
You must be signed in to change notification settings - Fork 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
Fix outside click listeners (from click
to mousedown
+mouseup
events)
#5160
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
Hi, Thanks a lot for your contribution! We didn't do mousedown for some reason but I can't remember. I will discuss this with the team and get back to you. |
@S3v3Nice Are you certain this won't cause issues with actions like the following?:
Logically, the dialog should not close because the click did not start / finish outside the dialog. It sounds like what you are describing would immediately dismiss the dialog if a mousedown started outside the dialog and finished within it (or possibly would close the dialog before they could finish dragging). (I'm not saying someone should do this pattern, and ideally they would be disabling "click outside to close", but I'm just pointing out that switching from click to mousedown might be a breaking change for some scenarios. I personally interpret "click outside to close" as inferring BOTH mousedown AND mouseup outside the dialog, not one or the other. IMO it's important to not be over-eager in closing dialogs.) |
click
to mousedown
event)click
to mousedown
+mouseup
events)
Fixes #2597.
The problem is related to the fact that the
click
event is used to recognize a click on the outside of an element, so that a planned action (e.g. closing a modal) is triggered by a full click, but only when the click starts OR ends on the outside (because the target of theclick
event is the nearest element, containing both the target of themousedown
event and the target of themouseup
event). Because of this, for example, selecting text inside a dialog and then moving the cursor to outside (while holding down the mouse button) causes the dialog to close.There may be two solutions to this problem:
mousedown
andmouseup
events, and check the targets ofmousedown
andmouseup
events (both targets should not be the current component)click
event tomousedown
, so the modal will close immediately when the user mouse down outside the modal, and will not close when the user starts clicking inside the modal and finishes outside.I chose the second solution because it's simpler, and it's standard practice (if the user starts clicking outside the modal, it's more likely that he wants to close it, and why not do it immediately, without waiting for the click to complete). By the way, this is how it was already implemented in the Calendar component.UPD:
For some reasons listed in the comment below, it was decided to use the first way of solving the problem -
mousedown
+mouseup
events - in almost all components.