Skip to content

Commit

Permalink
bug #2491 [LiveComponent] Fix `ComponentWithFormTrait::extractFormVal…
Browse files Browse the repository at this point in the history
…ues()` with edge cases (smnandre)

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

Discussion
----------

[LiveComponent] Fix `ComponentWithFormTrait::extractFormValues()` with edge cases

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #2487
| License       | MIT

#2403 fixed an old bug to simulate browser behaviour when a select field has no option selected,  no placeholder, and is required (it then uses the first option)
#2425 and #2426 fixed the way we handled empty strings as placeholders

In #2487 `@maciazek` explained us how a custom type with a "choices" option became unusable since the last changes.

Also.. while I was reading all this I realized we returned wrong values here when "option groups" were used.. leading to other problems.

I updated the code of `extractFormValues()` method to:
* check if most of the caracteristic keys added in `ChoiceType::buildView` were defined in the `$view->vars`, to ensure we are dealing with a `<select>` field built by a `ChoiceType`
* fetch recursively the value of the first displayed option (even with groups)

Commits
-------

b7f1ba4 [LiveComponent] Fix `ComponentWithFormTrait::extractFormValues()` with edge cases
  • Loading branch information
smnandre committed Jan 25, 2025
2 parents 04f40fd + b7f1ba4 commit 90a2eb1
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 13 deletions.
41 changes: 28 additions & 13 deletions src/LiveComponent/src/ComponentWithFormTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\UX\LiveComponent;

use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
use Symfony\Component\Form\ClearableErrorsInterface;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
Expand Down Expand Up @@ -255,25 +256,39 @@ private function extractFormValues(FormView $formView): array
continue;
}

// <input type="checkbox">
if (\array_key_exists('checked', $child->vars)) {
// special handling for check boxes
$values[$name] = $child->vars['checked'] ? $child->vars['value'] : null;
continue;
}

if (\array_key_exists('choices', $child->vars)
&& $child->vars['required']
&& !$child->vars['disabled']
&& !$child->vars['value']
&& (false === $child->vars['placeholder'] || null === $child->vars['placeholder'])
&& !$child->vars['multiple']
&& !$child->vars['expanded']
&& $child->vars['choices']
// <select> - Simulate browser behavior
// When no option is selected, browsers send the value of the first
// option, when the following conditions are met:
if (
\array_key_exists('choices', $child->vars)
&& $child->vars['choices'] // has defined choices
&& $child->vars['required'] // is required
&& !$child->vars['disabled'] // is not disabled
&& '' === $child->vars['value'] // has no value set ("0" can be a value)
&& !array_diff_key(
/* @see \Symfony\Component\Form\Extension\Core\Type\ChoiceType::buildView() */
array_flip(['choices', 'expanded', 'multiple', 'placeholder', 'placeholder_in_choices', 'preferred_choices']),
$child->vars,
)
&& !$child->vars['expanded'] // is a <select> (not a radio/checkbox)
&& !$child->vars['multiple'] // is not multiple
&& !\is_string($child->vars['placeholder']) // has no placeholder (empty string is valid)
) {
if (null !== $firstKey = array_key_first($child->vars['choices'])) {
$values[$name] = $child->vars['choices'][$firstKey]->value ?? null;
}

$choices = $child->vars['choices'];
do {
$choice = $choices[array_key_first($choices)];
if (!$choice instanceof ChoiceGroupView) {
break;
}
} while ($choices = $choice->choices);

$values[$name] = $choice?->value;
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ public function buildForm(FormBuilderInterface $builder, array $options)
'foo' => 1,
],
])
->add('choice_required_without_placeholder_and_choice_group', ChoiceType::class, [
'choices' => [
'Bar Group' => [
'Bar Label' => 'ok',
'Foo Label' => 'foo_value',
],
'foo' => 1,
],
])
->add('choice_expanded', ChoiceType::class, [
'choices' => [
'foo' => 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ public function testHandleCheckboxChanges(): void
'choice_required_with_placeholder' => '',
'choice_required_with_empty_placeholder' => '',
'choice_required_without_placeholder' => '2',
'choice_required_without_placeholder_and_choice_group' => 'ok',
'choice_expanded' => '',
'choice_multiple' => ['2'],
'select_multiple' => ['2'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function testFormValues(): void
'choice_required_with_placeholder' => '',
'choice_required_with_empty_placeholder' => '',
'choice_required_without_placeholder' => '2',
'choice_required_without_placeholder_and_choice_group' => 'ok',
'choice_expanded' => '',
'choice_multiple' => ['2'],
'select_multiple' => ['2'],
Expand Down

0 comments on commit 90a2eb1

Please sign in to comment.