ux
ux copied to clipboard
[Twig] boolean (true) CVA variants
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| Issues | Fix #1614 |
| License | MIT |
This is an attempt to solve #1614. As discussed in that issue, it seems like only a true should be valid. Because of this, I opted for the variant value(s) to be a string or list<string> - this is used when the recipe value is true. Examples:
{% props size = 'md', disabled = false %}
{% set button = cva({
variants: {
size: {
sm: 'alert-sm',
md: 'alert-md',
lg: 'alert-lg',
},
disabled: 'opacity-50 cursor-not-allowed',
disabled: ['opacity-50', 'cursor-not-allowed'], {# also valid #}
},
}) %}
<button class="{{ button.apply({size, disabled}, attributes.render('class'), 'flex p-4') }}">
...
</button>
TODO
- [ ] docs
- [ ] changelog
When disabled is false, how could we associate CSS classes with it ?
To me it should only accept an array with true and false as keys
When disabled is false, how could we associate CSS classes with it ?
Yeah, you can't.
I sort of followed this (I don't know for sure that false is not supported) but simplified the implementation.
To me it should only accept an array with true and false as keys
I went down this road initially but, since a real bool cannot be an array key, I'd have to fudge 0|'false' and 1|'true' to represent false and true respectively. This felt a bit brittle to me.
So, I think it would be interesting to have something like this (to make it clear 😄):
{% props size = 'md', disabled = false %}
{% set button = cva({
variants: {
size: {
sm: 'alert-sm',
md: 'alert-md',
lg: 'alert-lg',
},
disabled: {
true: 'opacity-50 cursor-not-allowed',
false: 'bg-blue-600 text-orange-400',
}
},
}) %}
<button class="{{ button.apply({size, disabled}, attributes.render('class'), 'flex p-4') }}">
...
</button>
So we're back to the suggestion: either all your "keys" are boolean, either all are strings ?
So we're back to the suggestion: either all your "keys" are boolean, either all are strings ?
But keys can't be real booleans so we have to say "true" === true, "false" === false and/or 1 === true, 0 === false. Something like array<string,string|string[]>|array<"true"|"false",string|string[]>
Do as you think it's best, and you'll have my full support :))
https://cva.style/docs/examples/other-use-cases
cva is really just a fancy way of managing a string…
It would/should treat boolean as a string, even false.
I think it will be great if these configs work:
<twig:Alert /> {# Default value #}
<twig:Alert disabled /> {# true #}
<twig:Alert disabled="" /> {# true #}
<twig:Alert disabled="disabled" /> {# true #}
<twig:Alert disabled="anyStringValue" /> {# true #}
<twig:Alert disabled="true" /> {# true #}
<twig:Alert disabled="false" /> {# false #}
<twig:Alert disabled="{{ true }}" /> {# true #}
<twig:Alert disabled="{{ false }}" /> {# false #}
{% set trueBoolean = true %}
<twig:Alert disabled="{{ trueBoolean }}" /> {# true #}
{% set trueBoolean = true %}
<twig:Alert :disabled="trueBoolean" /> {# true #}
We achieve that by normalizing the option to a string, and anything but false or 0 is true.
#[AsTwigComponent]
final class Alert
{
public string $disabled = 'false';
#[PreMount]
public function preMount(array $data): array
{
$resolver = new OptionsResolver();
$resolver->setDefaults(
[
'disabled' => 'false',
]
);
$resolver->setAllowedTypes('disabled', ['bool', 'string']);
$resolver->setNormalizer('disabled', function(Options $options, string|bool $value) {
if (is_bool($value)) {
$strBool = $value ? 'true' : 'false';
} else {
$strBool = $value;
}
return preg_match('/^(false|0)$/i', strtolower($strBool)) === 1 ? 'false' : 'true';
});
return $resolver->resolve($data);
}
}
How is this related to CVA ?
Because variants cannot be true only, but 'true' AND 'false'.
cva({
variants: {
disabled: 'opacity-50 cursor-not-allowed',
},
})
VS
cva({
variants: {
disabled: {
true: 'opacity-50 cursor-not-allowed',
false: 'bg-blue-600 text-orange-400',
}
},
})
As the doc mentioned above: https://cva.style/docs/examples/other-use-cases
So today, we have to cast boolean value/key to full letter "string boolean".
The moment #1653 is merged, i'll rework this code from @kbond (yeah i changed to original code ... so i'll assume this and won't ask him to adapt this PR haha) and we'll ship an easier DX, as multiple people ask it :)
I agree with @cavasinf in that all keys should be strings/"string booleans". I'll rework this now that #1653 is merged and we can do another round of discussion.
I'm in favor of the alternate implementation in #1710. WDYT @seb-jean, @Kocal, @cavasinf?
I'm also in favor of #1710, which keep the usage and maintenance very simple! :)
I agree with #1710 as well. 👍