angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.32k stars 6.73k forks source link

bug(mat-table): Non scoped style customizations behave differently from Scoped style customizations (css precedence conflict) #26628

Open simeyla opened 1 year ago

simeyla commented 1 year ago

Is this a regression?

The previous version in which this bug was not present was

Angular / Material 14

Description

There is a CSS precedence conflict for mat-table where the height of mat-row is not set to 52px (as per spec) unless scoped sass styles are used. This appears to be only in the 'flex' mat-table approach and not when using a native <table>.

Reproduction

Steps to reproduce:

Expected Behavior

See this line from the v15 migration docs under the Table component:

Row height is 52px instead of 48px.

That is the desired behavior for a table row per spec.

Actual Behavior

Example from Angular docs (no scoped styles)

If you look at this stackblitz example from the docs Basic use of <mat-table> (uses display flex) you will see an example which does not use Scoped style customizations.

That particular example defines the mat styling (no wrapping selector) like this:

 @include mat.all-component-themes($theme);

This mixin does indeed generate a height:52px selector as per spec, but it is overridden by a higher precedence selector - resulting in height: unset. The rows just end up at their 'natural' height. You can see that in this devtools screenshot.

image

The higher precedence style appears to be coming from the embedded scss styles in the Angular matTable component itself.

Forked version with scoped styles

Next I tried creating a fork with scoped styles for the stackblitz page (which is how my own application is configured).

.my-styles 
{
   @include mat.all-component-themes($theme);
}

Now the scoped selector has a higher precedence resulting in a row height of 52px as per spec.

image

So without scoped styles (as in the example stackblitz) the behavior is not to spec.

Another mat-table example in the docs

Another example on the same page Flex table where one column's cells has a greater height than others suggests using the following css to set the height of a column (the 'name' column).

.mat-column-name {
  height: 100px;
}
image

That works in the stackblitz, but as soon as you add scoped styles it fails spectacularly because the cells are of height 100px and the containing row is of height 52px!

Summary

For my own needs I actually want the unset behavior, with perhaps a min-height because my rows need to grow with their content. I can figure out a workaround for now.

I wanted to raise this since I can't find any other issues mentioning it and the current examples are not to spec since they don't use scoped styles.

Note: I've only observed this with mat-table-row heights (I only updated to v15 yesterday), but it is potentially a wider issue.

Environment

andrewseguin commented 1 year ago

In the table styles, there is a comment regarding this unset behavior:

// Flex rows should not set a definite height, but instead stretch to the height of their
// children. Otherwise, the cells grow larger than the row and the layout breaks.

It sounds like the unset behavior should always win. I think this will be soon resolved as we switch to a more token-based approach for our theming