Skip to content
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.

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

Fix ChoiceType terminology #6144

Closed
webmozart opened this issue Jan 14, 2016 · 1 comment
Closed

Fix ChoiceType terminology #6144

webmozart opened this issue Jan 14, 2016 · 1 comment
Labels
actionable Clear and specific issues ready for anyone to take them. DX ⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming Form hasPR A Pull Request has already been submitted for this issue.

Comments

@webmozart
Copy link
Contributor

Currently, the ChoiceType documentation is IMO not clearly worded. I know that readability is important, but since this type is quite flexible and complex, being precise is even more important. All code examples and descriptions should be updated to use the following terms:

  • choice: The model value of the choices array. E.g.: true, false, object (Category), ...
  • key: The key of the choices array.
  • value: The HTML value. The value is always a string. E.g.: '0', '1', 'yes', 'no', ... Values must be free of duplicates, since they are used to identify the corresponding choice when submitting the form.

This naming is important since those values are passed from one callable to another:

  1. The choice_value callable with signature function ($choice) receives the choice and returns the string value.
  2. The choice_label callable with signature function ($choice, $key, $value) receives the choice, the key and the value and returns a string label. The default value is function ($choice, $key, $value) => $key, i.e. the key of the choices array is used as label by default.
  3. The choice_name callable has the same signature as choice_label and outputs the form name used for checkboxes/radio buttons.
  4. The choice_attr callable has the same signature as choice_label and outputs the HTML attributes.

Furthermore, the choice_loader is not completely accurate:

The choice_loader can be used to only partially load the choices in cases where a fully-loaded list is not necessary. This is only needed in advanced cases and would replace the choices option.

I would rephrase this. The choice loader is used to load choices from a data source that can be queried with a query language, such as a database or a search engine. When the choice field is displayed, the full list is loaded, but when it is submitted, only the submitted value is looked up. A second benefit is that different fields that use the same ChoiceLoader instance use the same cached choice list, reducing N queries to 1.

Before/After code examples:

Before:

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
// ...

$builder->add('attending', ChoiceType::class, array(
    'choices' => array(
        'yes' => true,
        'no' => false,
        'maybe' => null,
    ),
    'choices_as_values' => true,
    'choice_label' => function ($value, $key, $index) {
        if ($value == true) {
            return 'Definitely!';
        }
        return strtoupper($key);

        // or if you want to translate some key
        //return 'form.choice.'.$key;
    },
));

After:

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
// ...

$builder->add('attending', ChoiceType::class, array(
    'choices' => array(
        'yes' => true,
        'no' => false,
        'maybe' => null,
    ),
    'choices_as_values' => true,
    'choice_label' => function ($choice, $key, $value) {
        return true === $choice ? 'Definitely!' : strtoupper($key);

        // or if you want to translate some key
        //return 'form.choice.'.$key;
    },
));

Before:

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
// ...

$builder->add('attending', ChoiceType::class, array(
    'choices' => array(
        'Yes' => true,
        'No' => false,
        'Maybe' => null,
    ),
    'choices_as_values' => true,
    'choice_attr' => function($val, $key, $index) {
        // adds a class like attending_yes, attending_no, etc
        return ['class' => 'attending_'.strtolower($key)];
    },
));

After:

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
// ...

$builder->add('attending', ChoiceType::class, array(
    'choices' => array(
        'Yes' => true,
        'No' => false,
        'Maybe' => null,
    ),
    'choices_as_values' => true,
    'choice_attr' => function($choice, $key, $value) {
        // adds a class like attending_yes, attending_no, etc
        return ['class' => 'attending_'.strtolower($key)];
    },
));

I also recommend to add type hints in the advanced example:

Before:

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
use AppBundle\Entity\Category;
// ...

$builder->add('category', ChoiceType::class, [
    'choices' => [
        new Category('Cat1'),
        new Category('Cat2'),
        new Category('Cat3'),
        new Category('Cat4'),
    ],
    'choices_as_values' => true,
    'choice_label' => function($category, $key, $index) {
        /** @var Category $category */
        return strtoupper($category->getName());
    },
    'choice_attr' => function($category, $key, $index) {
        return ['class' => 'category_'.strtolower($category->getName())];
    },
    'group_by' => function($category, $key, $index) {
        // randomly assign things into 2 groups
        return rand(0, 1) == 1 ? 'Group A' : 'Group B'
    },
    'preferred_choices' => function($category, $key, $index) {
        return $category->getName() == 'Cat2' || $category->getName() == 'Cat3';
    },
]);

After:

use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
use AppBundle\Entity\Category;
// ...

$builder->add('category', ChoiceType::class, [
    'choices' => [
        new Category('Cat1'),
        new Category('Cat2'),
        new Category('Cat3'),
        new Category('Cat4'),
    ],
    'choices_as_values' => true,
    'choice_label' => function(Category $category, $key, $value) {
        return strtoupper($category->getName());
    },
    'choice_attr' => function(Category $category, $key, $value) {
        return ['class' => 'category_'.strtolower($category->getName())];
    },
    'group_by' => function(Category $category, $key, $value) {
        // randomly assign things into 2 groups
        return rand(0, 1) == 1 ? 'Group A' : 'Group B'
    },
    'preferred_choices' => function(Category $category, $key, $index) {
        return $category->getName() == 'Cat2' || $category->getName() == 'Cat3';
    },
]);
@syther101
Copy link

Can I add to this and suggest that the terminology for the choice-attr option is worded in a slightly confusing manner also.

When setting choice-attr the docs suggest that you can:

Use this to add additional HTML attributes to each choice. This can be an array of attributes (if they are the same for each choice).

But to then also suggest that:

If an array, the keys of the choices array must be used as keys.

https://symfony.com/doc/current/reference/forms/types/choice.html#choice-attr

But isn’t that a bit of a contradiction. As by setting the attributes per key aren’t you therefore not setting the attributes the same for each choice?

Got caught up a little by this today. One would think that by providing an array it would apply to each choice no?

'choice_attr' => ['class' => 'choiceClass']

But in-fact the second point is correct and the array has to map to the individual choices:

'choice_attr' => [
                    1 => [ 'class' => 'choiceClass']
                ]

Be interested on your thoughts on this @webmozart. I can try and submit a pull request if so. Bit of a novice when it comes to contributing to projects like this but I can give it a go 😃 .

fabpot added a commit to symfony/symfony that referenced this issue Mar 17, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Fixed some phpdocs

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ~
| License       | MIT
| Doc PR        | symfony/symfony-docs#6393

ref symfony/symfony-docs#6144, symfony/symfony-docs#6297, #14050

Commits
-------

b9162e8 [Form] Fixed some phpdocs
@xabbuh xabbuh closed this as completed in d92285f Apr 7, 2019
@wouterj wouterj added ⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming and removed ⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming labels Apr 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable Clear and specific issues ready for anyone to take them. DX ⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming Form hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Development

No branches or pull requests

4 participants