-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(ripple): cleanup ripple even when the touch is very quick on iOS #11302
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
I signed it! |
CLAs look good, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! I just had one minor piece of feedback on the code.
Additionally, in order to merge this, we'll need the commit message to be updated to follow our commit guidelines as indicated in the first checkbox of the PR 😉 This is needed so that we can generate a proper Changelog to let people know about this fix 😃
src/core/services/ripple/ripple.js
Outdated
@@ -280,7 +280,9 @@ InkRippleCtrl.prototype.handleMousedown = function (event) { | |||
* mouseup, touchend or mouseleave event) | |||
*/ | |||
InkRippleCtrl.prototype.handleMouseup = function () { | |||
autoCleanup(this, this.clearRipples); | |||
setTimeout(function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use this.$timeout
instead of setTimeout
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this breaks the CI tests, any idea on why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't due to your change. There is a problem running tests against the latest AngularJS SNAPSHOT
. We're investigating now. Thanks for the quick update!
this issue sometimes occurred when md-button with ripple was tapped repeatedly Fixes angular#11069
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
this issue sometimes occurred when md-button with ripple was tapped repeatedly Fixes #11069
PR Checklist
Please check that your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #11069
http://streaka.testing.videos.s3.amazonaws.com/screencast_2018-05-29_14-38-08.mp4
What is the new behavior?
Issue doesn't occur anymore, seems fine
Does this PR introduce a breaking change?
Other information