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

Document limitation of matRipple on native table rows and possible solutions #11883

Closed
jchafik opened this issue Jun 21, 2018 · 25 comments · Fixed by #22797
Closed

Document limitation of matRipple on native table rows and possible solutions #11883

jchafik opened this issue Jun 21, 2018 · 25 comments · Fixed by #22797
Assignees
Labels
area: material/core docs This issue is related to documentation help wanted The team would appreciate a PR from the community to address this issue P4 A relatively minor issue that is not relevant to core functions

Comments

@jchafik
Copy link

jchafik commented Jun 21, 2018

Bug, feature request, or proposal:

Bug

What is the expected behavior?

The ripple effect should be contained within the table row.

What is the current behavior?

When using <tr mat-row> in Angular 6 instead of <mat-row ...>, the ripple effect isn't contained within its parent container. I believe this is caused by the fact that div (the container used for the ripple) is not a valid child of the tr element it's applied to.

What are the steps to reproduce?

https://stackblitz.com/edit/angular-material2-issue-myteuk

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

N/A

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular 6.x

@devversion devversion marked this as a duplicate of #11165 Jul 24, 2018
@devversion devversion marked this as not a duplicate of #11165 Jul 24, 2018
@devversion devversion reopened this Jul 24, 2018
@devversion
Copy link
Member

Actually that's a different issue than #11165. The issue seems to be that position: relative on elements with display table-row does not have any effect.

Meaning that the ripples won't have the proper positions. Also overflow: hidden does not work. Both things are the foundation for the ripples.

@Chris2011
Copy link

Have exactly the same problem. Any workrounds here?

@devversion
Copy link
Member

A workaround would be using <mat-row> instead of the native <tr> element.

@julien-vonoetinger
Copy link

I have faced the same problem too and it would be nice to have a fix for this.

@daniel-mckenna-zz
Copy link

Also have the same issue.

@glains
Copy link

glains commented Oct 26, 2018

I am having the same issue as well, but <mat-row> is not working as well.

@rocos-yishi
Copy link

  <table mat-table [dataSource]="items">
    <!-- Name Column -->
    <ng-container matColumnDef="name">
      <mat-header-cell *matHeaderCellDef> Name </mat-header-cell>
      <mat-cell *matCellDef="let item"> {{item.name}} </mat-cell>
    </ng-container>

    <mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row>
    <mat-row *matRowDef="let row; columns: displayedColumns;" matRipple></mat-row>
  </table>

That works for me. :)

@theunreal
Copy link

Using Material 7.0.1 - I still experience this issue.

@MatthiasKunnen
Copy link
Contributor

I believe this is because <tr> may only contain <td> and <th> (See spec). The overflow: hidden on the <tr> is simply not respected by the browser (Chrome nor Firefox). Using a ripple on mat-row works, contrary to @glains' comment. I tested using 7.0.1.

Opinion
After trying out the ripple on a table I came to the conclusion that, in my case, it's not a good idea due to the following reasons:

  1. The row is far larger horizontally compared to vertically, this makes the ripple quite ugly
  2. The ripple distracts when selecting text

@nalexander50
Copy link

@rocos-yishi solution worked for me. It would be nice if it were added to the Examples section for Table on the Angular Material website.

@glains
Copy link

glains commented Feb 24, 2019

@MatthiasKunnen Using the mat-row element, the ripple effect indeed works now.

@simeyla
Copy link

simeyla commented Apr 4, 2019

Still not working?

You probably forgot imports: [ MatRippleModule ]

@gautamkrishnar
Copy link

@simeyla thats the exact cause.. ripple is working fine for me even on tr.

@ccjmne
Copy link
Contributor

ccjmne commented Apr 22, 2019

@gautamkrishnar It's not working for me either, and @MatthiasKunnen's comment does make sense to me.
Would you be able to share a working StackBlitz using <tr mat-row matRipple> please? We can't get it to work wit the "plain" <table mat-table><tr mat-row><td mat-cell> model :(

@qortex
Copy link

qortex commented Feb 26, 2020

Any valid workaround on the <table mat-table><tr mat-row><td mat-cell> scenario?

@spyter
Copy link

spyter commented Feb 26, 2020

@MicMicMon, have you tried using the flex version of the table?

I'm using that in a project and matRipple works for me. If you have buttons in the row, you need to work some magic to disable the ripple effect from being propagated by the buttons, but other than that it seems to work.

See this link for more info:

https://material.angular.io/components/table/overview#tables-with-code-display-flex-code-

@qortex
Copy link

qortex commented Feb 26, 2020

Thanks @spyter

Actually, because of the warning in the docs, and all the good reasons listed here, I wanted to stick with the <table mat-table> scenario - but indeed, moving to <mat-table> makes ripple work out of the box.

@alicanertop
Copy link

Thanks @spyter

Actually, because of the warning in the docs, and all the good reasons listed here, I wanted to stick with the <table mat-table> scenario - but indeed, moving to <mat-table> makes ripple work out of the box.

When i used like u said then sticky scenario will broken

Sorry for bad english

@QuickScoP3s
Copy link

QuickScoP3s commented May 4, 2020

2 years after the original issue, Angular 9 still has this issue...
Luckily the workaround by @rocos-yishi still works, thanks!

@mmalerba mmalerba added the needs triage This issue needs to be triaged by the team label May 20, 2020
@devversion
Copy link
Member

devversion commented May 27, 2020

@QuickScoP3s Do you have any proposal in mind? This is a limitation in browsers we are hitting due to our ripple implementation. tr, td elements are not displayed as blocks, hence overflow: hidden does not work. the incorrect position is also caused by a limitation of native table elements. See: https://bugs.chromium.org/p/chromium/issues/detail?id=417223.

There is not much we can do about this. Given there is a reasonable solution of using the non-native table when needing ripples, I'm closing this issue.

Here is an alternative example that shows how to make ripples work for native table-rows: https://stackblitz.com/edit/comp-11883?file=app/app.component.html.

@QuickScoP3s
Copy link

QuickScoP3s commented May 27, 2020

I didn't mean to sound rude (apologies if I did), but since the issue was still open, I though the team was still working on it. I've been using the approach to work around the problem, as suggested, with the downside being the sticky headers not working... I'd loved seeing a solution as I originally wanted to use sticky headers 😉 I had no idea that the table itself was the limiting factor, but thanks for clearing this up!

@devversion
Copy link
Member

@QuickScoP3s No worries at all! Sorry if the state of the issue was unclear. I think we failed communicating clearly why this hasn't been resolved yet. Hopefully my latest message is clear enough. I've also created a demo StackBlitz that shows how ripples can be used together with native tables. That might be helpful to some people.

@Spencer-Easton
Copy link

Spencer-Easton commented May 29, 2020

@devversion StackBliz demo works as advertised. If this issue is not easily solvable your work around should be discoverable from the docs. Would save a bit of hair pulling ;)

Edit:
Demo needs:
<th mat-header-row *matHeaderRowDef="displayedColumns; sticky:true"></th>
changed to
<tr mat-header-row *matHeaderRowDef="displayedColumns; sticky:true"></tr>

@devversion devversion reopened this May 29, 2020
@devversion
Copy link
Member

@Spencer-Easton Sounds reasonable. I'll re-open this as we could certainly have this documented in the ripple overview. Any help on this would be appreciated as it's most likely low-priority for us.

@devversion devversion added docs This issue is related to documentation help wanted The team would appreciate a PR from the community to address this issue P4 A relatively minor issue that is not relevant to core functions and removed needs triage This issue needs to be triaged by the team labels May 29, 2020
@devversion devversion changed the title [Table] mat-ripple does not work with new <table mat-table> format. Document limitation of matRipple on native table rows and possible solutions May 29, 2020
@devversion devversion self-assigned this May 25, 2021
devversion added a commit to devversion/material2 that referenced this issue May 25, 2021
@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 Jun 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/core docs This issue is related to documentation help wanted The team would appreciate a PR from the community to address this issue P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.