Skip to content
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

Removing problematic preventDefault lines #39

Closed
wants to merge 1 commit into from
Closed

Removing problematic preventDefault lines #39

wants to merge 1 commit into from

Conversation

aphelionz
Copy link

Removing the lines that were causing the preventDefault warning (now an error!) Fixes #38

Removing the lines that were causing the `preventDefault` warning (now an error!) Fixes #38
@aleofreddi
Copy link
Owner

Hi Mark,
thanks for the PR. I'm wondering if removing these lines cause a regression on older versions or other browsers. What do you think?

@aphelionz
Copy link
Author

Good question! I can test with browserstack if we can get a public demo updated.

@aleofreddi
Copy link
Owner

I've noticed there are other preventDefault usages around, we should fix the issue for all of them

@aleofreddi
Copy link
Owner

aleofreddi commented Apr 19, 2020

Any news on this? Looking around I've found this one as proposed solution:

(passiveSupported && (active || el == window.document || el == window.document.body || el == window)) ? 
    el.addEventListener(name, fn, { passive: false, capture: bubble }) 
    : el.addEventListener(name, fn, bubble || false);

@aphelionz
Copy link
Author

I admit I haven't had much time to look at this since the original PR... your snippet looks good, I bet that can be abstracted to a function and re-used

@aphelionz aphelionz closed this Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

preventDefault warning
2 participants