Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.50%. Comparing base (28bc599) to head (af415db).
Report is 210 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #8987      +/-   ##
==========================================
+ Coverage   89.82%   90.50%   +0.67%     
==========================================
  Files         412      395      -17     
  Lines       11878    12138     +260     
  Branches     2364     2369       +5     
==========================================
+ Hits        10670    10986     +316     
+ Misses        681      620      -61     
- Partials      527      532       +5     

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

You now get all the good stuff from Records (comparison, etc) and can do this:

I think you need to give more explanation about why this change is useful. Otherwise, it sounds like we should switch all classes to records just because "we get good stuff". I just don't see how this change is being utilized in our code.

Also, isn't a record mainly for making immutable data, because by default the properties in records are get and init when you declare them in the primary constructor? Here, we have get and set properties.

@ScarletKuro I don't want to duplicate my palette styles

MudTheme _theme = new()
    {
        PaletteLight = new PaletteLight()
        {
            Primary = "#776be7",
            PrimaryLighten = "#978dec",
            Info = "#776be7",
            AppbarText = "#ffffffe6",
            Background = "#f7f6fb",
            TextDisabled = "#424242",
            HoverOpacity = 0.1,
        },

        PaletteDark = new PaletteDark()
        {
            Primary = "#776be7",
            Info = "#776be7",
            AppbarText = "#ffffffe6",
            TextPrimary = "#ffffffcc",
            TextSecondary = "#ffffff99",
            TextDisabled = "#ffffffb2",
            HoverOpacity = 0.1,
        },
}

    Palette _basePalette = new()
        {
            Primary = "#776be7",
            Info = "#776be7",
            AppbarText = "#ffffffe6",
            HoverOpacity = 0.1,
        };

    MudTheme _theme = new()
        {
            PaletteLight = _basePalette with
            {
                PrimaryLighten = "#978dec",
                Background = "#f7f6fb",
                TextDisabled = "#424242",
            },

            PaletteDark = _basePalette with
            {
                TextPrimary = "#ffffffcc",
                TextSecondary = "#ffffff99",
                TextDisabled = "#ffffffb2",
            },
}

Though I should have made Palette non-abstract

I think you need to give more explanation about why this change is useful. Otherwise, it sounds like we should switch all classes to records just because "we get good stuff". I just don't see how this change is being utilized in our code.

What's wrong with data classes being turned into records?

Also, isn't a record mainly for making immutable data, because by default the properties in records are get and init when you declare them in the primary constructor? Here, we have get and set properties.

Should palette be immutable since it's only used once right? 🤷

What's wrong with data classes being turned into records?

Nothing at all. I just want to know your use case for this change. Are we making this change for fun, or is there a good reason? This is a breaking change at the API level, since records can only inherit from other records. Therefore, if people have their own classes inheriting from it, they will need to change those to records as well. That's why I'm asking if this change is worth making, considering it is a breaking change.

Should the palette be immutable since it's only used once, right? 🤷

1. Yes, I think it should be immutable and values set only once for an instance.
2. Since you changed it to a record, it's another reason to make this immutable. Even though record allows mutable properties, I believe the main idea behind records is to make immutability easy and fast, along with the with syntax. Also, the GetHashCode should always work only with readonly/immutable properties. In your code, this is not the case, which means it's not good code.

Your edits for simpler docs are spot on! Is there a need for a palette for colorblind folks? If so, I think I could put one together with someone to help test. (A different PR of course)

@jperson2000 I definitely have accessibility concerns about certain contrasts in the default palette (ex: the text is way too gray) which I have to address in my own apps. I don't know if changing that is acceptable but I bet a separate one for color blindness would be. Maybe we can work something out - cc @henon

+1 on immutability. Shall we make this change in the next major version together with other theming changes?

+1 on immutability. Shall we make this change in the next major version together with other theming changes?

Wise idea

@henon @ScarletKuro I've updated the PR to exclude record changes so it should be easy to merge now

+1 on immutability. Shall we make this change in the next major version together with other theming changes?

Feel free to add to v8 project.