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

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).

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') }}

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>