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

fix(event): addEventListener set passive option to false by default #10751

Closed
wants to merge 3 commits into from

Conversation

btea
Copy link
Contributor

@btea btea commented Apr 20, 2024

When I bind the wheel event to the input, and then call e.preventDefault() to prevent the default event under certain circumstances, a warning message will appear on the console.

image

example

Copy link

github-actions bot commented Apr 20, 2024

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 90.8 kB (+11 B) 34.5 kB (+4 B) 31.1 kB
vue.global.prod.js 148 kB (+11 B) 53.8 kB (+6 B) 48 kB (+5 B)

Usages

Name Size Gzip Brotli
createApp 50.8 kB (+11 B) 19.9 kB (+3 B) 18.1 kB (+5 B)
createSSRApp 54.2 kB (+11 B) 21.2 kB (+3 B) 19.3 kB (+1 B)
defineCustomElement 53.1 kB (+11 B) 20.6 kB (+4 B) 18.8 kB (+9 B)
overall 64.5 kB (+11 B) 24.9 kB (+2 B) 22.5 kB (-20 B)

@ferferga
Copy link

What you want is @wheel.prevent. See https://vuejs.org/guide/essentials/event-handling.html#event-modifiers

@btea
Copy link
Contributor Author

btea commented Apr 20, 2024

No, the prevent modifier will prevent all default behaviors, but the scenario I need is to prevent the default behavior under specific conditions. You can check out the example I provided above.

@baiwusanyu-c
Copy link
Member

/ecosystem-ci run

@vue-bot
Copy link

vue-bot commented Apr 22, 2024

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools failure success
nuxt failure success
pinia success success
primevue success success
quasar success success
radix-vue success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros success success
vuetify failure failure
vueuse success success
vue-simple-compiler success success

Copy link
Member

@baiwusanyu-c baiwusanyu-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@baiwusanyu-c baiwusanyu-c added the ready for review This PR requires more reviews label Apr 22, 2024
@yyx990803
Copy link
Member

I'm not sure if I understand your problem, in what browsers are these warnings shown? I can't reproduce them.

@btea
Copy link
Contributor Author

btea commented Apr 22, 2024

It can be reproduced on both chrome and edge. You should click on the Show console sidebar icon and then select verbose as shown below:
image

@yyx990803
Copy link
Member

From my understanding:

  • You either make the event passive (which means you can't preventDefault())
  • Or you preventDefault() on the event which is not passive (and get the warning)

So the point of this PR is just to suppress the warning but the code is still doing what it warns you against. If that's the case, I don't think this PR makes sense. Vue should not default to hiding meaningful warnings.

@yyx990803 yyx990803 closed this Apr 29, 2024
@btea
Copy link
Contributor Author

btea commented Apr 29, 2024

Yes, the above modifications are only for handling warning messages.
For this scenario, if I want to solve this warning message, then I can only manually bind the event el.addEventListener('wheel', fn, { passive: false }) myself, and cannot rely on Vue's event binding mechanism to solve it (it seems there is no way to set passive: false).
image

@btea btea deleted the fix/addEventListener-add-passive-option branch April 30, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR requires more reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants