Action: Mark sidenav touchmove event listeners as passive

[doughballs]

Chrome throws 3 violation messages, directly connected to the touchmove event listener attached to sidenav:

materialize.min.js?ver=1.0.0:6 [Violation] Added non-passive event listener to a scroll-blocking 'touchmove' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952

You can see the issue on any materialize site - even the docs - by reducing screen size so that the sidenav is off the screen, then check console > messages:

https://materializecss.com/

The fix, it seems, is to amend lines 132, 134 & 136 of sidenav.js. I honestly don't know what the broader implications of this are but the warnings disappear.

Changed from:

this.dragTarget.addEventListener('touchmove', this._handleDragTargetDragBound);
this._overlay.addEventListener('touchmove', this._handleCloseDragBound);
this.el.addEventListener('touchmove', this._handleCloseDragBound);

to:

this.dragTarget.addEventListener('touchmove', this._handleDragTargetDragBound, { passive: true});
this._overlay.addEventListener('touchmove', this._handleCloseDragBound, { passive: true});
this.el.addEventListener('touchmove', this._handleCloseDragBound, { passive: true});

Maybe someone could confirm this, and then talk me through how to make this into a change?!!?

[DanielRuf]

A bit related:

0b6b70c (see also the discussion in there)
3afddef

[doughballs]

A bit related:

0b6b70c (see also the discussion in there)
3afddef

Cool. Ok, I will try to make my first PR...!

[DanielRuf]

What's useful: you can append something like - closes #20 or - fixes #20 to a commit in a PR so GitHub links it to the relevant issue which is then automatically closed when the PR is merged, and there is some information like shown that it fixes / closes the mentioned issue.

[wuda-io]

Hello @doughballs, thanks for your solution. It would be cool to further improve materializecss with you contribution.
Whats the current status of the PR? best regards

[DanielRuf]

Hi @doughballs,

did you find some time to start the work on a PR or would you suggest that someone else picks this up and you can support them with a PR review?

[doughballs]

@DanielRuf so time is not an issue, but I’m not sure where it’s up to. I thought I had created a PR? Forgive me, first time contributing so not 100% sure what the hell I’m doing 🙃

happy for someone else to pick up, or I can finish off...but may need a little guidance.

[DanielRuf]

@DanielRuf so time is not an issue, but I’m not sure where it’s up to. I thought I had created a PR? Forgive me, first time contributing so not 100% sure what the hell I’m doing 🙃

Great. This is no problem.

I would like to invite @materializecss/members to help here so we can foster a great community. Who wants to describe the needed steps and guide @doughballs?

[LoganTann]

@DanielRuf so time is not an issue, but I’m not sure where it’s up to. I thought I had created a PR? Forgive me, first time contributing so not 100% sure what the hell I’m doing 🙃

happy for someone else to pick up, or I can finish off...but may need a little guidance.

Any updates about the PR ? If you've found which line to change, your first PR will require some time to understand how to properly contribute in such projects, but you'll get habits after one or two supplementary contributions.

I might consider helping there if there is no activity.

[LoganTann]

Hello, I will start working on this issue today.

Is the global variable passiveIfSupported safe to use inside navbar.js ? Or do I just need to follow exactly the OP answer ?

[LoganTann]

What ? It's already fixed ?!

See 3cbfb25
and #21

Then should we also add passive to every touchmove events, and not just sidenav ?

[Smankusors]

oh wait, we seems forgot to close this? Is this still an issue?

[DanielRuf]

Probably still open since the original PR did not lmention / link this issue here.

And see #20 (comment)

Closing as fixed.