Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.99%. Comparing base (470147a) to head (db63590).
Report is 1 commits behind head on dev.

@@            Coverage Diff             @@
##              dev   #10884      +/-   ##
==========================================
- Coverage   91.01%   90.99%   -0.03%     
==========================================
  Files         429      429              
  Lines       13932    13934       +2     
  Branches     2691     2693       +2     
==========================================
- Hits        12680    12679       -1     
  Misses        649      649              
- Partials      603      606       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielchalmers I've made a few changes. First, I've undone all previous modifications. I added a Density object with a PaddingStep property to the main Theme. I have removed the effect of the Dense parameter in the MudTable. Now padding is managed by the PaddingDensity parameter, which can have four values and apply padding to the table, including the PaddingStep value in the various calculations. Below are two examples. By default, the PaddingParameter is set to Max. It makes no changes to the default table in that case for the display. Please let me know your thoughts.

public class Density
{
    public string PaddingStep { get; set; } = "4px";
}

...

public MudTheme()
{
    Density = new Density();
    PaletteLight = new PaletteLight();
    PaletteDark = new PaletteDark();
    Shadows = new Shadow();
    Typography = new Typography();
    LayoutProperties = new LayoutProperties();
    ZIndex = new ZIndex();
    PseudoCss = new PseudoCss();
}

Thanks for testing that - I think we need a more top-down approach with several components, but it would have to be saved for a major release regardless since it's a breaking change.

I talked to @Garderoben and we think the simplest solution would be to use MudTd.Class to change the padding, or do it directly through CSS on the app side, example: https://try.mudblazor.com/snippet/maQzYQlqKmSiBrqc.

It would mean adding the same class to each cell, so a new property (e.g. CellClass) could be added to make that easier. What do you think?

I think we need a more top-down approach with several components, but it would have to be saved for a major release regardless since it's a breaking change.

Totally agree with you.

I talked to @Garderoben and we think the simplest solution would be to use MudTd.Class to change the padding, or do it directly through CSS on the app side, example: https://try.mudblazor.com/snippet/maQzYQlqKmSiBrqc.

Thanks for discussing this with @Garderoben and for the snippet!

It would mean adding the same class to each cell, so a new property (e.g. CellClass) could be added to make that easier. What do you think?

Please take a look at https://try.mudblazor.com/snippet/GamfamvrcrAaNSPm. A common class is used to define cells for MudTd and for MudTh based on your previous example. Would your idea be to have something like below replace all Class="mud-table-small-cell" in the example? If so, I think it would avoid a lot of repetition and could be really useful. What about the cells when displaying supplementary information in the MudTr ?

<MudTable CellClass="mud-table-small-cell"...>

It would mean adding the same class to each cell, so a new property (e.g. CellClass) could be added to make that easier.

I've undone all previous changes. I've added a CellClass parameter at the <MudTable> level. It will be applied to <MudTh> and <MudTd>. Not to <MudTr>. I've also added an example. See below for the two examples. What do you think?

PS: I've also modified three unit tests to take into account the changes brought about by this implementation.

+1 for CellClass. Look at https://mudblazor.com/components/datagrid#visual-styling for comparison. Although we might not want all the ...Style and ...StyleFunc params which were added before we decided to not add any more Style parameters.

I like it, this is much better!

But lets not forget about DataGrid, i would like to have CellClass in there as well then since its very similar. Or what do you say @henon @danielchalmers

But lets not forget about DataGrid, i would like to have CellClass in there as well then since its very similar.

DataGrid has it already and in fact has even better styling suppport. I was suggesting the opposite, getting everything that datagrid supports supported in table as well (except the Style variants, we decided not to add any more Style properties and we may even remove them in the future).

On a side note, the CellClass property for the MudDataGrid is defined in the Column component. In this PR, the CellClass is defined at the MudTable level.

The advantage to define the CellClass at the MudTable level is that is applied everywhere. No need to define the same class in other components to apply the same class to the cells inside the MudTable component.

One solution could be to do the following, for consistency :

A - Rename CellClass and CellClassFunc for MudDataGrid component in ColumnClass and ColumnClassFunc. This is a breaking change.
B - Move CellClass (and CellClassFunc ?) for MudDataGrid component at the MudDataGrid level. This is a breaking change as the point A. These properties therefore behave in the same way as this PR.
C - Add RowClass, RowClassFunc, HeaderClass, HeaderClassFunc, FooterClass, FooterClassFunc, ColumnClass and ColumnClassFunc to MudTable component (if this is possible).
D - Add as well CellClassFunc to MudTable component.

What do you think? Or keep this PR as it is? Or another solution ?

MudGrid is column based and MudTable is not. They have a different design philosophy and so I think we're fine with the different behavior of CellClass. So I would not change MudGrid, meaning I'd not do A and B.

If you want to add C in this PR you are welcome to do so. If you don't have the time then just add CellClassFunc. It should have parameters to identify the row and column of the table so could theoretically have a different style for every cell in the table. I guess the func would also differ from MudGrid because the grid does not need information about the column as a parameter in the func.

Here is an updated version of the PR. I am pretty sure there is a better way to implement the CellClassFunc parameter.

PS : I do not know how to fix the last check.

Will leave this to @henon

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud