Skip to content

[Form] Adding callable to choices #21003

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[Form] Adding callable to choices #21003

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion reference/forms/types/choice.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ Field Options
choices
~~~~~~~

**type**: ``array`` **default**: ``[]``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callable is not allowed in ChoiceType option resolver

https://github.com/symfony/symfony/blob/db8b9da89978110c7b1b299e2218dbe6abcbef4e/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php#L380

$resolver->setAllowedTypes('choices', ['null', 'array', \Traversable::class]);

What is the return type of MyEnum::myChoices() in your comment ?

https://github.com/symfony/symfony/blob/db8b9da89978110c7b1b299e2218dbe6abcbef4e/src/Symfony/Component/Form/Extension/Core/Type/EnumType.php#L32C95-L32C100

 ->setDefault('choices', static fn (Options $options): array => $options['class']::cases())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the return type of MyEnum::myChoices() in your comment ?

An array of enum cases:

    public static function myChoices(): array
    {
        return [
            'foo' => self::One,
            'bar' => self::Two,
        ];
    }

I didn't look at the source code; just tried if it works. I'm getting only those 2 radiobuttons, but the array keys aren't used as label.
I just opened another issue about this yesterday: #21436

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand what you did here correctly, your configuration is relying on the built-in feature of the OptionsResolver component to define default options depending on another option: https://symfony.com/doc/current/components/options_resolver.html#default-values-that-depend-on-another-option

I don't think we should cover this by adding callable everywhere as the allowed type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I get what you mean.
What I want is to define the choices not directly in the form class (as @javiereguiluz shows it in fe70fe0), but rather define a function in the enum.
So this is not really supported, and works just cause I have luck?

Second: Looking at Javier's docs PR again: Am I right that adding array keys to be used as form labels will never work with enumType? If so, I think this should be added there - something like:

Contrary to choiceType, the array's keys will be ignored. If you want to set custom labels, use choice_label.

=> Is this right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's not possible, we should fix it in the code. Can you open a bug report please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, there you go :-) symfony/symfony#61927

But for the first question again: So is using a callable OK or not?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reading your PR description I am not sure you really do pass a callback to choices option.

->add('foo', EnumType::class, [
    'choices' => MyEnum::myChoices(),
])

Is this how you do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's how I do it.
And I now see what you mean :-) I was thinking that a function that returns an array counts as callable. But obviously callable is only meant for a "real" callback function that receives some arguments.
So if you agree, this PR can be closed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I submitted a PR that is supposed to fix the issue you reported. Please give it a try. :)

**type**: ``array``, ``callable`` **default**: ``[]``

This is the most basic way to specify the choices that should be used
by this field. The ``choices`` option is an array, where the array key
Expand Down