feat(material-experimental/mdc-table): add mat-table selector #20994
Conversation
annieyw
added
P2
google-cla
bot
added
the
cla: yes
| @@ -16,7 +16,7 @@ import { | |||
| import {_DisposeViewRepeaterStrategy, _VIEW_REPEATER_STRATEGY} from '@angular/cdk/collections'; | |||
|
|
|||
| @Component({ | |||
| selector: 'table[mat-table]', | |||
| selector: 'mat-table table[mat-table]', | |||
There was a problem hiding this comment.
Shouldn't it be mat-table, table[mat-table]? The current selector is looking for tables inside a mat-table element.
Also it seems like there are other directives that are only scoped to table elements, like MatHeaderCell, MatCell etc. I wonder whether @andrewseguin did this intentionally?
There was a problem hiding this comment.
Additionally, the element-based APIs will need additional CSS to align with the flexbox-based style of the older version.
|
Added selectors for cells and rows |
annieyw
force-pushed
the
mdc-mat-table-selector
branch
3 times, most recently
from
5c9e15e
to
2393c88
Compare
…d tables styles into private mixin
|
For some reason CI didn't run. |
| $mat-row-horizontal-padding: 24px; | ||
|
|
||
| // Only use tag name selectors here since the styles are shared between MDC and non-MDC | ||
| @mixin mat-private-table-flex-styles { |
There was a problem hiding this comment.
Looking through this again, do you think that we should make these selectors a bit more specific? Currently mat-table {} will match both the MDC and non-MDC elements which could interfere with each other. We could change it to something like mat-table.mat-mdc-table, but I'm not familiar enough to know whether it'll make much of a difference.
There was a problem hiding this comment.
The reason for extracting the flex styles into a mixin is so that both MDC and non-MDC table can use the styles
There was a problem hiding this comment.
Wouldn't we want the MDC version to be using MDC's styles though? Otherwise it's basically a duplicate of our current table.
There was a problem hiding this comment.
MDC only has native table styles. The flexbox-based table is an Angular special.
There was a problem hiding this comment.
Wouldn't we still want to make it work using MDC's styles? Not necessarily in this PR, but we can work with the MDC team to split their styles up into more granular mixins so we can apply it to our flexbox table too.
There was a problem hiding this comment.
I had been assuming the only styles we were applying exclusively to the flex table were the actual flexbox styles. If there are styles common between them, we should just use the appropriate css classes for those.
…ex based tables styles into private mixin
annieyw
added
the
merge ready
* feat(material-experimental/mdc-table): add mat-table selector * fix(material-experimental/mdc-table): extract variables and flex based tables styles into private mixin * fixup! fix(material-experimental/mdc-table): extract variables and flex based tables styles into private mixin (cherry picked from commit f0f3748)
…is initialized The MDC-based table tries to add a class to its internal `tbody` element and it assumes that it's always going to find it. The assumption used to be correct since we only supported native `table` elements for the MDC implementation, however as of angular#20994 we also support flexbox-based ones which throw an error, because they don't have a `tbody`. These changes add a check to prevent the error and include a couple of sanity tests to catch issues like this in the future. I've also reworked the tests so that they use the MDC-based `MatPaginator` and `MatTableDataSource`, instead of the base ones.
…is initialized (#21428) The MDC-based table tries to add a class to its internal `tbody` element and it assumes that it's always going to find it. The assumption used to be correct since we only supported native `table` elements for the MDC implementation, however as of #20994 we also support flexbox-based ones which throw an error, because they don't have a `tbody`. These changes add a check to prevent the error and include a couple of sanity tests to catch issues like this in the future. I've also reworked the tests so that they use the MDC-based `MatPaginator` and `MatTableDataSource`, instead of the base ones.
…is initialized (#21428) The MDC-based table tries to add a class to its internal `tbody` element and it assumes that it's always going to find it. The assumption used to be correct since we only supported native `table` elements for the MDC implementation, however as of #20994 we also support flexbox-based ones which throw an error, because they don't have a `tbody`. These changes add a check to prevent the error and include a couple of sanity tests to catch issues like this in the future. I've also reworked the tests so that they use the MDC-based `MatPaginator` and `MatTableDataSource`, instead of the base ones. (cherry picked from commit 2255b66)
…r#20994) * feat(material-experimental/mdc-table): add mat-table selector * fix(material-experimental/mdc-table): extract variables and flex based tables styles into private mixin * fixup! fix(material-experimental/mdc-table): extract variables and flex based tables styles into private mixin
…is initialized (angular#21428) The MDC-based table tries to add a class to its internal `tbody` element and it assumes that it's always going to find it. The assumption used to be correct since we only supported native `table` elements for the MDC implementation, however as of angular#20994 we also support flexbox-based ones which throw an error, because they don't have a `tbody`. These changes add a check to prevent the error and include a couple of sanity tests to catch issues like this in the future. I've also reworked the tests so that they use the MDC-based `MatPaginator` and `MatTableDataSource`, instead of the base ones.
|
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. |
Have consistent selectors between original MatTable and MDC-based MatTable.
Note that the mat-table tag is used for users that are styling tables with flex as per the MatTable documentation: https://material.angular.io/components/table/overview#tables-with-display-flex:~:text=Tables%20with%20display%3A%20flex,-The