[Form] choice_attr option in ChoiceType generates options attributes wrong #17019

a-sane · 2015-12-15T12:34:56Z

All code tested on clean symfony 2.8 installation

I've created TestFormType

class TestType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('testChoice', 'choice', [
                'choices' => [
                    1 => 'test 1',
                    2 => 'test 2',
                    3 => 'test 3',
                ],
                'choice_attr' => [
                    1 => ['data-test' => 'test 1'],
                    2 => ['data-test' => 'test 2'],
                    3 => ['data-test' => 'test 3'],
                ],
            ]);
    }
}

and i got this in view:

<select id="test_testChoice" name="test[testChoice]">
    <option value="1">test 1</option>
    <option value="2" data-test="test 1">test 2</option>
    <option value="3" data-test="test 2">test 3</option>
</select>

same with placeholder option:

class TestType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('testChoice', 'choice', [
                'choices' => [
                    1 => 'test 1',
                    2 => 'test 2',
                    3 => 'test 3',
                ],
                'placeholder' => 'test placeholder',
                'choice_attr' => [
                    1 => ['data-test' => 'test 1'],
                    2 => ['data-test' => 'test 2'],
                    3 => ['data-test' => 'test 3'],
                ],
            ]);
    }
}

<select id="test_testChoice" name="test[testChoice]" required="required">
    <option value="" selected="selected">test placeholder</option>
    <option value="1">test 1</option>
    <option value="2" data-test="test 1">test 2</option>
    <option value="3" data-test="test 2">test 3</option>
</select>

i'm not sure, but i think problem near this commit: d94c322
probably in normalizeLegacyChoices method

The text was updated successfully, but these errors were encountered:

stof · 2015-12-15T13:16:08Z

@nicolas-grekas looks like choice_attr is missing some normalization, similar to what is done for choice_label.

Btw, be careful that both choice_label and choice_attr can both accept callables and property paths, and your normalization layer does not account for that. It looses the passed value

stof · 2015-12-15T13:18:08Z

@a-sane my suggestion for a quick resolution is to convert your code to the new way of passing choices for Symfony 2.7+, i.e. with choices_as_value set to true

ureimers · 2016-02-10T13:51:14Z

The problem seems to be that they keys in choice_attr are actually the numbered indexes from the choices just like as if they where transformed using array_values($choices).

->add('action', 'choice', [
  'choices' => [
    'a' => 'Yes', // index 0
    'b' => 'no', // index 1
    'c' => 'Maybe', // index 2
  ],
  'choice_attr' => [
    1 => ['data-info' => 'option b'],
  ],
)

<select>
  <option value="a">Yes</option>
  <option value="b" data-info="option b">no</option>
  <option value="c">Maybe</option>
</select>

The problem is even more obvious when using:

->add('action', 'choice', [
  'choices' => [
    'a' => 'Yes', // index 0
    'b' => 'no', // index 1
    'c' => 'Maybe', // index 2
  ],
  'choice_attr' => function($choice, $key, $value) {
    return ['data-choice' => $choice, 'data-key' => $key];
  }]
)

<select>
  <option value="a" data-choice="a" data-key="0">Yes</option>
  <option value="b" data-choice="b" data-key="1">no</option>
  <option value="c" data-choice="c" data-key="2">Maybe</option>
</select>

The keys should be 'a', 'b' and 'c' and not 0, 1 and 2. The callback parameters are named just like they are used in DefaultChoiceListFactory:130. I don't know the difference between $choice and $value, but $key is definitely not $key but more like $index.

HeahDude · 2016-02-10T14:20:26Z

Hi @ureimers, actually when you set choices like

array(
    'a' => 'Yes', // $key = 'a', $value = 'Yes' and $index = 0
    'b' => 'no', // $key = 'b', $value = 'no' and $index = 1
    'c' => 'Maybe', // $key = 'c', $value = 'Maybe' and $index = 2
  ),

because when choices_s_values is false, they converted to an ArrayChoiceList which "holds" two arrays :

array(
    0 => 'a', // $index => $key value normalized for html attribute "value"
    1 => 'b',
    2 => 'c',
)
// and
array(
    0 => 'Yes', // $index => $choice as value choosen via its label
    1 => 'no',
    2 => 'Maybe'
)

So you got a callable for choice_attr which takes :

`choice_attr` => function ($choice, $index, $value) {
    return array(
        $index => array('data-property' => $choice->getProperty()),
        // or
        $index => array('data-string-value' => $value)
        ...
        // intial post example is missing the index 0 and that's the real issue.
    );
},

Expected behaviour.

You can close this issue, feel free to open one in the docs.

HeahDude · 2016-02-10T14:20:45Z

Status: reviewed

HeahDude · 2016-02-10T15:02:38Z

I've updated my answer to better explain what happened in initial post.

ureimers · 2016-02-11T18:54:02Z

I see, thanks @HeahDude for the reply.

The main problem of the initial post seems to be to use the old way of specifying choices (choices_as_values => false) doesn't work well together with using an array for choice_attr.

When using choice_as_values => true everything works as expected:

$builder->add('attending', 'choice', array(
    'choices' => array(
        'Yes' => 2,
        'No' => 1,
        'Maybe' => 4,
    ),
    'choices_as_values' => true,
    'choice_attr' => array(
        'Maybe' => array('class' => 'text-muted'),
    ),
));

But the other way around it doesn't:

$builder->add('attending', 'choice', array(
    'choices' => array( // switched
        2 => 'Yes',
        1 => 'No',
        4 => 'Maybe',
    ),
    'choices_as_values' => false, // switched
    'choice_attr' => array(
        4 => array('class' => 'text-muted'), // won't add any class to 'Maybe'
    ),
));

Internally this may make sense, but from the view of a developer using the form component as an API it doesn't.

HeahDude · 2016-02-12T02:05:10Z

Thanks @ureimers for your feedback, it seems you're right about a different behaviour of choice_attr depending on choices_as_values. I try to recap how it is actually working, because many people seem confused with this.

Using `choice_attr` with an array in 2.x

$builder->add('opinion', 'choice', array(
    'choices' => array(
        'Yes' => 10, // index = 0
        'Nop' => 7, // index = 1
        'Maybe' => 5, // index = 2
    ),
    'choices_as_values' => false, // default in 2.x
    'choice_attr' => array(
        2 => array('data-response' => 'Never mind'), // will map 'Maybe' with index 2
        0 => array('data-response' => 'Great!'), // will map 'Yes' with index 0
        1 => array('data-response' => 'Sure ?'), // will map 'Nop' with index 1

        // using anything else as keys (key 'Yes' or value '10' will fail
        // 'Yes' => array('data-response' => 'Great!') won't work

        // unless you set 'choices_as_values'  to true
        'Yes' => array('data-response' => 'Great!'), // will map 'Yes' by its key.
        // 0 => array('data-response' => 'Great!'), // will fail mapping
    ),
);

Statement : choice_attr as an array maps attributes by a flatten index (default)
or by choice keys when choices_as_values is true.
Issue : minor if it's explicit in the doc, this is an expected behaviour.

Using `choice_attr` with a callable in 2.x

$builder->add('opinion', 'choice', array(
    'choices' => array(
        'Yes' => 10, // index = 0
        'Nop' => 7, // index = 1
        'Maybe' => 5, // index = 2
    ),
    'choices_as_values' => false, // default in 2.x means that choices "value" are the keys of choices array
    // values of choices array are only used for labels.
    'choice_attr' => function ($choiceValue, $choiceIndex, $definedValue) {
        // $choiceValue = 'Yes', $choiceIndex = 0, $definedValue = 'Yes',
        // since we did not set some defined values with 'choice_value' they default to keys of choices array

        return array('data-index' => $choiceIndex);
    },
);

Will output :

<option ... data-index="0" value="Yes">10</option>
<option ... data-index="1" value="Nop">7</option>
<option ... data-index="2" value="Maybe">5</option>

And with choices_as_values true :

'choices_as_values' => true, // means that choices "value" are the values of the choices array
// keys of the choices array will be used as label
'choice_attr' => function ($choiceValue, $choiceKey, $definedValue) {
    // $choiceValue = '10', $choiceKey = 'Yes', $definedValue = 10

    return array('data-test' => $choiceKey);
},

Will output :

<option ... data-test="Yes" value="10">Yes</option>
<option ... data-test="Nop" value="7">Nop</option>
<option ... data-test="Maybe" value="5">Maybe</option>

Caution :
Either keys or values of the choice array will be used as values in html attribute value of inputs, so they need to be cast to string. You can let the component take care of it for you, the only thing you must remember is that null will be cast to empty string '' but false too (true will be cast to string '1'), it might be fixed by #17760, anyway you can use choice_value to handle it :

'choice_value` => function ($choiceValue) {
    return false === $choiceValue ? '0' : (string) $choiceValue;
}

Finally :
In real dev, you can use both and get something like :

$builder->add('opinion', 'choice', array(
    'choices' => array(
        'Yes' => 1,
        'Nop' => 0,
        'Maybe' => null,
    ),
    'choices_as_values' => true // string keys will be used for labels.
    'choice_value' => function ($choiceValue) {
        if (null === $choiceValue) {
            return 'null';
        }

        return $choiceValue ? 'true' : 'false';
    },
    'choice_attr' => function ($choiceValue, $choiceKey, $definedValue) {
        // $choiceValue = '1', $choiceKey = 'Yes', $definedValue = 'true'

        return array('data-raw-value' => $choiceValue);
    },
);

Will output :

<option ... data-raw-value="1" value="true">Yes</option>
<option ... data-raw-value="0" value="false">Nop</option>
<option ... data-raw-value="" value="null">Maybe</option>

Statement : To avoid confusion, don't use third argument $definedValue in the callable as it is the same as $choiceValue no matter how choices_as_values is set, unless you have specifically defined choice_value option.
Issue : none (IMO)

Conclusion

Prior to 2.7, you set your choices with an array like :

$choices = array(
    'String value for server use' => 'string label',
)

or you need to create a ChoiceList instance to get it more complex (handling objects ...).
Since 2.7 this usage is deprecated and you should use an array like :

$choices = array(
    'String label' => $anyServerValueEvenObject,
    // or simplier for syntax below
    $label => $choice,
)

and use the option choices_as_values as true so an ArrayChoiceList is created behind the scene.
You have then access to each $choice via PropertyPath or callable for many new options including :

`choice_value` => 'property' // property path, if $choice is an array = $choice['property'] and if choice is an object = $choice->getProperty()
// both must return a string or the component will try to cast it
`choice_value` => function ($choice) {
    // will be called on each choice
    // takes only the choice as argument
    // must return a unique custom string representation of this choice
    return (string) $choice; // default
},
'choice_label' => function ($choice, $label, $value) {
    // $value is the string return by the option 'choice_value', default to (string) $choice
    return $label; // default
},

choice_attr, choice_name, preffered_choices, group_by work the same way as choice_label and all could be property path or an array mapping each choice by its $label key :

'choice_attr` => array(
    'label' => $attributes,
)

or for BC mapped by flatten index when choices_as_values is false.

Again the original post is missing the key 0 as choices_as_values is false by default in his example.

Hope this helps clarifying a bit.

I think this issue should be closed.

ureimers · 2016-02-12T10:11:10Z

Phew, that was a comprehensive but very clear roundup. Thank you for taking the time to write it down!

HeahDude · 2016-02-12T10:26:24Z

@ureimers thank you, I was afraid it was to long for someone to read, but I could not make it shorter while keeping it as clear as possible. Hope it will help some others to better understand the way it goes.

xabbuh · 2016-02-17T17:26:12Z

So do you think there is anything we can do in the code or can we "fix" it by adding something to the documentation? What do you think?

HeahDude · 2016-02-18T13:42:07Z

@xabbuh I'm still a beginner/learner, but I don't think the code required to be fixed for that, it seems consistent as it is to me, but yes I think the docs should mention how choices are indexed :

by numerical order when choices_as_values is false (for BC it should remain that way IMHO)
by string label (choice keys) when choices_as_values is true

Tobion · 2016-02-18T14:54:01Z

Closing for the detailed reasoning by @HeahDude.
Also keep in mind choice_attr, choice_label etc are all new options in 2.7 that have been designed for 'choices_as_values' => true. So using them with 'choices_as_values' => false may not even be supported and mixing them will lead to headaches as it causes different behavior as outlines above. So just don't use the deprecated choices_as_values = false for new stuff.

xabbuh added the Form label Dec 15, 2015

javiereguiluz added the Bug label Feb 7, 2016

carsonbot added the Status: Needs Review label Feb 7, 2016

carsonbot added Status: Reviewed and removed Status: Needs Review labels Feb 10, 2016

Tobion closed this as completed Feb 18, 2016

This was referenced Mar 23, 2016

[Form] Fix ChoiceType options symfony/symfony-docs#6393

Closed

[Form] [DX] choice_attr and choice_label_attr options in ChoiceType #18318

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form] choice_attr option in ChoiceType generates options attributes wrong #17019

[Form] choice_attr option in ChoiceType generates options attributes wrong #17019

a-sane commented Dec 15, 2015

stof commented Dec 15, 2015

stof commented Dec 15, 2015

ureimers commented Feb 10, 2016

HeahDude commented Feb 10, 2016

HeahDude commented Feb 10, 2016

HeahDude commented Feb 10, 2016

ureimers commented Feb 11, 2016

HeahDude commented Feb 12, 2016

ureimers commented Feb 12, 2016

HeahDude commented Feb 12, 2016

xabbuh commented Feb 17, 2016

HeahDude commented Feb 18, 2016

Tobion commented Feb 18, 2016

[Form] choice_attr option in ChoiceType generates options attributes wrong #17019

[Form] choice_attr option in ChoiceType generates options attributes wrong #17019

Comments

a-sane commented Dec 15, 2015

stof commented Dec 15, 2015

stof commented Dec 15, 2015

ureimers commented Feb 10, 2016

HeahDude commented Feb 10, 2016

HeahDude commented Feb 10, 2016

HeahDude commented Feb 10, 2016

ureimers commented Feb 11, 2016

HeahDude commented Feb 12, 2016

Using choice_attr with an array in 2.x

Using choice_attr with a callable in 2.x

Conclusion

ureimers commented Feb 12, 2016

HeahDude commented Feb 12, 2016

xabbuh commented Feb 17, 2016

HeahDude commented Feb 18, 2016

Tobion commented Feb 18, 2016

Using `choice_attr` with an array in 2.x

Using `choice_attr` with a callable in 2.x