Skip to content

fix(material-experimental/mdc-table): add background color; disable hover styles; fix sticky columns #22723

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

Merged
merged 3 commits into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/material-experimental/mdc-table/_table-theme.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@use 'sass:map';
@use '@material/theme/theme-color' as mdc-theme-color;
@use '@material/data-table/data-table' as mdc-data-table;
@use '@material/data-table/data-table-theme' as mdc-data-table-theme;
Expand Down Expand Up @@ -41,6 +42,11 @@
mdc-data-table-theme.$sort-icon-color: $orig-sort-icon-color;
mdc-data-table-theme.$sort-icon-active-color: $orig-sort-icon-active-color;
mdc-data-table-theme.$stroke-color: $orig-stroke-color;

.mat-mdc-table {
$background: map.get($config, background);
background: theming.get-color-from-palette($background, 'card');
}
}

@mixin typography($config-or-theme) {
Expand Down
17 changes: 16 additions & 1 deletion src/material-experimental/mdc-table/table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// change since the table did not previously apply it.
// TODO: Add a mixin to MDC to set the layout instead of including this override,
// see this issue: https://github.com/material-components/material-components-web/issues/6412
.mdc-data-table__table {
.mat-mdc-table {
table-layout: auto;
}

Expand All @@ -27,3 +27,18 @@
mat-row.mat-mdc-row, mat-header-row.mat-mdc-header-row, mat-footer-row.mat-mdc-footer-row {
border-bottom: none;
}

// Cells need to inherit their background in order to overlap each other when sticky.
// The background needs to be inherited from the table, tbody/tfoot, row
// (already set in MDC), and cell.
.mat-mdc-table tbody, .mat-mdc-table tfoot,
.mat-mdc-cell, .mat-mdc-footer-cell {
background: inherit;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having these inherit the background, would it make sense to just assign the background to the cells in the theme? This feels like something that we could forget about if we remove the background from the theme.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean assign the card background to the cells? The benefit with the inherit approach is that users can style the table host element with a background and it works as expected, rather than identifying that they need to style the cell background.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, makes sense. Let's keep it like this.

}

// Disable hover styling while MDC uses an opacity for its color.
// When the hover style is used with sticky cells, the opacity shows the cells overlapping.
.mat-mdc-table .mat-mdc-row:hover,
.mat-mdc-table .mat-mdc-footer-row:hover {
background-color: inherit;
}