Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update for Material-UI 4 #62

Open
cmeeren opened this issue May 29, 2019 · 9 comments
Open

Update for Material-UI 4 #62

cmeeren opened this issue May 29, 2019 · 9 comments

Comments

@cmeeren
Copy link
Contributor

cmeeren commented May 29, 2019

Material-UI 4 has been released, with support for hooks (React 16.8.0). Any plans to update Fable.MaterialUI in the foreseeable future?

This could also solve #50 and #53.

@mvsmal
Copy link
Owner

mvsmal commented May 30, 2019

Hello @cmeeren, I am currently working on it. Will share some progress soon. Would you like to participate? I would like to hear an opinion about the architecture from you, as an active library user.

@cmeeren
Copy link
Contributor Author

cmeeren commented May 30, 2019

Glad to hear you're working on it! I'm getting married in three weeks and am busy with preparations, so I can't promise I can contribute much code, but I'm certainly available for discussion. :)

@Luiz-Monad
Copy link

Luiz-Monad commented Jun 6, 2019

How about makeStyles, withStyles is horribly slow, and doesn't plug very well into Elmish.

Example of what I wanted to do.

let styles ( theme: Theme ) = {|
    Button = style [
        S.Margin "5vh 0 5vh 0"
    ]
|}
let render dispatch model styled =
    let styled = inferType styles styled
    paper styled.Background [   .... omited   ]
let view = render |> withStyles styles 
code.

/// Declares a style class.

type CssStyle = {
    Props: CSSProp list
    ClassName: string
}
let style cssProps =
    Some { Props = cssProps; ClassName = "" }

type private IProps<'Style, 'Model, 'Msg> =
    abstract member key: string with get, set
    abstract member style: ( ITheme -> 'IStyle list ) with get, set
    abstract member model: 'Model with get, set
    abstract member view: ( IClasses -> 'Model -> ('Msg -> unit) -> ReactElement ) with get, set
    abstract member dispatch: ('Msg -> unit) with get, set
    inherit IClassesProps

type private StyledComponent<'Style, 'Model, 'Msg when 'Model : equality> (p) =
    inherit PureStatelessComponent<IProps<'Style, 'Model, 'Msg>> (p)

    let viewFun (p: IProps<'Style, 'Model, 'Msg>) = 
        p.view p.classes p.model p.dispatch
    
    let viewWithStyles sheet = 
        withStyles (StyleType.Func sheet) [] viewFun
    
    // override this.shouldComponentUpdate(nextProps, _nextState) =
    //         not <| (=) this.props.model nextProps.model

    override this.render () = 
        ReactElementType.create (viewWithStyles this.props.style) this.props []

/// Created a new styled component from a component and a style sheet.
let withStyles styleSheet fn ( key, dispatch, model ) =
    let mutable css: 'S = Unchecked.defaultof<'S>
    let idx k = ( k, css?(k) )
    let conv ( k, v : CssStyle ) = Styles.Custom ( k, v.Props ) :> IStyles
    let convBack k v = Some { Props = v.Props; ClassName = k }
    let createSheet theme =
        css <- styleSheet theme
        JS.Object.keys css |> Seq.map ( idx >> conv ) |> List.ofSeq
    let applySheet ( clz: IClasses ) model dispatch =
        let styleIt k = css?(k) <- convBack clz?(k) css?(k)
        JS.Object.keys css |> Seq.iter styleIt
        fn dispatch model css
    let props = createEmpty<IProps<_, _, _>>
    props.key <- key
    props.style <- createSheet
    props.model <- model
    props.view <- applySheet
    props.dispatch <- dispatch
    ofType<StyledComponent<'S, _, _>, _, _> props []

This works but doesn't do what we think it would do because the way withStyles is implemented, and changing Object.ReferenceEquals .

I've also tried using React's pure function.

code.

type CssStyle = {
    Props: CSSProp list
    ClassName: string
}
let style cssProps =
    Some { Props = cssProps; ClassName = "" }

type private IProps<'Style, 'Model, 'Msg> =
    abstract member key: string with get, set
    abstract member style: ( ITheme -> 'IStyle list ) with get, set
    abstract member model: 'Model with get, set
    abstract member view: ( IClasses -> 'Model -> ('Msg -> unit) -> ReactElement ) with get, set
    abstract member dispatch: ('Msg -> unit) with get, set
    inherit IClassesProps
let withStyles styleSheet fn key dispatch model =
    let mutable css: 'S = Unchecked.defaultof<'S>
    let idx k = ( k, css?(k) )
    let conv ( k, v : CssStyle ) = Styles.Custom ( k, v.Props ) :> IStyles
    let convBack k v = Some { Props = v.Props; ClassName = k }
    let createSheet theme =
        css <- styleSheet theme
        JS.Object.keys css |> Seq.map ( idx >> conv ) |> List.ofSeq
    let applySheet ( props: IProps<_, _, _> ) =
        let styleIt k = css?(k) <- convBack props.classes?(k) css?(k)
        JS.Object.keys css |> Seq.iter styleIt
        fn props.dispatch props.model css
    let comp = FunctionComponent.Of ( applySheet, "styled", 
                fun x y -> equalsButFunctions x.model y.model )
    let styler = withStyles (StyleType.Func createSheet) [] comp
    let props = createEmpty<IProps<_, _, _>>
    props.key <- key
    props.style <- createSheet
    props.model <- model
    props.dispatch <- dispatch
    FunctionComponent.Of ( 
        (fun p -> ReactElementType.create styler p []),
        memoizeWith = (fun x y -> equalsButFunctions x.style y.style) ) 
        props

I think I could use F# lazy () in some place and supply createSheet with the same reference every time to withStyles and it would at least make it be faster, but it still doesn't solve the #53 problem, and the React VDOM becomes a mess with lots of useless HoC components.

So, the real solution would be using makeStyles, and that's what I'm trying now.

withStyles is also too heavy, I just wanted to pass a dictionary of styles and get back the strings..
And for some reason it's also stupid and trigger redraw even when inside pure component with pure components inside it (or I'm becoming crazy and seeing things).
https://github.com/mui-org/material-ui/blob/master/packages/material-ui-styles/src/withStyles/withStyles.js

Perhaps I'd better use JSS directly...

@cmeeren
Copy link
Contributor Author

cmeeren commented Jul 25, 2019

Any progress with v4? :)

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 10, 2019

Is there any way I can help out with v4? :)

@et1975
Copy link

et1975 commented Aug 12, 2019

Getting a warning:

Warning: Material-UI: theme.spacing.unit usage has been deprecated.
It will be removed in v5.
You can replace theme.spacing.unit * y with theme.spacing(y).
You can use the https://github.com/mui-org/material-ui/tree/master/packages/material-ui-codemod/README.md#theme-spacing-api migration helper to make the process smoother.

Re: ergonomics of styling, I think going via props is unnecessary complicated, here's how I construct styles:

let styles = 
    App.Theme.withTheme (fun theme ->
        {| textField = Style [ MarginLeft theme.spacing.unit
                               MarginRight theme.spacing.unit
                               Width 200 ]
           container = Style [ BackgroundColor theme.palette.grey.``200`` ]
        |})

where withTheme is

module Theme =
    open Fable.MaterialUI.Core
    let instance = createMuiTheme []

    let withTheme ofTheme =
        ofTheme instance

And using the styles is as simple as

let view (model:Model) dispatch =
    div [ styles.container ]
        [ form 
             []
             [ Mui.textField [ Label "Name"
                               Required true
                               DefaultValue model.name
                               M.Margin M.FormControlMargin.Normal
                               styles.textField
                               OnChange (fun e -> dispatch (NameChanged e.Value)) ] [] ]]

I'd love to keep something simple like this, but with classes generation.

@Luiz-Monad
Copy link

Luiz-Monad commented Aug 14, 2019

Yes, I over-complicated it too much by using Props.

Now I simplified and I'm using JSS styling directly without messing with Hocs and Props.
Example of the new way I made it. (port from Material-UI-pickers I'm working)

type S = Fable.React.Props.CSSProp
let yearStyles = useStyles "MuiPickersYear" <| fun ( theme: ITheme ) ->
    {|
        Root = style [
            S.Height 40
            S.Display DisplayOptions.Flex
            S.AlignItems AlignItemsOptions.Center
            S.JustifyContent "center"
            S.Cursor "pointer"
            S.Outline "none"
            cascade "&:focus" [
              S.Color theme.palette.primary.main
              S.FontWeight theme.typography.fontWeightMedium
            ]
        ]
        YearSelected = style [
            S.Margin "10px 0"
            S.FontWeight theme.typography.fontWeightMedium
        ]
        YearDisabled = style [
            S.PointerEvents "none"
            S.Color theme.palette.text.hint
        ]
    |}

let year () = ofHighOrderWithKey "Year" <| fun props children ->
    let
        (
            onSelect,
            forwardedRef,
            value,
            selected,
            disabled        
        ) = props

    let handleClick = 
        Hooks.useMemo ( fun () -> fun () -> 
            onSelect value
        , [| onSelect; value |] )

    typography [
        yield HTMLAttr.Role "button"
        yield MaterialProp.Component nDiv
        yield HTMLAttr.TabIndex ( disabled ?- -1 <| 0 )
        yield DOMAttr.OnClick ( ignore >> handleClick )
        yield DOMAttr.OnKeyPress ( ignore >> handleClick )
        yield MaterialProp.Color ( selected ?- ComponentColor.Primary <| !!jsUndefined )
        yield TypographyProp.Variant ( selected ?- TV.H5 <| TV.Subtitle1 )
        yield RefValue forwardedRef
        yield MaterialProp.Classes [
            ClassNames.Paper <| classListName [
                styleClassName yearStyles.Root, true
                styleClassName yearStyles.YearSelected, selected
                styleClassName yearStyles.YearDisabled, disabled
            ]
        ]
    ] children
And the library code to make it work.

type CssStyle = {
    Props: CSSProp list
    ClassName: string
}

/// Declares a style class.
let style cssProps =
    Some { Props = cssProps; ClassName = "" }

/// Declares a style class.
let styleExtend baseStyle cssProps =
    match baseStyle with
    | Some s -> Some { Props = s.Props @ cssProps; ClassName = "" }
    | _ -> style cssProps

/// Runtime generated style class name. (for class nesting)
let styleClassName style =
    match style with
    | Some s -> s.ClassName
    | _ -> ""

/// Runtime generated style class name.
let styleClass style : IHTMLProp list =
    match style with
    | Some s -> [ HTMLAttr.ClassName s.ClassName ]
    | _ -> []

/// Multiple classes.
let styleClasses styles : IHTMLProp list =
    let className style =
        match style with
        | Some s -> s.ClassName
        | _ -> ""
    let names = styles |> List.map className |> String.concat " "
    [ HTMLAttr.ClassName names ]

/// Class nesting helper.
let classListName classes =
    classes
    |> classList
    |> function HTMLAttr.ClassName n -> n | _ -> ""

/// Cascading styles.
let cascade selector styles =
    CSSProp.Custom ( selector, styles
        |> keyValueList CaseRules.LowerFirst )

/// Cascading styles.
let inline inheritCascade selector =
    JS.Object.keys selector
    |> Seq.map ( fun k ->
        CSSProp.Custom ( k, selector?(k) )
    ) |> List.ofSeq

// Adapter for Material UI JSS compatibility.

// Material UI makeStyles
let private makeStyles'<'S, 'O, 'P> ( styles: 'S ) ( options: 'O )
    : 'P -> IClassesProps =
    !!((import "makeStyles" "@material-ui/core/styles") $ (styles, options))

// Material UI makeStyles
let private makeStyles ( styles : StyleType ) ( options: StyleOption seq ) =
    let opt = keyValueList CaseRules.LowerFirst options
    let styles' =
        match styles with
        | StyleType.Styles styles -> (keyValueList CaseRules.LowerFirst styles |> unbox)
        | StyleType.Func func -> func >> keyValueList CaseRules.LowerFirst
    makeStyles'<_, _, unit> styles' opt

// Material UI useTheme
let useTheme<'T> () : 'T =
    !!((import "useTheme" "@material-ui/core/styles") $ ())

/// Convert our CssStyle to IStyles list
let private createSheet ( styleSheet: ITheme -> 'StyleSheet ) =
    fun theme ->
        let css = styleSheet theme
        let idx k = ( k, css?(k) )
        let conv ( k, v: CssStyle ) = Styles.Custom ( k, v.Props ) :> IStyles
        JS.Object.keys css |> Seq.map ( idx >> conv ) |> List.ofSeq

/// Convert the IClassesProps to our CssStyle
let private applySheet styleSheet ( classes: IClassesProps ) =
    let css: 'StyleSheet = styleSheet ( useTheme<ITheme> () )
    let convBack k v = Some { Props = v.Props; ClassName = k }
    let styleIt k = css?(k) <- convBack classes?(k) css?(k)
    JS.Object.keys css |> Seq.iter styleIt
    css
    
/// Create the style sheet directly.
let useStyles name styleSheet =
    let sheet = styleSheet |> createSheet |> StyleType.Func |> makeStyles
    let useStyles = sheet [ StyleOption.Name name ]
    useStyles () |> applySheet styleSheet

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 14, 2019

@mvsmal Something to consider going forward with v4: Many methods accepting U2, U3 etc. are more comfortable to use if they are overloads instead. For example, IBreakpoints.up can have overloads MaterialSize -> string and int -> string instead of U2<MaterialSize, int> -> string.

@Luiz-Monad
Copy link

This is a simple change in ts2fable, but I don't know if the overload resolution mecanism would like it and won't create problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants