Add some filtering [feature-request] about default-passive-events HOT 8 CLOSED

Hi @vpArth thats a good point.

So you are suggesting something like this in here:

EventTarget.prototype.addEventListener = function(type, listener, options) {
  const validTypes = ['scroll', 'touchstart', '..etc'];

  if (!validTypes.includes(type)) return;
}

I think that will make sense, Im just not sure if adding {passive: true} to other events have some benefit, that way if we introduce this change we will lose that perf gain. Thoughts? :)

from default-passive-events.

We will lose that perf gain

passive events is just a way to say to browser that your listener never call preventDefault, so scroll should not be blocked. It's not about addEventListener call itself, but about listeners call time.
I mean, we can't lose its benefits with heavy addEventListener.

About implementation perf...
It should be O(1), at least:

var forcePassiveMap = {scroll: true, touchstart: true, ...};
if (forcePassiveMap[type]) {
  // manipulate options
}
superMethod.call(...)

But to be some more customizable I prefer to add it somewhere accesible from outer code.
For example,

var  forcePassiveMap = {...}; // default map
EventTarget.prototype.isEventTypeForcedToBePassive = function(type) {
  return forcePassiveMap[type];
}

And then

  if (this.isEventTypeForcedToBePassive(type)) {
    // manipulate options
  }

Pros is it can be redefined by user... Cons is excess complexity...

In my own code I'd preffer to use Symbol for method/map name, but this small script has not any exported object to get it, and excess dependency will brake es5 environment...

from default-passive-events.

This lib currently makes click handlers passive by default, which means default cannot be prevented on them, for no apparent benefit. This will break many apps as currently written; some filtering is needed.

from default-passive-events.

Hi! I ran across this issue today with ngx-bootstrap's modal, and ng2-currency-mask. Doing a cursory search here on github, it seems a lot of projects reference e.preventDefault() (which means probably a lot more that don't use e as a variable name).

That being said, I wanted to ask if instead of trying to add some sort of magic to addEventListener, would it be possible to encapsulate Event.preventDefault()?

e.g., something along these lines:

EventTarget.prototype.addListener = function(...) {
    // ... your code

    this.options = options;

    // call the original addListener()
}

Event.prototype.preventDefault = function() {
    if (!this._options.passive) {
        // call the original preventDefault()
    }
}

EventTarget.prototype.dispatchEvent = function(event) {
    event._options = this.options;

    // call the original dispatchEvent()
}

Basically, just pass the options through to the Event object so that the new preventDefault() can know if the event is passive or not (why they don't already do this is beyond me :)).

Anyway, thank you for your efforts! However this gets solved, this will be an extraordinarily important library. :)

from default-passive-events.

@dohpaz42 Thanks for the suggestion! Why do you it will be better to do it that way? Currently we only override one native method and we don't mutate the event instance, which I think its a good practice.

What you suggest will involve overriding 3 methods + this.options = options; which will add to every single event. So thats why i think is not a good idea.

Does it make sense to you? thanks!

from default-passive-events.

So, will you be happy if we make the library to only add the defaults for certain events? Like, scroll, touchstart, etc?

I will be more than happy to do that change

from default-passive-events.

Will any of you be interested of being a contributor of the library? I'll more than happy to add you as collaborators 💃

from default-passive-events.

@zzarcon After some more thought, I think I'm of the mindset that as painful as it is right now, I personally think I'd rather pester the offending developers who a) aren't defaulting to using passive listeners appropriately, and 2) using preventDefault() when they shouldn't be.

The more that I think about it, the more I can't see a sledgehammer like this working correctly in all situations.

from default-passive-events.