[WIP] [Form] Added support for Bootstrap 4 forms #16290
Conversation
EmanueleMinotto
commented
Oct 19, 2015
EmanueleMinotto
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #16289 |
| License | MIT |
| Doc PR | - |
|
You should probably create new |
| @@ -1,7 +1,7 @@ | |||
| {% use "bootstrap_3_layout.html.twig" %} | |||
|
|
|||
| {% block form_start -%} | |||
| {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-horizontal')|trim}) %} | |||
| {% set attr = attr|merge({class: attr.class|default('')|trim}) %} | |||
There was a problem hiding this comment.
This doesn't do anything.
|
@EmanueleMinotto maybe you should add the |
EmanueleMinotto
changed the title
| {# Labels #} | ||
|
|
||
| {% block form_label -%} | ||
| {% spaceless %} |
There was a problem hiding this comment.
We don't use spaceless anymore in form themes. this has a perf impact as it performs a regex replacement on the rendered string. You should use whitespace control to remove the unnecessary spaces during the template compilation instead
|
@jvasseur updated, thanks for all the feedbacks |
| {% if form.parent %}<span class="help-block">{% else %}<div class="alert alert-danger">{% endif %} | ||
| <ul class="list-unstyled"> | ||
| {%- for error in errors -%} | ||
| <li><span class="glyphicon glyphicon-exclamation-sign"></span> {{ error.message }}</li> |
There was a problem hiding this comment.
Glyphicon in not included anymore with bootstrap 4. Not sure what we should do here, maybe just removing it ?
There was a problem hiding this comment.
Nice catch! I agree that we should remove this.
|
@EmanueleMinotto what's the status of this PR? |
| {%- endblock datetime_row %} | ||
|
|
||
| {% block checkbox_row -%} | ||
| <div class="form-group{% if not valid %} has-error{% endif %}"> |
There was a problem hiding this comment.
Bootstrap4 has no has-error style. It uses has-danger instead
|
@fabpot I think it's incomplete yet. I've glanced over the code (starting to work with Bootstrap 4 also) and found some places, derived from V3, that should be changed. |
|
@fabpot I was following twbs/bootstrap#17021 waiting the v4 release (as @javiereguiluz told me above):
but I can also complete it without problems in any moment, could it be merged even if v4 isn't at the release? @FractalizeR thank you for your feedbacks, yes I started from the v3 template for the v4, but I'll fix the new templates following your feedbacks asap |
|
@EmanueleMinotto Thanks a lot. I will try to have another look over the weekend. I've started to use Bootstrap 4 for my home project and is also |
|
@fabpot Bootstrap 4 now removed icon set (Glyphicons) from main distribution. Should we consider ourselves completely empty of icons or should we use something like Font-Awesome? May be, by providing another alternative template, which will add icons? |
|
Why/where would icons be used in the form system? |
|
@mahono for alert signs in form errors |
|
Any news on this one? I'd love to have proper support for version 4 of Bootstrap |
|
@fabpot we'll need to wait a bit because Bootstrap 4 is still releasing alpha versions and this could change before the final release (unlikely but possible). |
|
Well then then the fixes can be added later when B4 gets released. If this works with latest alpha, then IMO it should get merged |
|
I agree, get this one finished, merged into master, then we have 6 months to tweak the code depending on what changes upstream. |
|
When (roughly) can we expect the release of Bootstrap 4? |
|
No ETA. Their usual moto was when it's done |
|
Closing it because we have other PR for this: #19648 I know that this one is older ... but the other one seems to have gained more attention and it's fully reviewed. I'm sorry and thanks for understanding it. |
