Shmew / Feliz.MaterialUI

Feliz-style Fable bindings for Material-UI
https://shmew.github.io/Feliz.MaterialUI/
MIT License
70 stars 19 forks source link

Overriding MuiCssBaseline has incorrect signature #55

Closed Shmew closed 3 years ago

Shmew commented 4 years ago

The current signature of theme.overrides.muiCssBaseline.global' takes a list of style properties, where the proper syntax is to set the elements that they apply to:

const theme = createMuiTheme({
  overrides: {
    MuiCssBaseline: {
      '@global': {
        html: {
          WebkitFontSmoothing: 'auto',
        },
      },
    },
  },
});

I'm not sure how you want to handle this as far as type safety goes, but I can confirm this works:

theme.overrides.muiCssBaseline.global' [
    unbox ("html", (createObj !![
        // styles go here
    ]))
]
cmeeren commented 4 years ago

Thanks for the report. I haven't had the chance to investigate this yet, and I'm not sure I'll be able to do it this week. It's on my agenda, but I have a lot of other stuff there at the moment. I'll get to it sooner or later. Let me know if you find out what's wrong, or how the signature should be.

Shmew commented 4 years ago

No rush! I didn't end up using it, and there's a workaround if I or anyone else comes across it in the meantime.

Let me know if you find out what's wrong, or how the signature should be.

Well it's really just a matter of if you want to create overloads for all the valid html elements or not. Otherwise theme.overrides.muiCssBaseline.global': (string * (IStyleProperty list) list is valid when used with my second snippet.

So that would look like this I believe:

[<Erase>]
type muiCssBaseline =
    static member inline global' (overrides: (string * (IStyleAttribute list)) list) : IThemeProp =
        unbox ("overrides.MuiCssBaseline.@global", createObj !!(overrides |> List.map (fun (k,v) -> k, createObj !!v)))
cmeeren commented 3 years ago

Hmm, but is string the only option for the keys? What if you want to override a MUI component?

And I see that inputBase also has a global name, do you know if that works similarly?

I'm having a hard time finding official MUI docs on this.

Shmew commented 3 years ago

This case is specific only to MuiCssBaseline AFAIK, and so the keys will only be actual html elements.

cmeeren commented 3 years ago

Do you know how the @global rule works with classes, i.e.

Mui.cssBaseline [
  cssBaseline.classes.global' "someClassName"
]

Is that something that makes sense?

Shmew commented 3 years ago

Since cssBaseline.classes only supports @global you can just do it like the normal components: cssBaseline.classes "someClassName".

cmeeren commented 3 years ago

Are you confusing @global with the root class name that all MUI components have? I don't think any components other than cssBaseline and inputBase have @global, so that's why I'm unsure whether it's usable in classes at all.

Shmew commented 3 years ago

I could just be confused, but this is what I was looking at that gave me that impression.

cmeeren commented 3 years ago

I'm thoroughly confused about the class stuff, so I'm leaving that part alone until someone complains.

I have checked in a fix now. Are you able to give it a try and see if it works?

Shmew commented 3 years ago

Yep looks good

cmeeren commented 3 years ago

Great, thanks a lot! v1.2.1 is in the pipeline now.