[TwigBridge] Re-add Bootstrap 3 Checkbox Layout #24711
Conversation
arkste
commented
Oct 27, 2017
| Q | A |
|---|---|
| Branch? | 3.4 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #24702 |
| License | MIT |
| Doc PR |
|
AppVeyor error seems unrelated |
|
Thank you @arkste. |
This PR was merged into the 3.4 branch. Discussion ---------- [TwigBridge] Re-add Bootstrap 3 Checkbox Layout | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24702 | License | MIT | Doc PR | Commits ------- 8ac8f9d [TwigBridge] Re-add Bootstrap 3 Checkbox Layout
| @@ -123,10 +123,15 @@ | |||
| {%- endblock datetime_row %} | |||
|
|
|||
| {% block checkbox_row -%} | |||
| {% spaceless %} | |||
There was a problem hiding this comment.
Is spaceless here really needed? It makes the block much slower and we removed them everywhere when not needed. I can see some more usages, but we should consider removing them if not strictly needed.
There was a problem hiding this comment.
you're right, it's not needed, but spaceless was there when the template was added back in 2014 and got copy-pasted over the time for other row-blocks.
to make sure this gets merged asap and to avoid possible BC breaks, i only re-added the old code with spaceless.
but... after reviewing the code myself today, i noticed that i made a mistake:
the code should've been re-added to the bootstrap_3_horizontal_layout.html.twig (where it was removed) instead of replacing the checkbox_row-block in the the bootstrap_3_layout.html.twig (it causes an unnecessary empty container in the vertical layout now).
i'll prepare a new PR where i'll fix this when #24727 gets merged to avoid conflicts.
…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