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.

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

disableRipple on list item leaves ripple class with overflow:hidden side effect #4156

Closed
willshowell opened this issue Apr 19, 2017 · 9 comments · Fixed by #4426
Closed

disableRipple on list item leaves ripple class with overflow:hidden side effect #4156

willshowell opened this issue Apr 19, 2017 · 9 comments · Fixed by #4426
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@willshowell
Copy link
Contributor

Bug, feature request, or proposal:

Bug

What is the expected behavior?

When using disableRipple on a list item, .mat-ripple class should be removed.

What is the current behavior?

.mat-ripple class remains on the mat-list-item-content.

What are the steps to reproduce?

https://plnkr.co/edit/lO8Eudx4GFgevN4XAy3n?p=preview

What is the use-case or motivation for changing an existing behavior?

Justification here. The .mat-ripple class adds overflow: hidden.

It may be entirely valid that list items should always have overflow: hidden on their content, but that should be done explicitly and not through a side effect of the ripple class.

@jelbourn jelbourn added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Apr 20, 2017
@jelbourn
Copy link
Member

Agreed- there shouldn't be a overflow: hidden on the list item when the ripple is disabled. It would be even better if there wasn't an overflow: hidden on the list-item at all, even when the ripple is present.

@devversion
Copy link
Member

I don't think it will be possible to drop the overflow: hidden when the ripple is present. The ripples are always big circles that would then overflow the list-item.

When the ripples are disabled, removing overflow: hidden can be done, but I'm not sure about that. It makes it more magic and could led to other side-changes in components.

@jelbourn
Copy link
Member

jelbourn commented Apr 21, 2017

Why not have the ripple container be a child of the list item that is position: absolute; left: 0; top: 0; right: 0; bottom: 0 with overflow: hidden?

@devversion
Copy link
Member

That would be the easiest solution. I did something similar in Material 1 with the underlying button. I'll play with it.

@willshowell
Copy link
Contributor Author

I think that solution would solve #4275 too (assuming that 4275's issue is stacking context related)

@devversion
Copy link
Member

I don't really think that those issues relate together. Issue #4275 is about overflow: hidden not working due to a missing position: relative.

@willshowell
Copy link
Contributor Author

Ahh ok. I initially read Jeremy's answer as proposing that the md-ripple directive automatically insert a child container with position: absolute, etc. I see now that he meant to create a child container just for this specific case.

@devversion
Copy link
Member

@willshowell Yeah exactly. Letting the md-ripple service take care of that would be really overkill.

devversion added a commit to devversion/material2 that referenced this issue May 8, 2017
* Removes the `overflow: hidden` on the list-items by moving the ripples into a child element that overlays the content.

Fixes angular#4156
@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 Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants