[docs][material-ui] Fixed incorrectly spread shadows in templates

[Andarist]

All of those were spreading strings into arrays, resulting in many 1-character strings in the target array.

[mui-bot]

Netlify deploy preview

https://deploy-preview-43369--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad) No bundle size changes

Generated by :no_entry_sign: dangerJS against c4099631d240086a6813aa0efce1da2f4d99a6d9

[zanivan]

Thanks for tackling this @Andarist! This was actually a workaround added in https://github.com/mui/material-ui/pull/42445/commits/dc92ade70e8d3f52a5dec2afb4a3a9bcd9892f89 to address the test_types check—that is currently failing :(

It'd be good If you could fix the reason why it is failing as well, otherwise we'll have to find another solution. Any suggestions? cc @siriwatknp

[Andarist]

I can fix that issue but I need a little bit more context on it. What is this Shadows type even supposed to represent here? Why it's typed like this? 😅 Is "none" as the first item in that tuple a strong requirement? Does it always have to be a 25-item tuple? :D

[zanivan]

I’ll provide a bit more context—this shadow is based on the levels defined in Material UI's default theme. There are 25 shadow levels in total, starting from 0: "none". You can see all the default shadow levels here.

The goal behind these changes was to use either the custom shadow or "none," with distinct shadows for both light and dark modes. cc @noraleonte

[Andarist]

is the user required to configure all 25 levels? I understand they shouldn't configure more than 25 but is the system OK with less than 25?

[zanivan]

is the user required to configure all 25 levels? I understand they shouldn't configure more than 25 but is the system OK with less than 25?

I think so—I believe the issue comes up when we set different shadows for light and dark modes. For more details, I’ll probably need input from @siriwatknp

[Andarist]

Ok, so I made all of those shadows optional - this should address the issue (if I understand your constraints correctly)

[DiegoAndai]

The 25 shadows are expected if the user wants to override the theme. Otherwise, they could run into this:

AFAIK this is the only place that would cause an error.

I think the proper fix here would be generating the 25 values for the templates theme, would that be possible @zanivan?

If not possible, then we can bypass it casting to Shadows (as Shadows), but people using the templates might run into the issue.

[zanivan]

I think the proper fix here would be generating the 25 values for the templates theme, would that be possible @zanivan?

That’s possible, but I don’t see why someone would need 25 tokens for shadows, to be honest. I’m not even sure why Material UI has 25 values for it—I couldn’t find any resources about Material Design 2 that support this choice. While it does have elevation levels from 00dp to 24dp, there aren’t tokens for every single dp level.

All in all, I think the best approach is indeed to come up with shadows for all the 25 levels. I'll add this to the issue https://github.com/mui/material-ui/issues/41469 where we track the improvements for the templates (item 24).

[zanivan]

Since this isn't the right fix for the issue, I’ll close this PR for now.

[DiegoAndai]

@zanivan by the way, the approach used here is the correct one, besides the type change in Shadows, this seems incorrect:

...(mode === 'dark'
      ? 'hsla(220, 30%, 5%, 0.7) 0px 4px 16px 0px, hsla(220, 25%, 10%, 0.8) 0px 8px 16px -5px'
      : 'hsla(220, 30%, 5%, 0.07) 0px 4px 16px 0px, hsla(220, 25%, 10%, 0.07) 0px 8px 16px -5px'),

And I'm not sure why it's working 😅

If you think the 25 values are not required, we can warn users that they can't use up to certain elevations.

[Andarist]

Yep, the runtime change here is 100% better than what you had before. Maybe it could be improved further - if the user has always to provide all 25 levels but I'm not sure. Anyway, I'll leave this for you to fix as you have better context on it