Can't resolve some theme properties
Closed this issue · 18 comments
I'm not sure if there is a way to get this, but it doesn't appear to be possible to use the theme.palette type within Styles.makeStyles similar to how it's shown in the examples:
const useStyles = makeStyles(theme => ({
root: {
color: theme.status.danger,
'&$checked': {
color: theme.status.danger,
},
},
checked: {},
}));theme.palette.primary for example is a U2<PaletteIntention,Color> and trying to use it like this:
let useStyles : unit -> _ = Styles.makeStyles <| fun (theme: Theme) ->
let drawerWidth = 200
{| appBar =
Styles.create
[ style.zIndex (theme.zIndex.drawer + 1)
style.cursor "default"
style.userSelect.none
style.display.grid
style.color (
match theme.palette.primary with
| U2.Case1 pI -> pI.dark
| U2.Case2 c -> c.``500``) ] |}Results in the following compiler error:
FABLE: Cannot type test: interface "Feliz.MaterialUI.Color"
BTW: Thanks for the tip about the markdown language thing, very handy!
Taking a step back from the actual type test error, I'm not sure the palette is supposed to be (or indeed can be) used that way.
For reference, here is the relevant MUI documentation. It says that
The intention value can either be a color object, or an object with one or more of the keys specified by the following TypeScript interface:
interface PaletteIntention { light?: string; main: string; dark?: string; contrastText?: string; };
It can be either a color or a intention object when setting it, but I'm not sure whether it can be a color object when getting it from a MUI-supplied theme argument. C.f. the default theme object where it's an intention object.
I encourage you to ask a MUI-related question on Stack Overflow, or if you think the MUI docs should be improved, to post an issue on their repo. If you find out that Feliz.MaterialUI should actually support something it doesn't today, let me know.
Yeah, I expected the value to just be a color string, and that's what it looks like it should be returning. That's why I posted the issue, I think the type should be different in Feliz.MaterialUI when it's returned vs when setting it. I only had the | U2.Case2 c -> c.``500`` because that was the only way to resolve the compiler errors.
Yeah, I expected the value to just be a color string
What exactly is it you expected to be a color string? For clarity, I was talking about theme.palette.primary, which is either a color object (not string) or a PaletteIntention.
I think the type should be different in Feliz.MaterialUI when it's returned vs when setting it
Related: #2 and in particular #2 (comment) by @Zaid-Ajaj.
Also, Feliz.MaterialUI must be based on documented MUI behaviour. It'd be fantastic if you could point to the MUI documentation (or a MUI issue) that shows how Feliz.MaterialUI is wrong. :) Otherwise I'd just be implementing based on guesswork from observed behavior, which might change.
I had expected to be able to just do this:
let useStyles : unit -> _ = Styles.makeStyles <| fun (theme: Theme) ->
let drawerWidth = 200
{| appBar =
Styles.create
[ style.zIndex (theme.zIndex.drawer + 1)
style.cursor "default"
style.userSelect.none
style.display.grid
style.color theme.palette.primary.dark ] |}I'm sorry if this all is just my naive understanding of MaterialUI, I'll make more of an effort to ensure I'm doing things correctly before bugging you with more issues ;).
The comment by @Zaid-Ajaj is exactly what I was getting at. As a user I would expect that if a function is returning an object to me everything I could try to use would be correct. Which could just be me being spoiled by .NET.
I had expected to be able to just do this:
Ah, I see.
I'm sorry if this all is just my naive understanding of MaterialUI, I'll make more of an effort to ensure I'm doing things correctly before bugging you with more issues ;).
No problem, not all parts of MUI are equally well documented (I'm confused by styling/theming stuff myself, particularly since there is a difference between creating/setting vs. getting).
As a user I would expect that if a function is returning an object to me everything I could try to use would be correct. Which could just be me being spoiled by .NET.
No, the goal is definitely to have "compiles = works" as far as possible here.
If you have any tips regarding a custom DSL for creating styles/themes, let me know. :) I haven't gotten to it yet but it's on my list (which is why #2 is still open).
To keep things focused, let's discuss theme/style creation DSL in #2. I'll leave this issue open.
I'm pretty busy right now, but I definitely want to help. If we don't have a solution/decision on it I'll see if I can help resolve it when I have more time.
@Shmew I had the same issue, I’ve resolved it by using the color property of the component, instead of setting it inside a style.
As you can see in the appBar docs https://material-ui.com/api/app-bar/
It also has a color prob. For dark mode you have to set it during theme creation https://material-ui.com/customization/palette/#type-light-dark-theme
Yeah I have different themes created for light/dark mode. The problem is that you can only set the component color from these options:
defaultinheritprimarysecondary
I don't really get why MUI lets you build a theme with alternate values for primary, secondary and so on; then restrict you to only those options in the component api.
I don't really get why MUI lets you build a theme with alternate values for
primary,secondaryand so on; then restrict you to only those options in the component api.
I don't get it. If you create a theme with custom primary and secondary, then using the color.primary and color.secondary component props should use your custom colors.
I may be wrong, but I think that often, it's sufficient to customize primary/secondary without needing to set specific colors for specific components. (Certainly not in all cases, but for most plain, non-branded Material Design use-cases, it should suffice.)
By the way, in case it's helpful, I just added dark mode support to this project. Specifically, see Renderer/App.fs.
Yeah I have dark mode working fine. I'm talking about if I want a component to be colored using the primary dark color like this:
Mui.textField [
textField.color.primary.dark // or
textField.color.primaryDark
...
]Basically I'm asking why we can even assign dark and light accents to our primary if we can't use them:
let getTheme themeType = Styles.createMuiTheme(jsOptions<Theme> <| fun t ->
t.palette <-
jsOptions<Palette> (fun p ->
p.``type`` <- themeType.PaletteType
p.primary <-
!^(jsOptions<PaletteIntention> (fun pi ->
pi.main <- themeType.PMain
pi.dark <- themeType.PDark))Oh, right. You are talking about the dark shade of the primary/secondary colors, not dark theme.
To be honest I'm not sure how to do this in MUI at all. If you figure out the way to do this in vanilla MUI, then let me know, and we can see if this is an issue with Feliz.MaterialUI. (If you already have figured out and are trying to tell me, then sorry, I apparently really need things spelled out today.)
Regarding textField.color.primary.dark and textField.color.primaryDark, those won't work, because they don't exist in MUI in the first place. Feliz.MaterialUI exposes MUI functionality, but I don't want to add "fake props" like that; I'd rather implement a good F# way to do it that mirrors the MUI way.
Regarding textField.color.primary.dark and textField.color.primaryDark, those won't work, because they don't exist in MUI in the first place. Feliz.MaterialUI exposes MUI functionality, but I don't want to add "fake props" like that; I'd rather implement a good F# way to do it that mirrors the MUI way.
Yeah I absolutely agree. I was just asking/complaining "why can't we do this?". As far as I know there isn't really a way to do this except by getting the theme and setting the prop color via CSS.
@cmeeren found an example of what I was trying to deal with above here
const styles = createStyles(theme => ({
dayWrapper: {
position: 'relative',
},
day: {
width: 36,
height: 36,
fontSize: theme.typography.caption.fontSize,
margin: '0 2px',
color: 'inherit',
},
customDayHighlight: {
position: 'absolute',
top: 0,
bottom: 0,
left: '2px',
right: '2px',
border: `1px solid ${theme.palette.secondary.main}`,
borderRadius: '50%',
},
nonCurrentMonthDay: {
color: theme.palette.text.disabled,
},
highlightNonCurrentMonthDay: {
color: '#676767',
},
highlight: {
background: theme.palette.primary.main,
color: theme.palette.common.white,
},
firstHighlight: {
extend: 'highlight',
borderTopLeftRadius: '50%',
borderBottomLeftRadius: '50%',
},
endHighlight: {
extend: 'highlight',
borderTopRightRadius: '50%',
borderBottomRightRadius: '50%',
},
}));I see, thanks! Are you able to find any part of the MUI docs which says that theme.palette.primary will always be a PaletteIntention object, never a color object?
By the way, in the meantime, you can work around the issue by unboxing:
style.color (unbox<PaletteIntention> theme.palette.primary).darkNothing in the docs that says for certain, but digging around it looks like all leaf nodes of the theme should be strings.
Thanks!
it looks like all leaf nodes of the theme should be strings.
IIRC the leaf nodes of the Color object are strings, too. The issue at hand is whether theme.palette.primary, .secondary, etc. are Color objects or PaletteIntention ojects (containing light, main, dark, and contrastText).
I just re-read the MUI docs on customizing the palette, and I now interpret them as saying that they will always be PaletteIntention objects, and that if setting them to a Color object, the values will simply be set according to this mapping:
palette: {
primary: {
light: palette.primary[300],
main: palette.primary[500],
dark: palette.primary[700],
contrastText: getContrastText(palette.primary[500]),
},
secondary: {
light: palette.secondary.A200,
main: palette.secondary.A400,
dark: palette.secondary.A700,
contrastText: getContrastText(palette.secondary.A400),
},
error: {
light: palette.error[300],
main: palette.error[500],
dark: palette.error[700],
contrastText: getContrastText(palette.error[500]),
},
},My conclusion is that in order to solve this particular issue, we need a proper theme creation/customization DSL. This DSL needs to account both for creating a (partial) theme from scratch, and for customizing an existing theme. Let's take that in #2.
With the new theme DSL, my understanding is that this is no longer an issue and works as expected. Let me know if that's incorrect.