Bjorn-Strom / FSS

MIT License
101 stars 4 forks source link

Add a Feliz specific version? #3

Closed Zaid-Ajaj closed 3 years ago

Zaid-Ajaj commented 3 years ago

Hi @Bjorn-Strom, this library looks great! I've wanted to build emotion binding for a while and here it is 😄 the only problem for me if I want to use in Feliz is that is uses a different styling API than the one that already exists in Feliz. It would be great if there was a version of this library that uses the existing styling API plus the extra element states such as hover, focus etc. Do you have something like this on the roadmap?

Bjorn-Strom commented 3 years ago

Hey @Zaid-Ajaj. Thank you for reaching out and sorry for the late reply! I was indeed hoping to do something Feliz specific. I havent had the time to really try it out, so I am not entirely sure of the idiomatic way to style using Feliz. It should work directly with classnames already. Do you have an example of how you would like to use Fss with Feliz?

Bjorn-Strom commented 3 years ago

I was thinking something like this:

Html.div [
    prop.fss [
        BackgroundColor.red
        Hover [ BackgroundColor.blue ]
    ]
]

However the following does work

 Html.h1 [
    prop.className (fss [ 
        BackgroundColor.red
        Hover [
            BackgroundColor.blue
        ]])
    prop.children [
        Html.text "Hello there"
    ]
]
Zaid-Ajaj commented 3 years ago

Hi @Bjorn-Strom I was thinking more of using the styling API that exits in Feliz instead like this:`

Html.div [
    prop.fss [
        style.backgroundColor.red
        style.hover [
            style.backgroundColor.red 
        ]
    ]
]

prop.fss and style.hover comes from the extension but things like style.backgroundColor.red are from the Feliz style API

Zaid-Ajaj commented 3 years ago

Actually, my personal preference would be that the property is called prop.css instead of prop.fss so the final snippet would be

Html.div [
    prop.css [
        style.backgroundColor.green
        style.hover [
            style.backgroundColor.red
            style.color.white
        ]
    ]
]

What do you think?

Bjorn-Strom commented 3 years ago

I think that looks nice! I will look into it :blush:

Zaid-Ajaj commented 3 years ago

Awesome! thanks a lot 🙏

Bjorn-Strom commented 3 years ago

While Feliz interoppability is a big priority for Fss I have decided that I do not wish to support Feliz specific syntax.

I feel that the normal Fss syntax is nice as is and piping it into className is and should be the idomatic way to write Fss.

Html.div [
    prop.className <| fss [
        BackgroundColor.red
        Hover [
            BackgroundColor.blue
        ]
    ]
]

I do appreciate the request though and I am open for further issues on this in the future 🙏

MangelMaxime commented 3 years ago

Hello,

I just randomly come across this issue and wanted to offer my opinion.

It should be possible to avoid the backward pipe by having 1-2 specifics Feliz operators:

Html.div [
    prop.fss [
        BackgroundColor.red
        Hover [
            BackgroundColor.blue
        ]
    ]
]

// I didn't check but I think in Feliz you can't do
// prop.className "some class"
// prop.fss [ .. ] 
// Because they would both set the className and only the last one is taken into account at least in pure React it is
// That's why we could need this helper too:

Html.div [
    prop.fssWithClass "some class" [
        BackgroundColor.red
        Hover [
            BackgroundColor.blue
        ]
    ]
]

I am mostly mentioning these helpers in case stumble into this issue and want save a characters and avoid the backward pipe.

Bjorn-Strom commented 3 years ago

Hello, @MangelMaxime 👋

I appreciate the feedback. You are right you cannot do

prop.className "some class"
prop.fss [ .. ] 

As Fss does not provide any Feliz specific functionality.

You mention an interesting case here, but I believe Fss has a built in solution for this, namely the combine function. Combine: string list -> (string * bool) list -> string takes a list of strings, which are the classnames to combine, and a list of strings to optionally combine based on the boolean value. That means your code example can be written like this:

    Html.div [
        prop.className (combine ["some class"
                                 fss [ BackgroundColor.red
                                       Hover [ BackgroundColor.blue ]]] [])
    ]

This will work regardless of where your classname string comes from, as Fss itself produces strings.

However this won't save you parens or a backward pipe, could you elaborate on why you would want that here?

MangelMaxime commented 3 years ago

However this won't save you parens or a backward pipe, could you elaborate on why you would want that here?

Sure, the main reason is to keep the DSL/View code clean/ less noisy.

It is all subjective but after working for so long with Elmish and helping design DSL like Fulma, Feliz.Bulma, etc. I noticed that the view code can quickly become messy and hard to read.

There are no major issues here, it was mostly for if someone find this issue and want to simplify a bit the noise in their views this could be a solution for them.

Bjorn-Strom commented 3 years ago

I see.

I have been playing around with a Feliz-specific syntax that I thought could be living in a separate nuget package Fss-lib-Feliz or something. I just haven't been able to make it work smoothly. When I continue to play around with it I can also try to add smoother functionality for combining existing classnames with Fss as well, like you suggest 👍

If I am still unable to make it work I can at least make an extra helper function to make combining classnames simpler overall 😄