Skip to content

[2.7][Form] Patch label_attr in choice_attr. Fix #14404, #14165, #14393 #14405

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 6 commits into from
Closed

[2.7][Form] Patch label_attr in choice_attr. Fix #14404, #14165, #14393 #14405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
beb5044
Add test
HeahDude Apr 19, 2015
4156b24
Patch
HeahDude Apr 19, 2015
8fa549d
clean comments + cs fix
HeahDude Apr 19, 2015
737f308
fix-14393
HeahDude Apr 19, 2015
d672aba
fix-14165-14393
HeahDude Apr 19, 2015
1f3ee53
fix bootstrap tests
HeahDude Apr 19, 2015
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ package*.tar
packages.json
phpunit.xml
/vendor/
/.idea
Copy link
Contributor

Choose a reason for hiding this comment

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

this belongs into your own global ignore

9 changes: 7 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
use Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory;
Expand Down Expand Up @@ -248,7 +247,7 @@ public function configureOptions(OptionsResolver $resolver)
};

$emptyValue = function (Options $options) {
return $options['required'] ? null : '';
return $options['required'] && empty($options['choices']) ? '' : null;
};

// for BC with the "empty_value" option
Expand Down Expand Up @@ -418,6 +417,12 @@ private function addSubForm(FormBuilderInterface $builder, $name, ChoiceView $ch
'block_name' => 'entry',
);

// Resolve label_attr to the right level
if (isset($choiceOpts['attr']['label_attr'])) {
$choiceOpts['label_attr'] = $choiceOpts['attr']['label_attr'];
unset($choiceOpts['attr']['label_attr']);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not the correct thing. the attr is for html attributes. but label_attr doesn't exist as html attr.
To support your use-case, we would need to add another option: choice_label_attr with the same logic as the other choice options.

}

if ($options['multiple']) {
$choiceType = 'checkbox';
// The user can check 0 or more checkboxes. If required
Expand Down
10 changes: 4 additions & 6 deletions src/Symfony/Component/Form/Tests/AbstractBootstrap3LayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,10 @@ public function testSingleChoiceNonRequired()
[@class="my&class form-control"]
[not(@required)]
[
./option[@value=""][.="[trans][/trans]"]
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
./option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
]
[count(./option)=3]
[count(./option)=2]
'
);
}
Expand All @@ -383,11 +382,10 @@ public function testSingleChoiceNonRequiredNoneSelected()
[@class="my&class form-control"]
[not(@required)]
[
./option[@value=""][.="[trans][/trans]"]
/following-sibling::option[@value="&a"][not(@selected)][.="[trans]Choice&A[/trans]"]
./option[@value="&a"][not(@selected)][.="[trans]Choice&A[/trans]"]
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
]
[count(./option)=3]
[count(./option)=2]
'
);
}
Expand Down
60 changes: 60 additions & 0 deletions src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,4 +778,64 @@ public function testWidgetContainerAttributeNameRepeatedIfTrue()
// foo="foo"
$this->assertContains('<div id="form" foo="foo">', $html);
}

/**
* This is a test for div rendering and maybe needs testing with bootstrap and table layouts
*
* @dataProvider getOptionsWithChoiceAttr
*/
public function testFormWithChoiceFieldHavingOptionsWithChoiceAttr($multiple)
{
$view = $this->factory->createNamedBuilder('name', 'form')
->add('choice', 'choice', array(
'required' => false,
'expanded' => true,
'multiple' => $multiple,
'attr' => array(
'class' => 'choice_field_class', // expected
),
'label_attr' => array(
'class' => 'choice_label_class', // expected
),
'choices' => array(
'Bernhard' => 'a',
'Fabien' => 'b',
),
'choices_as_values' => true,
'choice_attr' => function($option, $key) {
return array(
'class' => 'option_input_class', // expected
'label_attr' => array(
'class' => 'option_label_class', // expected
),
);
},
))
->getForm()
->createView();

$html = $this->renderRow($view['choice']);

// __OK__ @Field div --expected class="choice_field_class" | attr
$this->assertContains('<div id="name_choice" class="choice_field_class">', $html);

// __OK__ @Field_label --expected class="choice_label_class" | label_attr
$this->assertContains('<label class="choice_label_class">[trans]Choice[/trans]</label>', $html);

// All green
// __OK__ @Options x2 --error label_attr=_ARRAY_ | choice_attr
if (!$multiple) {
$this->assertContains('<input type="radio" id="name_choice_0" name="name[choice]" class="option_input_class" value="0" />', $html);
} else {
$this->assertContains('<input type="checkbox" id="name_choice_0" name="name[choice][]" class="option_input_class" value="0" />', $html);
}

// __OK__ @Options_label --expected class="option_label_class" | choice_attr [ label_attr ]
$this->assertContains('<label class="option_label_class" for="name_choice_0">[trans]Bernhard[/trans]</label>', $html);
}

public function getOptionsWithChoiceAttr()
{
return array(array(true), array(false));
}
}
10 changes: 4 additions & 6 deletions src/Symfony/Component/Form/Tests/AbstractLayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,10 @@ public function testSingleChoiceNonRequired()
[@name="name"]
[not(@required)]
[
./option[@value=""][.="[trans][/trans]"]
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
./option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
]
[count(./option)=3]
[count(./option)=2]
'
);
}
Expand All @@ -658,11 +657,10 @@ public function testSingleChoiceNonRequiredNoneSelected()
[@name="name"]
[not(@required)]
[
./option[@value=""][.="[trans][/trans]"]
/following-sibling::option[@value="&a"][not(@selected)][.="[trans]Choice&A[/trans]"]
./option[@value="&a"][not(@selected)][.="[trans]Choice&A[/trans]"]
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
]
[count(./option)=3]
[count(./option)=2]
'
);
}
Expand Down