npm-dom / dom-event

Add/remove DOM events
http://npm.im/dom-event
34 stars 7 forks source link

fixes #5 and #6 and avoids evaluation on every on/off call #7

Closed phloe closed 9 years ago

phloe commented 9 years ago

This PR would fix legacy IE8 support mentioned in #5 and #6 but also avoid the (element.addEventListener || element.attachEvent) evaluation on every on call. Also makes.call() unnecessary...

azer commented 9 years ago

Could you send a PR that just fixes the bug without refactoring the code ?

phloe commented 9 years ago

I wanted to keep the basic structure as true to the original as possible - but it's just not possible (in a meaningful way) - I think... :(

If you can spot a way to keep it intact I'd love to hear it.

The culprit here is that attach/removeEvent need prepended event names and can't be used with call.

The PR code is lengthier - but because evaluation is done upfront (instead of on every call to the function) it will faster (with IE8 support) :)

phloe commented 9 years ago

You don't have to pull it - it's just working IE8 code. Feel free to write your own version :)

azer commented 9 years ago

fixed by 95b9e723a27da2e04bd3f816634e886c985625e4