[TwigComponent] CVA - boolean variants not working

[gremo]

Sadly, the boolean support for variants doesn't work. It's supposed two work.

The problem is true passed as string to the Twig. Is there a way to solve this issue

{% props theme = "primary", size = null, text = null, disabled = false %}

<button
    class="{{ cva({
        base: "btn",
        variants: {
            theme: {
                primary: "btn-primary",
                secondary: "btn-secondary",
                ghost: "btn-ghost",
                link: "btn-link",
            },
            size: {
                xs: "btn-xs",
                sm: "btn-sm",
                md: "btn-md",
                lg: "btn-lg",
            },
            disabled: {
                true: "btn-disabled",
            },
        },
        compoundVariants: [],
    }).apply({ theme, size }, attributes.render("class")|default("")) }}"
    {{ attributes.without("class") }}
>
    {% if block("content") is defined %}
        {{ block("content") }}
    {% elseif text|length %}
        {{ text }}
    {% endif %}
</button>

[smnandre]

Some others think it's not "supposed to work": https://cva.style/docs/examples/other-use-cases

For now the option taken has been: "we only deal with strings".

Poke @WebMamba who is the expert there :)

[gremo]

@smnandre Oh... I see. I just found that example searching for "CVA boolean variant". I think it's a nice addition because a lot of utility first framework have some "boolean" classes like btn-block, btn-wide, btn-outline.

Right now I'm just combining cva with html_classes but it seems so odd to me (daisyui example here):

{# templates/components/Button.html.twig #}
{% props
    as = "button",
    theme = "primary",
    outline = false,
    shape = null,
    size = null,
    wide = false,
    block = false,
    active = false,
    text = null
%}

<{{ as }}
    class="{{
        cva({
            base: "btn",
            variants: {
                theme: {
                    primary: "btn-primary",
                    secondary: "btn-secondary",
                    ghost: "btn-ghost",
                    link: "btn-link",
                },
                size: {
                    xs: "btn-xs",
                    sm: "btn-sm",
                    md: "btn-md",
                    lg: "btn-lg",
                },
                shape: {
                    square: "btn-square",
                    circle: "btn-circle",
                },
            },
            compoundVariants: [],
        }).apply(
            { theme, size, shape },
            html_classes({
                "btn-outline": outline,
                "btn-wide": wide,
                "btn-block": block,
                "btn-active": active,
            }),
            attributes.render("class") ?? ""
        )
    }}"
    {% if "a" == as %}
        role="{{ attributes.render("role")|default("button") }}"
    {% endif %}
    {{ attributes }}
>
    {% if block("content") is defined %}
        {{ block("content") }}
    {% elseif text|length %}
        {{ text }}
    {% endif %}
</{{ as }}>

[smnandre]

(side note: you may want to use {{ attributes.without('class') }} or you risk to render the class attribute twice)

For now the choice has been made not to transform use inputs and deal with boolean, but nothing can't change in the future :)

Just by curiosity, and taking your example as base, what would it look like if boolean were casted ?

[gremo]

@smnandre yes, I was using without but I then switched to render followed by attributes, I know that order matters here 👍

What would it look like if boolean were casted?

If I understand correcly, the variants would look like this (just one of:

{
  base: "btn",
  variants: {
    outline: { true: "btn-outline" },
    active: { true: "btn-active" }
  }
}

[WebMamba]

I am not sure about this feature, because it's a bit like transforming a key (so string) as a boolean. And I don't know I feel like it's less readable. But you are the second one to ask for it, and I promise to stay as close as possible to the JS version of it. I am not sure yet what to do...

[gremo]

@WebMamba It was just an idea/question of course. I'm still not sure if it's an "official" feature of just a side effect. If you want to stick with the original js version, feel free to close this issue! Thanks!

[kbond]

For the record, I also had a need for it the other day. Using true as a string is awkward.

[smnandre]

In "Stitches" (one of the CVA inspirations), there is indeed a real significance to the Boolean true values:

const Button = styled('button', {
  // base styles

  variants: {
    outlined: {
      true: {
        borderColor: 'lightgray',
      },
    },
  },
});

() => <Button outlined>Button</Button>;

https://stitches.dev/docs/variants#boolean-variants

That is something i'd feel a more comfortable with: either you have a hashmap with strings => values, either you have just "true" => values.

That would suppress any risk to mix booleans, strings (numbers?) ... in a chaotic/non-deterministic way.

[kbond]

So we'd support true|string? What would happen with false? Ignored?

[smnandre]

As i see it, the safest method for now / the future would be to allow array<string, mixed>|array<boolean, mixed>

So you could either do as before, or pass an array with only booleans as key.

And in this (nonsensical) case.. we either raise an exception, or convert all in strings.

{
  variants: {
    outline: { 
        true: "btn-foo"
        false: "btn-bar",
        foobar: "btn-foobar",
    }
  }
}

This has no real reason to happen ....

so i'd prefer to be strict now than having later to deal with BC break :|