varabyte / kobweb

A modern framework for full stack web apps in Kotlin, built upon Compose HTML
https://kobweb.varabyte.com
Apache License 2.0
1.53k stars 68 forks source link

Provide a receiver function in addition to the generator function in `ComponentStyle.base` #530

Closed XibalbaM closed 4 months ago

XibalbaM commented 4 months ago

Add a function taking Modifier.() -> Unit instead of () -> Modifier to ComponentStyle. Allowing a DSL-like syntax, it would allow the Style's code to be less verbose and easyer to write, to add conditions, etc. For exemple : Instead of this :

val Dummy by ComponentStyle.base {
    Modifier
        .background(Color.red)
        .then(if (someCheck) Modifier.color(Color.red) else Modifier)
        .padding(5.px)
        .margin { 
            left(10.px)
            right(3.px)
        }
}

We could have

val Dummy by ComponentStyle.base {
    background(Color.red)
    if (someCheck) color(Color.red)
    padding(5.px)
    margin {
        left(10.px)
        right(3.qpx)
    }
}

Witch is lot cleaner and easyer. It's more logic to do it like that and allows less verbose code. We can even imagine (not sure about that) that all functions acutally taking a modifier could also take this kind of function, allowing things like

Row({background(Color.red)}) {
    ...
}

And a static function Modifier(Modifier.() -> Unit) generating a modifier with this syntax, for funtions taking, for example, Attrs instead.

I know it's not a game changer, but it will make the coding experience more straigtforward and more pleasant.

bitspittle commented 4 months ago

We use Modifiers explicitly intentionally. Here are three reasons that come to mind, there may be others:

  1. The Modifier class is immutable, and it must be chained (this is a powerful, intentional design, and this is how it works in Jetpack Compose).

Immutability is a great feature because it lets you store modifiers safely in global variables. I discuss that a bit more here, particularly contrasting it with Compose HTML (which actually works more the way you are proposing in this issue): https://bitspittle.dev/blog/2022/kotlin-site#modifier

When you call a modifier function, it returns itself appended to a previous modifier chain.

val modifier = Modifier.a().b().c() // a + b + c

// vs.

var modifier = Modifier.a() // a
modifier = Modifier.b() // b
modifier = Modifier.c() // c
// Here, the intermediate calls to "a" and "b" are lost

So your proposal is actually not doing what you think it is doing.

val Dummy by ComponentStyle.base {
    background(Color.red) // not included in the final result
    if (someCheck) color(Color.red) // not included in the final result
    padding(5.px) // not included in the final result
    margin {
        left(10.px)
        right(3.qpx)
    }
   // ^ This block basically returns "style { margin-left: 10px; margin-right: 10px }" and nothing else
}
  1. Explicit modifier chains are consistent with the rest of your codebase which is better for refactorability.

For example, you can easily migrate an inline Modifier.blah(...) call into a style block as is:

// Before
Box(Modifier.backgroundColor(Colors.Red))

// After
val PayAttentionToMeStyle = ComponentStyle.base {
   Modifier.backgroundColor(Colors.Red)
}
Box(PayAttentionToMeStyle.toModifier())

or you could go back the other way if you needed to as well.

  1. This enables a pattern where you can declare a private modifier outside of the the style blocks and share it across more than one of them:
val commonTextModifier = Modifier.fontSize(16.px).fontFamily("...")

val NoteStyle = ComponentStyle.base { commonTextModifier }
val AsideStyle = ComponentStyle.base { commonTextModifier.then(Modifier.fontStyle(FontStyle.Italics))

With your syntax, if we could even get it to work using some tricks, you'd have to use a floating then which would look weird:

val NoteStyle = ComponentStyle.base { then(commonTextModifier) }
val AsideStyle = ComponentStyle.base {
   then(commonTextModifier)
   fontStyle(FontStyle.Italics)

Even if there was a clever way to support both, I don't think we should, because that will introduce an ambiguity issue. When you type Style.base { ... }, which code completion option should you choose?

I think people getting used to writing Modifier chains is fundamental to Kobweb (and Jetpack Compose overall) so best to be consistent with it.

And ultimately, the only reason to support the new syntax would be to save a single line of code.

Hope this helps understand the nuance of the decision!

XibalbaM commented 4 months ago

OK after thinking to it, I think I'll forget this suggestion, as you said it's a lot of work. Thanks for your patience!

XibalbaM commented 4 months ago

I just found StyleScope. Isn't it exactly what I want ? All Modifier method seems implemented, and it seems to be what I was talking bout. Can't we allow methods like CssStyle.base to take a StyleScope receiver and voilà, we can do:

CssStyle.base {
    background(ColorSchemes.Red._900)
    color(ColorSchemes.Red._50)
}

Don't hesitate to tell me if I misunderstood something, but it seems to be exactly what I wanted The only question is how do we make it accessible, because I guess the compiler won't be able to figure out what implementation to use if we do as I said, but it's not a big deal.

bitspittle commented 4 months ago

StyleScope is a Compose HTML concept and it is fine to use; however, Modifiers has some pretty big advantages, mainly they are immutable and chainable, which makes it safe to share them in variables and break them apart into separate pieces which can be great for refactoring.

It definitely does not make sense to support them in CssStyle because we can only choose one, and CssStyle is a Kobweb concept.

You can use Modifier.styleModifer { ... } if you want access to a StyleScope.

You can read a bit more about the decision to add modifiers here: https://bitspittle.dev/blog/2022/kotlin-site#modifier

bitspittle commented 4 months ago

(Closing this issue for process purposes but you can continue leaving comments here if you had more questions)