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

Add support for `Arrangement.spacedBy` #156

Closed bitspittle closed 3 months ago

bitspittle commented 2 years ago

See also: https://developer.android.com/reference/kotlin/androidx/compose/foundation/layout/Arrangement#spacedBy(androidx.compose.ui.unit.Dp,androidx.compose.ui.Alignment.Horizontal)

How would this get done in an html world?

conradboehnke commented 2 years ago

I think it would work with columnGap and rowGap but I'm not a CSS or HTML expert so It's possible that I say false stuff... :D

bitspittle commented 1 year ago

This bug was originally about spacedBy but I'd like to support all of:

The first three should hopefully not be too bad, as that should just require adding some extra CSS rules for them. spacedBy may still be tricky because it's a function and not just an enum value I can add.

bitspittle commented 1 year ago

Supporting the three crossed off arrangement values was easy. There already was a direct CSS support for them.

spacedBy will require another pass later.


For more context, a call like this:

Row(horizontalArrangment = Arrangement.SpaceAround)

is simply converted into a class selector: .kobweb-row.kobweb-arrange-space-around

The relevant code in Row looks like this:

Div(modifier.asAttributesBuilder {
    classes("kobweb-row", horizontalArrangement.toClassName(), verticalAlignment.toClassName())
}

In contrast, spacedBy takes a dynamic value, so you can't just bake in a simple class name. Instead, I'd need to detect that spacedBy is a special type of Arrangment and then do something like this:

Div(modifier.asAttributesBuilder {
    when (horizontalArrangement) {
       is SpacedRowArrangement -> ... do something with rowGap, probably? ...
       else -> classes("kobweb-row", horizontalArrangement.toClassName(), verticalAlignment.toClassName())
    }
}

That should work, but the when statement seems a little messy, so I'd want to take a bit more time to think about it.

@conradboehnke I know it's been a long time, so if you don't respond, I won't mind. But would you still want spacedBy if you could have instead used, say, SpaceAround?

rafaeltonholo commented 4 months ago

Hi @bitspittle,

I would love to see the spacedBy feature added to the Arrangement. In my personal project, I have added support for it in one of my widgets by creating an ExtendedArrangement that adds support for spacedBy. I can contribute to this if it is still wanted.

bitspittle commented 4 months ago

Sure I would be more than happy to discuss adding spacedBy into Kobweb. At that point, you could just modify Arrangement directly and not created an ExtendedArrangement, correct?

rafaeltonholo commented 4 months ago

Yes, the idea is to get what I did into the ExtendedArrangement and migrate to the Arrangement, with some adjustments to the API to match with the Koweb APIs.

My initial idea would be to add a spacing value in the Arrangement with a default of -1 or null. Since we will use gap to create the space, and gap accepts values in the range of [0,∞], we could use that value to determine whether to add the class/style of the gap.

Does it sound good?

bitspittle commented 4 months ago

Yeah, it sounds great, thanks!

I should note that I'm not very familiar with spacedBy in Jetpack Compose so I may need to compare the code you write with the code in Jetpack Compose, but as long as you're ok with a discussion there, I think that's fine.

rafaeltonholo commented 4 months ago

Hi @bitspittle, I have opened a PR (https://github.com/varabyte/kobweb/pull/541) for this issue

bitspittle commented 3 months ago

This feature is now available in 0.18.1!