Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[TwigBridge] Re-add Bootstrap 3 Checkbox Layout#24711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
arkste commentedOct 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 |
Simperfit commentedOct 28, 2017
AppVeyor error seems unrelated |
nicolas-grekas commentedOct 28, 2017
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
| {%-endblockdatetime_row %} | ||
| {%blockcheckbox_row -%} | ||
| {%spaceless %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Isspaceless 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
you're right, it's not needed, butspaceless 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 withspaceless.
but... after reviewing the code myself today, i noticed that i made a mistake:
the code should've been re-added to thebootstrap_3_horizontal_layout.html.twig (where it was removed) instead of replacing thecheckbox_row-block in the thebootstrap_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-grekasCommits-------f84749f [Bridge\Twig] fix bootstrap checkbox_row to render properly & remove spaceless