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

Jump to bottom

bug(matRipple): Ripple effect sometimes persists when used with cdkDrag #29159

Closed
1 task
gkrzysztofbury opened this issue May 31, 2024 · 8 comments · Fixed by #29184 or #29323
Closed
1 task

bug(matRipple): Ripple effect sometimes persists when used with cdkDrag #29159

gkrzysztofbury opened this issue May 31, 2024 · 8 comments · Fixed by #29184 or #29323
Assignees
@crisbeto
Labels
area: material/core P4 A relatively minor issue that is not relevant to core functions

Comments

@gkrzysztofbury
Copy link

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

When setting up cdkDrag with multiple cdkDropList components, initiating a drag very quickly can cause the ripple effect to remain frozen on the component.

Reproduction

Expected Behavior

After the drag ends, the ripple effect should be removed from the button.

Actual Behavior

The ripple effect remains on the button indefinitely.

Cause:

When the drag starts, the original button is removed from the DOM, and a placeholder is created in its place. Immediately after creation, the ripple state is set to RippleState.FADING_IN (source). However, the actual FADING_IN transition does not occur because the button is removed from the DOM.

Proposed Solution:

Modify the code at line 184 to set the state to RippleState.VISIBLE, and then set it to RippleState.FADING_IN on the transitionstart event. This way, the ripple will be removed here:
Remove Ripple

Environment

  • Angular:
  • CDK/Material:
  • Browser(s):
  • Operating System (e.g. Windows, macOS, Ubuntu):
@gkrzysztofbury gkrzysztofbury added the needs triage This issue needs to be triaged by the team label May 31, 2024
@andrewseguin andrewseguin added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: cdk/drag-drop and removed needs triage This issue needs to be triaged by the team labels Jun 3, 2024
@crisbeto crisbeto self-assigned this Jun 4, 2024
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 4, 2024
@crisbeto
fix(material/core): hide ripples inside drag&drop elements
fb61b72 
When we create previews and placeholders for the drag&drop module, we clone the DOM node in its current state. This means that if there are ripples, they'll be in the clone as well.

These changes add a default styles to hide the ripple in such cases since it'll never disappear.

Fixes angular#29159.
@crisbeto crisbeto mentioned this issue Jun 4, 2024
Merged
crisbeto added a commit that referenced this issue Jun 4, 2024
@crisbeto
fix(material/core): hide ripples inside drag&drop elements (#29184)
c4a1407 
When we create previews and placeholders for the drag&drop module, we clone the DOM node in its current state. This means that if there are ripples, they'll be in the clone as well.

These changes add a default styles to hide the ripple in such cases since it'll never disappear.

Fixes #29159.
crisbeto added a commit that referenced this issue Jun 4, 2024
@crisbeto
fix(material/core): hide ripples inside drag&drop elements (#29184)
dfc19e2 
When we create previews and placeholders for the drag&drop module, we clone the DOM node in its current state. This means that if there are ripples, they'll be in the clone as well.

These changes add a default styles to hide the ripple in such cases since it'll never disappear.

Fixes #29159.

(cherry picked from commit c4a1407)
@gkrzysztofbury
Copy link
Author

This fix doesn't work, the problem is that mat-ripple-element stays inside the original element, not placeholder/preview

Screenshot

@crisbeto
Copy link
Member

crisbeto commented Jun 5, 2024

I just tried it and couldn't reproduce this issue with the code at head:

Screen.Recording.2024-06-05.at.10.12.32.mov

@gkrzysztofbury
Copy link
Author

You have to:

  1. have a setup with 2 connected cdkDragLists
  2. click & drag very fast, like in single move, so the drag starts before ripple animation starts playing

@crisbeto
Copy link
Member

crisbeto commented Jun 5, 2024

I tried it again, but couldn't reproduce it. I also tried slowing down the animation to see if that makes a difference, but it didn't. Can you try it on this link? https://ng-dev-previews-comp--pr-angular-components-29197-dev-p8hzqfu4.web.app/drag-drop

It's our demo app with your repro pasted in.

@gkrzysztofbury
Copy link
Author

Screen.recording.2024-06-05.12.54.30.webm

Here you go, it takes some practice to do this quick move, but once you get it is easy to reproduce

@gkrzysztofbury
Copy link
Author

I can do this on ChromeOS and MacOS (Chrome & Safari)

@crisbeto crisbeto added P4 A relatively minor issue that is not relevant to core functions and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Jun 5, 2024
@crisbeto
Copy link
Member

crisbeto commented Jun 5, 2024

I managed to reproduce it now, but I had to use a mouse so I can click around faster.

@crisbeto crisbeto reopened this Jun 5, 2024
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 26, 2024
@crisbeto
fix(material/core): add fallback if ripples get stuck
c09194f 
Currently ripples assume that after the transition is started, either a `transitionend` or `transitioncancel` event will occur. That doesn't seem to be the case in some browser/OS combinations and when there's a high load on the browser.

These changes add a fallback timer that will clear the ripples if they get stuck.

Fixes angular#29159.
@crisbeto crisbeto mentioned this issue Jun 26, 2024
Merged
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 26, 2024
@crisbeto
fix(material/core): add fallback if ripples get stuck
73f2fa3 
Currently ripples assume that after the transition is started, either a `transitionend` or `transitioncancel` event will occur. That doesn't seem to be the case in some browser/OS combinations and when there's a high load on the browser.

These changes add a fallback timer that will clear the ripples if they get stuck.

Fixes angular#29159.
crisbeto added a commit that referenced this issue Jun 27, 2024
@crisbeto
fix(material/core): add fallback if ripples get stuck (#29323)
674538b 
Currently ripples assume that after the transition is started, either a `transitionend` or `transitioncancel` event will occur. That doesn't seem to be the case in some browser/OS combinations and when there's a high load on the browser.

These changes add a fallback timer that will clear the ripples if they get stuck.

Fixes #29159.
crisbeto added a commit that referenced this issue Jun 27, 2024
@crisbeto
fix(material/core): add fallback if ripples get stuck (#29323)
e5c5f15 
Currently ripples assume that after the transition is started, either a `transitionend` or `transitioncancel` event will occur. That doesn't seem to be the case in some browser/OS combinations and when there's a high load on the browser.

These changes add a fallback timer that will clear the ripples if they get stuck.

Fixes #29159.

(cherry picked from commit 674538b)
essjay05 pushed a commit to essjay05/angular-components that referenced this issue Jun 27, 2024
@crisbeto @essjay05
fix(material/core): add fallback if ripples get stuck (angular#29323)
5740659 
Currently ripples assume that after the transition is started, either a `transitionend` or `transitioncancel` event will occur. That doesn't seem to be the case in some browser/OS combinations and when there's a high load on the browser.

These changes add a fallback timer that will clear the ripples if they get stuck.

Fixes angular#29159.
DBowen33 pushed a commit to DBowen33/components that referenced this issue Jul 3, 2024
@crisbeto @DBowen33
fix(material/core): add fallback if ripples get stuck (angular#29323)
598ab66 
Currently ripples assume that after the transition is started, either a `transitionend` or `transitioncancel` event will occur. That doesn't seem to be the case in some browser/OS combinations and when there's a high load on the browser.

These changes add a fallback timer that will clear the ripples if they get stuck.

Fixes angular#29159.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@crisbeto crisbeto

Labels
area: material/core P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Milestone
No milestone
3 participants
@crisbeto @andrewseguin @gkrzysztofbury