Skip to content

Commit 4420274

Browse files
lacatoirenicolas-grekas
authored andcommitted
[Form] Document the hidden placeholder of required ChoiceType fields
Since 8.1, placeholder_attr defaults to ["hidden" => true] on required collapsed ChoiceType fields, but the change shipped without CHANGELOG or UPGRADE entries and no layout test asserts the attribute. Add both notes and pin the rendering at HTML level: hidden on the placeholder option of required fields, absent for non-required fields and for placeholders injected via view variables (the default only applies to the placeholder option). The required-field assertions detect the installed symfony/form behavior so the bridge test suite keeps passing against 7.4/8.0 in the lowest-dependencies jobs.
1 parent 4f44742 commit 4420274

3 files changed

Lines changed: 31 additions & 7 deletions

File tree

Tests/Extension/AbstractBootstrap3LayoutTestCase.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,13 +775,14 @@ public function testSingleChoiceRequiredWithPlaceholder()
775775
'placeholder' => 'Test&Me',
776776
]);
777777

778+
$placeholderHidden = $this->isRequiredPlaceholderHiddenByDefault() ? '[@hidden="hidden"]' : '[not(@hidden)]';
778779
$this->assertWidgetMatchesXpath($form->createView(), ['attr' => ['class' => 'my&class']],
779780
'/select
780781
[@name="name"]
781782
[@class="my&class form-control"]
782783
[@required="required"]
783784
[
784-
./option[@value=""][not(@selected)][not(@disabled)][.="[trans]Test&Me[/trans]"]
785+
./option[@value=""][not(@selected)][not(@disabled)]'.$placeholderHidden.'[.="[trans]Test&Me[/trans]"]
785786
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
786787
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
787788
]
@@ -799,13 +800,15 @@ public function testSingleChoiceRequiredWithPlaceholderViaView()
799800
'expanded' => false,
800801
]);
801802

803+
// The hidden attribute is only defaulted for the "placeholder" option:
804+
// placeholders injected via view variables render without it.
802805
$this->assertWidgetMatchesXpath($form->createView(), ['placeholder' => '', 'attr' => ['class' => 'my&class']],
803806
'/select
804807
[@name="name"]
805808
[@class="my&class form-control"]
806809
[@required="required"]
807810
[
808-
./option[@value=""][not(@selected)][not(@disabled)][.=""]
811+
./option[@value=""][not(@selected)][not(@disabled)][not(@hidden)][.=""]
809812
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
810813
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
811814
]

Tests/Extension/AbstractBootstrap5LayoutTestCase.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -783,13 +783,14 @@ public function testSingleChoiceRequiredWithPlaceholder()
783783
'placeholder' => 'Test&Me',
784784
]);
785785

786+
$placeholderHidden = $this->isRequiredPlaceholderHiddenByDefault() ? '[@hidden="hidden"]' : '[not(@hidden)]';
786787
$this->assertWidgetMatchesXpath($form->createView(), ['attr' => ['class' => 'my&class']],
787788
'/select
788789
[@name="name"]
789790
[@class="my&class form-select"]
790791
[@required="required"]
791792
[
792-
./option[@value=""][not(@selected)][not(@disabled)][.="[trans]Test&Me[/trans]"]
793+
./option[@value=""][not(@selected)][not(@disabled)]'.$placeholderHidden.'[.="[trans]Test&Me[/trans]"]
793794
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
794795
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
795796
]
@@ -807,13 +808,15 @@ public function testSingleChoiceRequiredWithPlaceholderViaView()
807808
'expanded' => false,
808809
]);
809810

811+
// The hidden attribute is only defaulted for the "placeholder" option:
812+
// placeholders injected via view variables render without it.
810813
$this->assertWidgetMatchesXpath($form->createView(), ['placeholder' => '', 'attr' => ['class' => 'my&class']],
811814
'/select
812815
[@name="name"]
813816
[@class="my&class form-select"]
814817
[@required="required"]
815818
[
816-
./option[@value=""][not(@selected)][not(@disabled)][.=""]
819+
./option[@value=""][not(@selected)][not(@disabled)][not(@hidden)][.=""]
817820
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
818821
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
819822
]

Tests/Extension/AbstractLayoutTestCase.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,21 @@ protected function tearDown(): void
6363
}
6464
}
6565

66+
/**
67+
* The "hidden" placeholder_attr default of required ChoiceType fields depends
68+
* on the installed symfony/form version (the default was introduced in 8.1).
69+
*/
70+
protected function isRequiredPlaceholderHiddenByDefault(): bool
71+
{
72+
$view = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ChoiceType', null, [
73+
'choices' => [],
74+
'required' => true,
75+
'placeholder' => 'placeholder',
76+
])->createView();
77+
78+
return (bool) ($view->vars['placeholder_attr']['hidden'] ?? false);
79+
}
80+
6681
protected function assertWidgetMatchesXpath(FormView $view, array $vars, $xpath)
6782
{
6883
// include ampersands everywhere to validate escaping
@@ -820,7 +835,7 @@ public function testSingleChoiceNonRequiredWithPlaceholder()
820835
[@name="name"]
821836
[not(@required)]
822837
[
823-
./option[@value=""][not(@selected)][not(@disabled)][.="[trans]Select&Anything&Not&Me[/trans]"]
838+
./option[@value=""][not(@selected)][not(@disabled)][not(@hidden)][.="[trans]Select&Anything&Not&Me[/trans]"]
824839
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
825840
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
826841
]
@@ -842,12 +857,13 @@ public function testSingleChoiceRequiredWithPlaceholder()
842857
// The "disabled" attribute was removed again due to a bug in the
843858
// BlackBerry 10 browser.
844859
// See https://github.com/symfony/symfony/pull/7678
860+
$placeholderHidden = $this->isRequiredPlaceholderHiddenByDefault() ? '[@hidden="hidden"]' : '[not(@hidden)]';
845861
$this->assertWidgetMatchesXpath($form->createView(), [],
846862
'/select
847863
[@name="name"]
848864
[@required="required"]
849865
[
850-
./option[@value=""][not(@selected)][not(@disabled)][.="[trans]Test&Me[/trans]"]
866+
./option[@value=""][not(@selected)][not(@disabled)]'.$placeholderHidden.'[.="[trans]Test&Me[/trans]"]
851867
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
852868
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
853869
]
@@ -868,12 +884,14 @@ public function testSingleChoiceRequiredWithPlaceholderViaView()
868884
// The "disabled" attribute was removed again due to a bug in the
869885
// BlackBerry 10 browser.
870886
// See https://github.com/symfony/symfony/pull/7678
887+
// The hidden attribute is only defaulted for the "placeholder" option:
888+
// placeholders injected via view variables render without it.
871889
$this->assertWidgetMatchesXpath($form->createView(), ['placeholder' => ''],
872890
'/select
873891
[@name="name"]
874892
[@required="required"]
875893
[
876-
./option[@value=""][not(@selected)][not(@disabled)][.=""]
894+
./option[@value=""][not(@selected)][not(@disabled)][not(@hidden)][.=""]
877895
/following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"]
878896
/following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"]
879897
]

0 commit comments

Comments
 (0)