primevue icon indicating copy to clipboard operation
primevue copied to clipboard

Fix outside click listeners (from `click` to `mousedown`+`mouseup` events)

Open S3v3Nice opened this issue 1 year ago • 3 comments

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 the click event is the nearest element, containing both the target of the mousedown event and the target of the mouseup 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:

  1. Implement proper full click handling: capture mousedown and mouseup events, and check the targets of mousedown and mouseup events (both targets should not be the current component)
  2. Simply change the click event to mousedown, 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.

S3v3Nice avatar Jan 27 '24 20:01 S3v3Nice

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Visit Preview Apr 6, 2024 7:43pm
primevue-v4 ⬜️ Ignored (Inspect) Visit Preview Apr 6, 2024 7:43pm

vercel[bot] avatar Jan 27 '24 20:01 vercel[bot]

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.

mertsincan avatar Jan 28 '24 21:01 mertsincan

@S3v3Nice Are you certain this won't cause issues with actions like the following?:

  1. Say someone implements a drag-drop component.
  2. They click down outside a dialog, and drag into the dialog.

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.)

matthew-dean avatar Mar 07 '24 18:03 matthew-dean

We plan to work on this issue first in version 4. For now, we can close the PR due to conflicts.

tugcekucukoglu avatar Aug 22 '24 12:08 tugcekucukoglu