-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(ripple): not fading out on touch devices #12488
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.
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
fix(ripple): not fading out on touch devices #12488
Conversation
From that issue, it makes it sound like this only happens if you're constantly tapping, no? If that's the case, wouldn't it be simpler just to cap the number of ripples that could appear? |
It also happens if you just scroll constantly around and accidentally touch an elements that have ripples registered. Capping the number of ripples that can appear, would be another possible solution, but I guess that kind of limits how the ripples work. Each ripple target has its own Each ripple should work independently from other ripples. e.g. there can be people who touch multiple elements at the same time (I have no specific example), or there are ripples manually being launched for a tutorial on a page. Also I've wanted to switch away from timeouts a long time because by using This should not only fix the issue, but also give us a small performance improvement for the ripples. |
091c448
to
4e93368
Compare
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
e7efe4c
to
eafe877
Compare
Hi @devversion! This PR has merge conflicts due to recent upstream merges. |
eafe877
to
e9c5d2b
Compare
@jelbourn Rebased. |
97a26fa
to
64ccbfd
Compare
Hi @devversion! This PR has merge conflicts due to recent upstream merges. |
64ccbfd
to
b415e23
Compare
Hi @devversion! This PR has merge conflicts due to recent upstream merges. |
d00ffa7
to
5edee00
Compare
2615e48
to
532f8c9
Compare
532f8c9
to
7d9ef8b
Compare
I've rebased this one again. Could we get another presubmit on it? cc. @andrewseguin @mmalerba I see in the caretaking notes that ripples persisted unexpectedly. I assume this happened in tests? Hopefully not too many apps test the ripple timing. |
4ffbb33
to
0c7325c
Compare
@jelbourn I've rebased this one again. I think we should still merge this as it improves overall performance of the ripples, and fixes a bug that results in ripples persisting while the user scrolls on mobile. Can we have another presubmit for this? I assume this this fix is still relevant since our MDC-based components also use our ripple. cc. @wagnermaciel |
a2fe2e9
to
ef82a87
Compare
ef82a87
to
95e0b8a
Compare
Deployed dev-app to: https://ng-comp-dev--pr-12488-cbc5f7c46800842ffcc8c3bad0ff5939-vpfh9aye.web.app |
0562bb9
to
54595ac
Compare
54595ac
to
88170e9
Compare
…lling * Makes the ripple animations no longer dependent on `setTimeout` that does not always fire properly / or within the specified duration. (related chrome issue: https://bugs.chromium.org/p/chromium/issues/detail?id=567800) * Fix indentation of a few ripple tests * Fixes that the speed factor tests are basically not checking anything (even though they will be removed in the future; they need to pass right now) Fixes angular#12470
…en scrolling Backwards compatibility change for g3 tests using just transition: none without the noopanimations module
…en scrolling Support transition-duration
6f67dbf
to
cbc5f7c
Compare
This reverts commit 65fb5f4.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
setTimeout
that does not always fire properly / or within the specified duration. (related chrome issue: https://bugs.chromium.org/p/chromium/issues/detail?id=567800)Fixes #12470