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.

Sign up for GitHub

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

Upgrade waves #11

Merged
merged 5 commits into from
Aug 15, 2020
Merged

Upgrade waves #11

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fce7b8b
Waves: Update to 0.7.6. See #5779, #5472 and #3153
Niels-Be Aug 5, 2018
7a7bc06
Init waves for amd modules
Niels-Be Aug 10, 2020
0b6b70c
Fix chrome passive event warning
Niels-Be Aug 10, 2020
1c39343
Add materialize waves colors
Niels-Be Aug 14, 2020
3afddef
Add feature detection for passive event listeners
Niels-Be Aug 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions js/waves.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,9 +446,9 @@
removeListeners();
};

element.addEventListener('touchmove', touchMove, false);
element.addEventListener('touchend', hideEffect, false);
element.addEventListener('touchcancel', hideEffect, false);
element.addEventListener('touchmove', touchMove, { capture:false, passive: true });
element.addEventListener('touchend', hideEffect, { capture:false, passive: true });
element.addEventListener('touchcancel', hideEffect, { capture:false, passive: true });
Copy link
Member

Choose a reason for hiding this comment

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

eh I'm not sure about this. Have a look at Dogfalo#4720

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you're planning on merging passive event receivers already, no? #7

Copy link

Choose a reason for hiding this comment

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

Isn't this part of waves.js? I guess this is fine because I think the files are completely replaced with the new ones. Correct?

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this part of waves.js? I guess this is fine because I think the files are completely replaced with the new ones. Correct?

hmm the passive event, no it's not https://github.com/fians/Waves/blob/83fc07e2c0a31a10aa5dc2cf97b965113ddd882c/dist/waves.js#L443-L445

Copy link

Choose a reason for hiding this comment

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

Ah ok, but I think the 0.8 dev branch is the correct one, the master branch is an old version.

@Waeco please describe from where you got this file / version so we can check this and please split this into two commits. The first one with the new original files and a reference to where it is from and a second commit with your changes.

Copy link

Choose a reason for hiding this comment

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

I guess it's regarding this:

Screenshot_20200814-151147.jpg

Screenshot_20200814-151155.jpg

Copy link

Choose a reason for hiding this comment

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

@Waeco can you change the commit for the passive listeners according to the proposed solution at MDN?

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Browser_compatibility

Copy link

Choose a reason for hiding this comment

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

Alternatively we can drop the commit for passive event listeners and do it separately and with a global constant for all parts.

Copy link

Choose a reason for hiding this comment

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

grafik

Copy link
Author

Choose a reason for hiding this comment

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

I added it


var removeListeners = function() {
element.removeEventListener('touchmove', touchMove);
Expand All @@ -460,12 +460,12 @@
Effect.show(e, element);

if (isTouchAvailable) {
element.addEventListener('touchend', Effect.hide, false);
element.addEventListener('touchcancel', Effect.hide, false);
element.addEventListener('touchend', Effect.hide, { capture:false, passive: true });
element.addEventListener('touchcancel', Effect.hide, { capture:false, passive: true });
}

element.addEventListener('mouseup', Effect.hide, false);
element.addEventListener('mouseleave', Effect.hide, false);
element.addEventListener('mouseup', Effect.hide, { capture:false, passive: true });
element.addEventListener('mouseleave', Effect.hide, { capture:false, passive: true });
}
}
}
Expand All @@ -484,12 +484,12 @@
}

if (isTouchAvailable) {
body.addEventListener('touchstart', showEffect, false);
body.addEventListener('touchcancel', TouchHandler.registerEvent, false);
body.addEventListener('touchend', TouchHandler.registerEvent, false);
body.addEventListener('touchstart', showEffect, { capture:false, passive: true });
body.addEventListener('touchcancel', TouchHandler.registerEvent, { capture:false, passive: true });
body.addEventListener('touchend', TouchHandler.registerEvent, { capture:false, passive: true });
}

body.addEventListener('mousedown', showEffect, false);
body.addEventListener('mousedown', showEffect, { capture:false, passive: true });
};


Expand Down