BoxShadow Modifier API should support multiple box shadows

[bitspittle]

Right now, it only supports a single one while CSS lets you create multiple shadows for a single element.

[rafaeltonholo]

Hi @bitspittle, I needed this on my project, so I believe I could contribute to this issue.

Should I assign the issue to myself and open a PR with the changes?

[bitspittle]

Hey Rafael, sure that would be great! No pressure. You can also ask questions here -- it's been a while since I filed this so can't really remember if there would be anything tricky about having it accept multiple arguments or not. You can always try this and, if it ends up being harder to do than you expected, you're welcome to bail.

For reference code, probably the transition CSS property and the background CSS property could be good code to look at (although note that background is kind of complicated, hopefully box shadows won't be that bad)

Also FYI, normally I'm in a pretty responsive state, but this is a weird time. We've got a huge change landing in a couple of days (hopefully) and I'm scheduled to give a talk on June 7 at Droidcon, which at the moment is still just an outline...

So please expect this to be a relatively flaky time, and my apologies in advance if it actually is.

[rafaeltonholo]

Hey Rafael, sure that would be great! No pressure. You can also ask questions here -- it's been a while since I filed this so can't really remember if there would be anything tricky about having it accept multiple arguments or not. You can always try this and, if it ends up being harder to do than you expected, you're welcome to bail.

For reference code, probably the transition CSS property and the background CSS property could be good code to look at (although note that background is kind of complicated, hopefully box shadows won't be that bad)

Also FYI, normally I'm in a pretty responsive state, but this is a weird time. We've got a huge change landing in a couple of days (hopefully) and I'm scheduled to give a talk on June 7 at Droidcon, which at the moment is still just an outline...

So please expect this to be a relatively flaky time, and my apologies in advance if it actually is.

Thanks for the input. I already have a solution in place that I could share here, but I'll take a look into the two APIs you provide to understand how it is usually done in the project and adapt it to my solution.

This is how I'm doing in my internal project right now:

interface BoxShadow {
    var offsetX: CSSLengthNumericValue
    var offsetY: CSSLengthNumericValue
    var blurRadius: CSSLengthNumericValue?
    var spreadRadius: CSSLengthNumericValue?
    var color: CSSColorValue?
    var inset: Boolean
}

class BoxShadowScope {
    internal val shadows = mutableListOf<BoxShadow>()

    @BoxShadowDsl
    fun shadow(builder: BoxShadow.() -> Unit) {
        shadows += BoxShadowImpl().apply(builder)
    }
}

fun Modifier.boxShadow(block: BoxShadowScope.() -> Unit): Modifier = styleModifier {
    val scope = BoxShadowScope().apply(block)
    boxShadow(value = scope.shadows.toBoxShadowString())
}

The toBoxShadowString method does the same as you are doing in the StyleScope.boxShadow, but for a List. And this is how I'm using it right now:

Modifier.boxShadow {
    shadow {
        offsetX = 0.px
        offsetY = 1.px
        blurRadius = 3.px
        spreadRadius = 1.px
        color = Colors.Black.copyf(alpha = 0.15f)
    }
    shadow {
        offsetX = 0.px
        offsetY = 1.px
        blurRadius = 2.px
        spreadRadius = 0.px
        color = Colors.Black.copyf(alpha = 0.3f)
    }
}

and the CSS generated looks like this:

box-shadow: rgba(0, 0, 0, 0.298) 0px 4px 4px 0px, rgba(0, 0, 0, 0.15) 0px 8px 12px 6px;

However, I believe that is quite different from how it is done in the project. Let me check it first and I come back with a PR.

[bitspittle]

Ah I didn't realize I was talking to someone who is already working on a Kobweb project and already had something working locally. Nice!

While I am a huge fan of DSL builders in general, I'm not sure the shadow blocks here are intuitive, since there's really nothing else like them in the CSS property it is derived from. I believe here we just want a vararg parameter. But I will admit I haven't looked at this code for a while or thought about it too carefully. Thanks for taking a look at the other CSS properties in the meantime, in case they help.

We can continue to discuss things here or you can upload a PR and we can chat there, and apologies in advance if I'm slower to review it than normal with everything else going on.

(Optional: If you're curious to see why we've been busier than normal, you can take a look at https://github.com/varabyte/kobweb/blob/main/docs/css-style.md; this has been in discussion for over eight months and has finally almost made its way almost to release..... coming in 0.18.0)

[DennisTsar]

I agree we want a vararg parameter here. This is consistent with what we do for transition and background, and also ensures the required values (offsetX and offsetY) have to be provided. We also generally use DSLs/*Scope classes for configuring the sub-properties of a shorthand property (see BorderScope as an example), so using it here could cause confusion since there are no sub-properties being configured.

[rafaeltonholo]

Thanks for all the inputs, I have opened the PR https://github.com/varabyte/kobweb/pull/536 for this issue :)