Describe the bug
When zoom in using the touchpad, browser gives many duplicated console errors as below.
[Intervention] Unable to preventDefault inside passive event listener due to target being treated as passive. See https://www.chromestatus.com/features/6662647093133312
To Reproduce
Using MacOS Google Chrome Version 73.0.3683.86 (Official Build) (64-bit)
When Using the touch pad to zoom in at then timeline canvas, the error logs come out.
And not only the date range changes, also whole page is zoomed in.
Expected behavior
When zoom in guesture at the timeline canvas, only the date range changes, the whole page should not be zoomed in.
Library Version
"react-calendar-timeline": "^0.23.0",
Desktop (please complete the following information):
I'm seeing this is as well on the latest version of chrome
Here's how I prevent Chrome from scrolling the page when I scroll the timeline.
Since scrolling the timeline requires pressing a modifier like ctrl, alt or cmd, you can call custom methods when a modifier is touched (disable scroll) and on key up (re-enable scrolling)
Here is how I implemented it (psuedo code):
constructor(props){
super(props)
this.state = {
currentKey: ''
}
}
componentWillUnmount(){
document.removeEventListener('keydown', this.handleKeyPress)
document.removeEventListener('keyup', this.handleKeyUp )
}
componentDidMount(){
document.addEventListener('keydown', this.handleKeyPress)
document.addEventListener('keyup', this.handleKeyUp )
}
handleKeyPress =(e)=> {
this.setState({ currentKey: e.keyCode })
if( e.keyCode === 16 // shift
|| e.keyCode === 17 // ctrl
|| e.keyCode === 18 // alt/option
|| e.keyCode === 91 // cmd (mac)
) {
console.log('pressed modifier to prevent scrolling page so can scroll timeline')
this.disableScroll()
} else {
this.enableScroll()
}
}
handleKeyUp = (e)=>{
this.enableScroll()
}
disableScroll() {
if (window.addEventListener) // older FF
window.addEventListener('DOMMouseScroll', this.prevDef, false)
window.onwheel = this.prevDef // modern standard
window.onmousewheel = document.onmousewheel = this.prevDef // older browsers, IE
window.ontouchmove = this.prevDef // mobile
}
enableScroll() {
if (window.removeEventListener)
window.removeEventListener('DOMMouseScroll', this.prevDef, false)
window.onmousewheel = document.onmousewheel = null
window.onwheel = null
window.ontouchmove = null
}
prevDef=(e)=> {
e = e || window.event
if (e.preventDefault)
e.preventDefault()
e.returnValue = false
}
Based on this example: https://stackoverflow.com/questions/4770025/how-to-disable-scrolling-temporarily#4770179 and https://codepen.io/anon/pen/ZOzaPW
This does not prevent touch screen scrolling. I found another solution so that it also works on iPad #544
@stahlmanDesign
Sorry, I tried your method, but I didn't see that solves the problem.
@FutureArchitect-wang2449 @stahlmanDesign @richardgirges
I've been following this issue on the react repo here
A lot of opinions are being thrown there and I am still not settled on a permanent solution since react doesn't have an api to change passive events.
I would like to hear what do you guys think?
@Ilaiwi
In my opinion, what about add something like
window.addEventListener("wheel", func, {passive: false} );
into Timeline ScrollElement?
According to the Google Dev Page
Making wheel scrolling fast by default, for the performance purpose, they make wheel listener "passive: true" default, and here we want to customize the listener func, so explicitly make "passive: false" seems to be the "right" way.
@FutureArchitect-wang2449 could you open a PR with this solution please?
@FutureArchitect-wang2449 @richardgirges
could any of you guys confirm that the fix is working here?
https://csb-ov2pr5j9zy-npcizlrdri.now.sh/
This fixed everything for me https://github.com/facebook/react/issues/14856#issuecomment-478144231 and I removed all the code above that I was playing with
@stahlmanDesign my problem with this is that this code will affect all the react code that you have which could cause other problems that this library should not be concerned about
I understand, it's not a long-term solution. I just wanted people to know that it is simpler than what I had proposed
@Ilaiwi
could any of you guys confirm that the fix is working here?
https://csb-ov2pr5j9zy-npcizlrdri.now.sh/
Yes, that works for my environment.
@Ilaiwi I added the PR (by another account @wsysuper)
Temporarily fix the issue by adding the snippet https://github.com/facebook/react/issues/14856#issuecomment-478144231 to src/index.js
released in 0.25.1
@Ilaiwi Thank you for fixing this. That helps!
Most helpful comment
@FutureArchitect-wang2449 @richardgirges
could any of you guys confirm that the fix is working here?
https://csb-ov2pr5j9zy-npcizlrdri.now.sh/