MudBlazor icon indicating copy to clipboard operation
MudBlazor copied to clipboard

MudListItem: SecondaryText Feature

Open mckaragoz opened this issue 1 year ago • 1 comments

Description

Added SecondaryText parameter to MudListItem.

How Has This Been Tested?

Already using with MudListItemExtended.

https://github.com/MudBlazor/MudBlazor/assets/78308169/450bdca1-67ac-43f4-8daa-c371a73b2536

Type of Changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Documentation (fix or improvement to the website or code docs)

Checklist

  • [x] The PR is submitted to the correct branch (dev).
  • [x] My code follows the code style of this project.
  • [ ] I've added relevant tests.

mckaragoz avatar May 08 '24 11:05 mckaragoz

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.48%. Comparing base (28bc599) to head (418b754). Report is 185 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8921      +/-   ##
==========================================
+ Coverage   89.82%   90.48%   +0.65%     
==========================================
  Files         412      419       +7     
  Lines       11878    12197     +319     
  Branches     2364     2381      +17     
==========================================
+ Hits        10670    11036     +366     
+ Misses        681      627      -54     
- Partials      527      534       +7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 08 '24 11:05 codecov[bot]

@henon test done.

mckaragoz avatar May 12 '24 11:05 mckaragoz

Thanks!

henon avatar May 12 '24 12:05 henon

@henon I was reminded that mud is missing a basic implementation of <ul> <li>. Taking this opportunity, what do you guys think about introducing a simple implementation?

Alerinos avatar May 14 '24 14:05 Alerinos

@Alerinos I struggle to imagine how it should look like compared to MudList. Can you share examples from other toolkits that have what you mean?

henon avatar May 14 '24 15:05 henon

@henon image I made such a thing, it is a simple list. I am missing such a component in MudBlazor. You might want to make a new component called <MudSimpleList>?

Alerinos avatar May 14 '24 16:05 Alerinos

That is so simple that it is probably not worth adding this. Can't you just directly use <ul> and put <MudText> inside the <li> ?

henon avatar May 14 '24 16:05 henon

@henon You can manually do it but it spoils our look and doesn't fit with the rest. Having the Mud component everything is nicely compatible with each other, if we want to change the look of the layout everything changes nicely. We should have as many components as possible

I'm just in migration, MudList requires T parameter, shouldn't we allow nullable? Currently this is what my code looks like: image

Alerinos avatar May 16 '24 03:05 Alerinos

I'm just in migration, MudList requires T parameter, shouldn't we allow nullable?

Nullable types are allowed, for instance int?.

henon avatar May 16 '24 07:05 henon

@henon image I meant that MudList requires T, should be default value or nullable possible.

Alerinos avatar May 16 '24 07:05 Alerinos

I know, I would prefer that too, but Blazor doesn't allow it

henon avatar May 16 '24 08:05 henon

@henon Can't we set a default T, or make it null?

    public partial class MudList<T> : MudComponentBase, IDisposable : this(default(T))

Alternatively, it is possible to do “overloading” and create two classes, one with T and one without. In this case, we will handle everything.

Alerinos avatar May 16 '24 09:05 Alerinos

@henon Can't we set a default T, or make it null?

    public partial class MudList<T> : MudComponentBase, IDisposable : this(default(T))

Is this even legal C# ?

Alternatively, it is possible to do “overloading” and create two classes, one with T and one without. In this case, we will handle everything.

We tried that too, doesn't work. This is a technical limitation of Blazor that only Microsoft can fix.

henon avatar May 16 '24 09:05 henon