Skip to content

Commit

Permalink
bug #24728 [Bridge\Twig] fix bootstrap checkbox_row to render properl…
Browse files Browse the repository at this point in the history 
…y & remove spaceless (arkste)

This PR was squashed before being merged into the 3.4 branch (closes #24728).

Discussion
----------

[Bridge\Twig] fix bootstrap checkbox_row to render properly & remove spaceless

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24711
| License       | MIT
| Doc PR        | -

As discussed in #24711 i reverted the change i did in `bootstrap_3_layout.html.twig` (which caused an unnecessary empty div-container in the vertical-layout), added the `checkbox_row` block to the `bootstrap_3_horizontal_layout.html.twig` and removed `spaceless` (as proposed in #24727).

since i added `{#--#}` in bootstrap 3, i did the same for the same horizontal blocks in bootstrap 4 as well.

I moved the `form_label_class` & `form_group_class` blocks to the top of `bootstrap_3_horizontal_layout.html.twig` & `bootstrap_4_horizontal_layout.html.twig`, this should improve DX as they were spreaded across the file.

#24702 affected the bootstrap 4 horizontal layout as well, so i added the `checkbox_row` block to bootstrap 4 too.

ping @fabpot @nicolas-grekas

Commits
-------

f84749f [Bridge\Twig] fix bootstrap checkbox_row to render properly & remove spaceless
  • Loading branch information
@fabpot
fabpot committed Oct 30, 2017
2 parents 039250a + f84749f commit 4ae046f
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 61 deletions.
44 changes: 24 additions & 20 deletions src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_horizontal_layout.html.twig
View file
Expand Up @@ -8,14 +8,12 @@
{# Labels #}

{% block form_label -%}
{% spaceless %}
{% if label is same as(false) %}
{%- if label is same as(false) -%}
<div class="{{ block('form_label_class') }}"></div>
{% else %}
{% set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' ' ~ block('form_label_class'))|trim}) %}
{%- else -%}
{%- set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' ' ~ block('form_label_class'))|trim}) -%}
{{- parent() -}}
{% endif %}
{% endspaceless %}
{%- endif -%}
{%- endblock form_label %}

{% block form_label_class -%}
Expand All @@ -35,27 +33,33 @@ col-sm-2
{%- endblock form_row %}

{% block submit_row -%}
{% spaceless %}
<div class="form-group">
<div class="{{ block('form_label_class') }}"></div>
<div class="form-group">{#--#}
<div class="{{ block('form_label_class') }}"></div>{#--#}
<div class="{{ block('form_group_class') }}">
{{ form_widget(form) }}
</div>
{{- form_widget(form) -}}
</div>{#--#}
</div>
{% endspaceless %}
{% endblock submit_row %}
{%- endblock submit_row %}

{% block reset_row -%}
{% spaceless %}
<div class="form-group">
<div class="{{ block('form_label_class') }}"></div>
<div class="form-group">{#--#}
<div class="{{ block('form_label_class') }}"></div>{#--#}
<div class="{{ block('form_group_class') }}">
{{ form_widget(form) }}
</div>
{{- form_widget(form) -}}
</div>{#--#}
</div>
{% endspaceless %}
{% endblock reset_row %}
{%- endblock reset_row %}

{% block form_group_class -%}
col-sm-10
{%- endblock form_group_class %}

{% block checkbox_row -%}
<div class="form-group{% if not valid %} has-error{% endif %}">{#--#}
<div class="{{ block('form_label_class') }}"></div>{#--#}
<div class="{{ block('form_group_class') }}">
{{- form_widget(form) -}}
{{- form_errors(form) -}}
</div>{#--#}
</div>
{%- endblock checkbox_row %}
47 changes: 21 additions & 26 deletions src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_layout.html.twig
View file
Expand Up @@ -10,7 +10,7 @@
{%- endblock form_widget_simple %}

{% block button_widget -%}
{% set attr = attr|merge({class: (attr.class|default('btn-default') ~ ' btn')|trim}) %}
{%- set attr = attr|merge({class: (attr.class|default('btn-default') ~ ' btn')|trim}) -%}
{{- parent() -}}
{%- endblock button_widget %}

Expand All @@ -22,18 +22,18 @@
<div class="checkbox">
{{- form_label(form, null, { widget: parent() }) -}}
</div>
{%- endif %}
{%- endif -%}
{%- endblock checkbox_widget %}

{% block radio_widget -%}
{%- set parent_label_class = parent_label_class|default(label_attr.class|default('')) -%}
{% if 'radio-inline' in parent_label_class %}
{%- if 'radio-inline' in parent_label_class -%}
{{- form_label(form, null, { widget: parent() }) -}}
{% else -%}
{%- else -%}
<div class="radio">
{{- form_label(form, null, { widget: parent() }) -}}
</div>
{%- endif %}
{%- endif -%}
{%- endblock radio_widget %}

{# Labels #}
Expand Down Expand Up @@ -61,30 +61,30 @@
{{- block('checkbox_radio_label') -}}
{%- endblock radio_label %}

{% block checkbox_radio_label %}
{% block checkbox_radio_label -%}
{# Do not display the label if widget is not defined in order to prevent double label rendering #}
{% if widget is defined %}
{% if required %}
{% set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' required')|trim}) %}
{% endif %}
{% if parent_label_class is defined %}
{% set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' ' ~ parent_label_class)|trim}) %}
{% endif %}
{% if label is not same as(false) and label is empty %}
{%- if widget is defined -%}
{%- if required -%}
{%- set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' required')|trim}) -%}
{%- endif -%}
{%- if parent_label_class is defined -%}
{%- set label_attr = label_attr|merge({class: (label_attr.class|default('') ~ ' ' ~ parent_label_class)|trim}) -%}
{%- endif -%}
{%- if label is not same as(false) and label is empty -%}
{%- if label_format is not empty -%}
{% set label = label_format|replace({
{%- set label = label_format|replace({
'%name%': name,
'%id%': id,
}) %}
}) -%}
{%- else -%}
{% set label = name|humanize %}
{%- endif -%}
{% endif %}
{%- endif -%}
<label{% for attrname, attrvalue in label_attr %} {{ attrname }}="{{ attrvalue }}"{% endfor %}>
{{- widget|raw }} {{ label is not same as(false) ? (translation_domain is same as(false) ? label : label|trans({}, translation_domain)) -}}
</label>
{% endif %}
{% endblock checkbox_radio_label %}
{%- endif -%}
{%- endblock checkbox_radio_label %}

{# Rows #}

Expand Down Expand Up @@ -123,15 +123,10 @@
{%- endblock datetime_row %}

{% block checkbox_row -%}
{% spaceless %}
<div class="form-group{% if not valid %} has-error{% endif %}">
<div class="{{ block('form_label_class') }}"></div>
<div class="{{ block('form_group_class') }}">
{{- form_widget(form) -}}
{{- form_errors(form) -}}
</div>
{{- form_widget(form) -}}
{{- form_errors(form) -}}
</div>
{% endspaceless %}
{%- endblock checkbox_row %}

{% block radio_row -%}
Expand Down
22 changes: 16 additions & 6 deletions src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_horizontal_layout.html.twig
View file
Expand Up @@ -47,23 +47,33 @@ col-sm-2
{%- endblock fieldset_form_row %}

{% block submit_row -%}
<div class="form-group row">
<div class="{{ block('form_label_class') }}"></div>
<div class="form-group row">{#--#}
<div class="{{ block('form_label_class') }}"></div>{#--#}
<div class="{{ block('form_group_class') }}">
{{- form_widget(form) -}}
</div>
</div>{#--#}
</div>
{%- endblock submit_row %}

{% block reset_row -%}
<div class="form-group row">
<div class="{{ block('form_label_class') }}"></div>
<div class="form-group row">{#--#}
<div class="{{ block('form_label_class') }}"></div>{#--#}
<div class="{{ block('form_group_class') }}">
{{- form_widget(form) -}}
</div>
</div>{#--#}
</div>
{%- endblock reset_row %}

{% block form_group_class -%}
col-sm-10
{%- endblock form_group_class %}

{% block checkbox_row -%}
<div class="form-group row">{#--#}
<div class="{{ block('form_label_class') }}"></div>{#--#}
<div class="{{ block('form_group_class') }}">
{{- form_widget(form) -}}
{{- form_errors(form) -}}
</div>{#--#}
</div>
{%- endblock checkbox_row %}
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig
View file
Expand Up @@ -24,9 +24,9 @@
{% block checkbox_widget -%}
{%- set parent_label_class = parent_label_class|default(label_attr.class|default('')) -%}
{%- set attr = attr|merge({class: attr.class|default('form-check-input')}) -%}
{%- if 'checkbox-inline' in parent_label_class -%}
{% if 'checkbox-inline' in parent_label_class %}
{{- form_label(form, null, { widget: parent() }) -}}
{%- else -%}
{% else -%}
<div class="form-check">
{{- form_label(form, null, { widget: parent() }) -}}
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/composer.json
View file
Expand Up @@ -24,7 +24,7 @@
"symfony/asset": "~2.8|~3.0|~4.0",
"symfony/dependency-injection": "~2.8|~3.0|~4.0",
"symfony/finder": "~2.8|~3.0|~4.0",
"symfony/form": "~3.4|~4.0",
"symfony/form": "~3.4-beta2|~4.0",
"symfony/http-foundation": "^3.3.11|~4.0",
"symfony/http-kernel": "~3.2|~4.0",
"symfony/polyfill-intl-icu": "~1.0",
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Form/Tests/AbstractBootstrap3HorizontalLayoutTest.php
View file
Expand Up @@ -154,4 +154,13 @@ public function testStartTagWithExtraAttributes()

$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar form-horizontal">', $html);
}

public function testCheckboxRow()
{
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\CheckboxType');
$view = $form->createView();
$html = $this->renderRow($view, array('label' => 'foo'));

$this->assertMatchesXpath($html, '/div[@class="form-group"]/div[@class="col-sm-2" or @class="col-sm-10"]', 2);
}
}
15 changes: 12 additions & 3 deletions src/Symfony/Component/Form/Tests/AbstractBootstrap4HorizontalLayoutTest.php
View file
Expand Up @@ -26,8 +26,8 @@ public function testLabelOnForm()
$html = $this->renderLabel($view);

$this->assertMatchesXpath($html,
'/label
[@class="col-form-label col-sm-2 form-control-label required"]
'/legend
[@class="col-form-label col-sm-2 col-form-legend required"]
[.="[trans]Name[/trans]"]
'
);
Expand Down Expand Up @@ -118,7 +118,7 @@ public function testLegendOnExpandedType()

$this->assertMatchesXpath($html,
'/legend
[@class="col-sm-2 col-form-legend form-control-label required"]
[@class="col-sm-2 col-form-legend required"]
[.="[trans]Custom label[/trans]"]
'
);
Expand Down Expand Up @@ -178,4 +178,13 @@ public function testStartTagWithExtraAttributes()

$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar">', $html);
}

public function testCheckboxRow()
{
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\CheckboxType');
$view = $form->createView();
$html = $this->renderRow($view, array('label' => 'foo'));

$this->assertMatchesXpath($html, '/div[@class="form-group row"]/div[@class="col-sm-2" or @class="col-sm-10"]', 2);
}
}
6 changes: 3 additions & 3 deletions src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php
View file
Expand Up @@ -28,8 +28,8 @@ public function testLabelOnForm()
$html = $this->renderLabel($view);

$this->assertMatchesXpath($html,
'/label
[@class="form-control-label required"]
'/legend
[@class="col-form-legend required"]
[.="[trans]Name[/trans]"]
'
);
Expand Down Expand Up @@ -120,7 +120,7 @@ public function testLegendOnExpandedType()

$this->assertMatchesXpath($html,
'/legend
[@class="col-form-legend form-control-label required"]
[@class="col-form-legend required"]
[.="[trans]Custom label[/trans]"]
'
);
Expand Down

0 comments on commit 4ae046f

Please sign in to comment.