[Form] Fix EnumType choice_label logic for grouped choices #62237
Conversation
yoeunes
force-pushed
the
issue-62234
branch
2 times, most recently
from
ff5989f to
85ff6f4
Compare
src/Symfony/Component/Form/Tests/Extension/Core/Type/EnumTypeTest.php
Outdated
Show resolved
Hide resolved
| return null; | ||
| $choices = $options['choices']; | ||
|
|
||
| if (\is_array($choices) && !array_is_list($choices)) { |
There was a problem hiding this comment.
What happens if you have a mix of grouped and non-grouped choices, for example:
'choices' => [
'Group 1' => [
'Custom Yes' => Answer::Yes,
'Custom No' => Answer::No,
],
'Custom 42' => Answer::FortyTwo,
],Also, what if you have a mix of labeled and unlabeled choices:
'choices' => [
Answer::Yes,
Answer::No,
'Custom 42' => Answer::FortyTwo,
],There was a problem hiding this comment.
hi @HypeMC, thank you for your feedback!
I have improved the logic and added tests for grouped and custom-labeled Enum choices.
For hybrid cases (mixing grouped, non-grouped, labeled, and non-labeled choices), the expected behavior isn’t so clear.
Should we make it flexible and auto-detect labels, or do you prefer a strict approach (numeric indexes for non-labeled, labels only if set)?
What do you think is best for EnumType in Symfony?
There was a problem hiding this comment.
This approach makes sense to me. Let's see what others think.
|
Thank you @yoeunes. |
… (yoeunes) This PR was merged into the 6.4 branch. Discussion ---------- [Form] Fix EnumType choice_label logic for grouped choices | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #62234 | License | MIT This is #62237 for 6.4 Commits ------- 323b6db [Form] Fix EnumType choice_label logic for grouped choices
5 participants
This PR fixes a regression in the
EnumType's defaultchoice_labellogic.The previous logic checked
!array_is_list($options['choices'])to determine if custom labels were provided. If so, it wouldreturn nullto disable the default enum labeler (i.e.,$choice->name).However, this check incorrectly matched grouped choices (e.g.,
['Group 1' => [Answer::Yes]]), which are also associative arrays. This caused the default labels for enums within groups to break (they became empty).This fix adds a check to see if the associative array is actually for grouping (by detecting if any value
is_array) before returningnull.Tests are added to cover both: