Skip to content

Commit 4861d59

Browse files
committed
bug #2491 [LiveComponent] Fix ComponentWithFormTrait::extractFormValues() 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 ------- b7f1ba437 [LiveComponent] Fix `ComponentWithFormTrait::extractFormValues()` with edge cases
2 parents 807d755 + 255887d commit 4861d59

File tree

4 files changed

+39
-13
lines changed

4 files changed

+39
-13
lines changed

src/ComponentWithFormTrait.php

+28-13
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\UX\LiveComponent;
1313

14+
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
1415
use Symfony\Component\Form\ClearableErrorsInterface;
1516
use Symfony\Component\Form\FormInterface;
1617
use Symfony\Component\Form\FormView;
@@ -255,25 +256,39 @@ private function extractFormValues(FormView $formView): array
255256
continue;
256257
}
257258

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

264-
if (\array_key_exists('choices', $child->vars)
265-
&& $child->vars['required']
266-
&& !$child->vars['disabled']
267-
&& !$child->vars['value']
268-
&& (false === $child->vars['placeholder'] || null === $child->vars['placeholder'])
269-
&& !$child->vars['multiple']
270-
&& !$child->vars['expanded']
271-
&& $child->vars['choices']
265+
// <select> - Simulate browser behavior
266+
// When no option is selected, browsers send the value of the first
267+
// option, when the following conditions are met:
268+
if (
269+
\array_key_exists('choices', $child->vars)
270+
&& $child->vars['choices'] // has defined choices
271+
&& $child->vars['required'] // is required
272+
&& !$child->vars['disabled'] // is not disabled
273+
&& '' === $child->vars['value'] // has no value set ("0" can be a value)
274+
&& !array_diff_key(
275+
/* @see \Symfony\Component\Form\Extension\Core\Type\ChoiceType::buildView() */
276+
array_flip(['choices', 'expanded', 'multiple', 'placeholder', 'placeholder_in_choices', 'preferred_choices']),
277+
$child->vars,
278+
)
279+
&& !$child->vars['expanded'] // is a <select> (not a radio/checkbox)
280+
&& !$child->vars['multiple'] // is not multiple
281+
&& !\is_string($child->vars['placeholder']) // has no placeholder (empty string is valid)
272282
) {
273-
if (null !== $firstKey = array_key_first($child->vars['choices'])) {
274-
$values[$name] = $child->vars['choices'][$firstKey]->value ?? null;
275-
}
276-
283+
$choices = $child->vars['choices'];
284+
do {
285+
$choice = $choices[array_key_first($choices)];
286+
if (!$choice instanceof ChoiceGroupView) {
287+
break;
288+
}
289+
} while ($choices = $choice->choices);
290+
291+
$values[$name] = $choice?->value;
277292
continue;
278293
}
279294

tests/Fixtures/Form/FormWithManyDifferentFieldsType.php

+9
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ public function buildForm(FormBuilderInterface $builder, array $options)
6565
'foo' => 1,
6666
],
6767
])
68+
->add('choice_required_without_placeholder_and_choice_group', ChoiceType::class, [
69+
'choices' => [
70+
'Bar Group' => [
71+
'Bar Label' => 'ok',
72+
'Foo Label' => 'foo_value',
73+
],
74+
'foo' => 1,
75+
],
76+
])
6877
->add('choice_expanded', ChoiceType::class, [
6978
'choices' => [
7079
'foo' => 1,

tests/Functional/Form/ComponentWithFormTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ public function testHandleCheckboxChanges(): void
181181
'choice_required_with_placeholder' => '',
182182
'choice_required_with_empty_placeholder' => '',
183183
'choice_required_without_placeholder' => '2',
184+
'choice_required_without_placeholder_and_choice_group' => 'ok',
184185
'choice_expanded' => '',
185186
'choice_multiple' => ['2'],
186187
'select_multiple' => ['2'],

tests/Unit/Form/ComponentWithFormTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public function testFormValues(): void
4646
'choice_required_with_placeholder' => '',
4747
'choice_required_with_empty_placeholder' => '',
4848
'choice_required_without_placeholder' => '2',
49+
'choice_required_without_placeholder_and_choice_group' => 'ok',
4950
'choice_expanded' => '',
5051
'choice_multiple' => ['2'],
5152
'select_multiple' => ['2'],

0 commit comments

Comments
 (0)