fix(overlayable): check browser support for passive events #6710
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6710 +/- ##
=========================================
- Coverage 85.31% 85.3% -0.01%
=========================================
Files 298 298
Lines 7210 7220 +10
Branches 1802 1803 +1
=========================================
+ Hits 6151 6159 +8
- Misses 958 960 +2
Partials 101 101
Continue to review full report at Codecov.
|
| } | ||
| }) | ||
| window.addEventListener('testPassive', () => {}, opts) | ||
| window.removeEventListener('testPassive', () => {}, opts) |
There was a problem hiding this comment.
This line does nothing. There's an example of this on MDN: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Safely_detecting_option_support
There was a problem hiding this comment.
Having supportPassive = false in the catch block seems unnecessary since the var is initialized as false, but I guess it doesn't hurt anything either.
| @@ -211,7 +211,19 @@ export default Vue.extend<Vue & Toggleable & Stackable & options>().extend({ | |||
| if (this.$vuetify.breakpoint.smAndDown) { | |||
| document.documentElement!.classList.add('overflow-y-hidden') | |||
| } else { | |||
| window.addEventListener('wheel', this.scrollListener as EventHandlerNonNull, { passive: false }) | |||
| // IE11 does not support passive | |||
There was a problem hiding this comment.
Move this to util/helpers, we use passive in other places too.
There was a problem hiding this comment.
Do I also need to find all the other passive usages and update them to use this from helpers after I move it?
packages/vuetify/src/util/helpers.ts
Outdated
| }, Object()) | ||
| el.addEventListener('testListener', () => {}, testOpts) | ||
| el.removeEventListener('testListener', () => {}, testOpts) | ||
| } catch (e) { console.log(e) } |
There was a problem hiding this comment.
This should be rather console.warn() (console.error()?) but I'm not sure if this is even required, maybe you could skip it.
|
This will need to go to next as this particular functionality has already been converted to v2. |
|
So I won't be able to have this bug fix in the 1.5.x version? That was really why I did the pull request. v2 is still in alpha, so I assume it'll be a while before it's released. Plus it'll have a lot of breaking changes, so we probably won't upgrade immediately. |
|
@jacekkarczmarczyk @KaelWD In which version can expect this fix? |
|
I would also like to see this change in 1.5.x. We are using 1.5.7 in production and this is a serious UX problem in our case. Since 2.x is not ready for production yet, having this in 1.5.x would be great. :) |
johnleider
added
the
T: bug
|
This pull request is automatically deployed with Now. |
|
When this fix will be merged please ? this is a serious UX problem... |
Description
check browser support for passive events and only pass in options object if browser supports it
Motivation and Context
fixes #6617
How Has This Been Tested?
playground (scroll, open dialog, confirm scrolling doesn't happen while dialog is open, close and scroll page again)
Markup:
Types of changes
Checklist:
masterfor bug fixes and documentation updates,devfor new features and breaking changes).