Skip to content

Commit

Permalink
bug #2426 [LiveComponent] Handle loose comparison with empty placehol…
Browse files Browse the repository at this point in the history
…der (Matthieu Renard)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[LiveComponent] Handle loose comparison with empty placeholder

Fix for a bug that arises when using an empty placeholder in a required select field, that is wrongly treated as an absence of placeholder by LiveComponents.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #2425  <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

LiveComponents test placeholders of required selects to detect when to set a default value from the choices (i.e. if the select is required and does not have a placeholder, among other conditions) by just casting the placeholder to a boolean. This leads to empty placeholders (placeholders that are the empty string) to be treated as an absence of placeholders when they should not. This MR fixes this by replacing the condition on placeholders with a more appropriate one.
According to the docs https://symfony.com/doc/current/reference/forms/types/choice.html#placeholder and https://symfony.com/doc/current/reference/forms/types/choice.html#field-variables, the placeholder can be a boolean (`false` indicates that there should not be a placeholder), a string (the text to display for the empty value) or a TranslatableMessage (same as string, but goes through a translator before being displayed). The vars property is either the value of the field, or `null` if there is no placeholder specified. From this, I considered that there is no placeholder either if the placeholder holds `false` or `null`.

I added a simple test that checks the behaviour when there is a required select with an empty placeholder ; we expect the value of the field to be empty in such cases when we do not modify it.

Commits
-------

3965302 [LiveComponent] Handle loose comparison with empty placeholder
  • Loading branch information
Kocal committed Dec 7, 2024
2 parents 49449f2 + 3965302 commit 6008eed
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/LiveComponent/src/ComponentWithFormTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ private function extractFormValues(FormView $formView): array
&& $child->vars['required']
&& !$child->vars['disabled']
&& !$child->vars['value']
&& !$child->vars['placeholder']
&& (false === $child->vars['placeholder'] || null === $child->vars['placeholder'])
&& !$child->vars['multiple']
&& !$child->vars['expanded']
&& $child->vars['choices']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ public function buildForm(FormBuilderInterface $builder, array $options)
],
'placeholder' => 'foo',
])
->add('choice_required_with_empty_placeholder', ChoiceType::class, [
'choices' => [
'bar' => 2,
'foo' => 1,
],
'placeholder' => '',
])
->add('choice_required_without_placeholder', ChoiceType::class, [
'choices' => [
'bar' => 2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ public function testHandleCheckboxChanges(): void
'range' => '',
'choice' => '',
'choice_required_with_placeholder' => '',
'choice_required_with_empty_placeholder' => '',
'choice_required_without_placeholder' => '2',
'choice_expanded' => '',
'choice_multiple' => ['2'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public function testFormValues(): void
'range' => '',
'choice' => '',
'choice_required_with_placeholder' => '',
'choice_required_with_empty_placeholder' => '',
'choice_required_without_placeholder' => '2',
'choice_expanded' => '',
'choice_multiple' => ['2'],
Expand Down

0 comments on commit 6008eed

Please sign in to comment.