-
-
Notifications
You must be signed in to change notification settings - Fork 364
[Twig] boolean (true) CVA variants #1647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When |
To me it should only accept an array with |
Yeah, you can't. I sort of followed this (I don't know for sure that
I went down this road initially but, since a real |
if (isset($this->variants[$recipeName]) && (\is_string($this->variants[$recipeName]) || (\is_array($this->variants[$recipeName]) && array_is_list($this->variants[$recipeName])))) { | ||
if (true === $recipeValue) { | ||
$classes .= ' '.implode(' ', (array) $this->variants[$recipeName]); | ||
} | ||
|
||
continue; | ||
} | ||
|
||
if (!isset($this->variants[$recipeName][$recipeValue])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I fear this become hard to understand for the next ones :)
I'll try another way, you'll tell me if it's worth the change 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something i don't understand in the algorithm will ask you questions about this, but don"t wait for me in this PR, that's 100% implementation and can be handled way after :)
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 ? |
But keys can't be real booleans so we have to say |
Do as you think it's best, and you'll have my full support :)) |
https://cva.style/docs/examples/other-use-cases
It would/should treat boolean as a string, even 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 #[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 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". |
I'm also in favor of #1710, which keep the usage and maintenance very simple! :) |
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Twig] Resolving boolean as variant keys After some "back and forth", we agreed on the following behaviour. I hope this will match your needs :) This PR allows users to pass real boolean when calling the component, to match string-boolean defined in the CVA definition. Based on the work of `@kbond` in #1647 to fix #1614 ```twig {# templates/components/Button.html.twig #} {% props color = 'blue', giant = false %} {% set button = cva({ base: 'button', variants: { color: { blue: 'btn-blue', green: 'btn-red', }, giant: { true: 'btn-giant', } } }) %} <button class="{{ button.apply({color, giant}, attributes.render('class')) }}"> {{ label }} </button> ``` Then the 3 following calls should be equivalent ```twig {# String value #} <twig:Button color="blue" giant="true" label="Save" /> {# Boolean value #} <twig:Button color="blue" giant="{{ true }}" label="Save" /> {# Boolean expression #} <twig:Button color="blue" giant="{{ 2 > 1 }}" label="Save" /> ``` Commits ------- 12e0a4c [Twig] Resolving boolean as variant keys
I agree with #1710 as well. 👍 |
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 astring
orlist<string>
- this is used when the recipe value istrue
. Examples:TODO