Skip to content

Conversation

@RyanLiu0235
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 72.17% when pulling e2696bd on stop2stare:refix-passive-event-listener into d92b328 on SSENSE:master.

}
// we are good to prevent the move and handle the translation
e.preventDefault();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when this line was written, maybe the author was still using chrome 56-, and option passive defaults to false.

@quinnlangille
Copy link
Member

Hmmm, I don't know if removing that line will achieve the same results. I think we still want our listeners to be passive: false here if we expect developers to be modifying the scroll behaviour.

@RyanLiu0235
Copy link
Contributor Author

@quinnlangille sure it'll achieve the same results. Please check out this chromestatus documentation and this addEventListener documentation. The reason why warnings occurred in console was that we invoked preventDefault in passive event listener. The former PR set passive to false, and that resolved this warning.
But, why does Chrome set this option to true?

This change prevents the listener from blocking page rendering while a user is scrolling.

The detailed documentation is here
SO, manually setting option passive to false is not the best choice, and even worsening the scrolling performances.
BUT, maybe you or @toddlawton wrote this line for some other special reasons or optimization. This PR just offers you guys another way to solve this problem.😄

@quinnlangille
Copy link
Member

Ok I agree, I'll merge this in and release with the next batch of updates! Thanks @stop2stare

@quinnlangille quinnlangille merged commit ff0065d into SSENSE:master Apr 19, 2018
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.

3 participants