merged branch craue/patch-9 (PR #1787)

fabpot · fabpot · commit c248f8123d7b · 2011-07-27T07:59:53.000+02:00
Commits ------- c558b78 avoid rendering the `ChoiceType` separator if all `choices` are `preferred_choices` Discussion ---------- avoid rendering the `ChoiceType` separator if all `choices` are `preferred_choices` --------------------------------------------------------------------------- by fabpot at 2011/07/24 00:51:21 -0700 The same change should be made to the PHP template. --------------------------------------------------------------------------- by fabpot at 2011/07/25 00:31:39 -0700 I forgot to ask you to add some unit tests too. Thanks. --------------------------------------------------------------------------- by craue at 2011/07/25 10:23:34 -0700 Are you asking for PHPUnit tests? If so, unfortunately, I'm not able to add those because I haven't used PHPUnit yet. ;) --------------------------------------------------------------------------- by lenar at 2011/07/25 12:47:51 -0700 I would prefer ```choises | length``` without spaces as everywhere else. --------------------------------------------------------------------------- by lenar at 2011/07/25 12:50:32 -0700 @fabpot: Since <option disabled> is unclickable in browser (by HTML spec) this really doesn't change anything (something not there is as unclickable) except the the look when rendered. I have hard time to imagine what could become unit-testable here by this change. --------------------------------------------------------------------------- by stof at 2011/07/25 13:03:47 -0700 @lenar unit testing is not about what the browser could do. What should be unit-tested is that an example will only preferred choices does not output the separator, which is exactly what this PR is about --------------------------------------------------------------------------- by stof at 2011/07/25 13:04:03 -0700 @lenar unit testing is not about what the browser could do. What should be unit-tested is that an example will only preferred choices does not output the separator, which is exactly what this PR is about --------------------------------------------------------------------------- by lenar at 2011/07/25 13:08:33 -0700 @stof: ok, put this way you are definitely right. --------------------------------------------------------------------------- by craue at 2011/07/25 13:37:50 -0700 @lenar: You're right about the spaces. I'm using them in my projects but will remove them here for the sake of consistency. --------------------------------------------------------------------------- by stloyd at 2011/07/25 13:40:40 -0700 @craue I will write today/tomorrow test to cover your code and send you PR. --------------------------------------------------------------------------- by craue at 2011/07/25 14:00:26 -0700 @stloyd: That would be nice. But I'm still not that familiar with Git(Hub). Is there anything I have to take care of? Also, I'd like to squash my three commits into one ... if this is possible for an open PR and if I find out how to do that easily. :D --------------------------------------------------------------------------- by fabpot at 2011/07/26 00:18:22 -0700 @craue: yes, you should squash your commits into one and use `--force` when you push (the PR will automatically be updated accordingly).
diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
@@ -57,7 +57,9 @@
         {% if preferred_choices|length > 0 %}
             {% set options = preferred_choices %}
             {{ block('widget_choice_options') }}
-            <option disabled="disabled">{{ separator }}</option>
+            {% if choices|length > 0 %}
+                <option disabled="disabled">{{ separator }}</option>
+            {% endif %}
         {% endif %}
         {% set options = choices %}
         {{ block('widget_choice_options') }}
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget.html.php
@@ -13,7 +13,9 @@
         <?php if (null !== $empty_value): ?><option value=""><?php echo $view->escape($view['translator']->trans($empty_value)) ?></option><?php endif; ?>
         <?php if (count($preferred_choices) > 0): ?>
             <?php echo $view['form']->renderBlock('choice_options', array('options' => $preferred_choices)) ?>
-            <option disabled="disabled"><?php echo $separator ?></option>
+            <?php if (count($choices) > 0): ?>
+                <option disabled="disabled"><?php echo $separator ?></option>
+            <?php endif ?>
         <?php endif ?>
         <?php echo $view['form']->renderBlock('choice_options', array('options' => $choices)) ?>
     </select>
