slim-select icon indicating copy to clipboard operation
slim-select copied to clipboard

Relinquish use of event.stopPropagation()

Open rtarnaud opened this issue 4 years ago • 2 comments

Hello and thanks for this neat library!

As you may know, the usage of event.stopPropagation() is a bad idea, most of the time. Here is an article that explores the problem.

In my case, SlimSelect is tampering with my Sentry reports which are listening for the click events, among others. Given that SlimSelect "eats" them, they don't appear in bug reports which makes the investigation slightly more challenging than it should be.

Would you consider getting rid of all the event.stopPropagation() in the code. As it often turns out, stopping the propagation of an event is very rarely necessary.

rtarnaud avatar Nov 28 '19 19:11 rtarnaud

I would be open to removing them if i knew that current functionality wouldnt cause an issue.

But if i remember the ones i did add were added for a reason. Ill try to keep this on the list of things to look into.

Thanks.

brianvoe avatar Nov 29 '19 20:11 brianvoe

Removing event.stopPropagation() is not very difficult from my experience.

Generally, when you call it, this means that you simply wish to mark the event as "handled", so that it won't be processed by other listeners that would normally receive it when bubbling up if it is in the bubble phase (or down if it is in the capture phase but that's a rare use case).

However, you don't need to stop the event from propagating to achieve this result. Instead, you can simply call preventDefault which in addition to preventing the default action of the event, also marks the event as defaultPrevented, which a property that you can re-use in other listeners to check if the event has already been handled.

For example, let us say you have this markup:

<div id="d1">
  <div id="d2"></div>
</div>

Then you could process a click event on d2 with a listener on d2 without processing it with a listener set on d1 like this:

document.getElementById('d2').addEventListener('click', (e) => {
  e.preventDefault();
  // process e
}
document.getElementById('d1').addEventListener('click', (e) => {
  if (e.defaultPrevented)) {
    return;
  }

  // process e if the click has not targeted d2
}

rtarnaud avatar Dec 02 '19 21:12 rtarnaud

New version of slim select has been released. If this is still an issue let me know and ill try to take care of it.

brianvoe avatar Nov 21 '22 01:11 brianvoe