Bootstrap4 support for Twig form theme #21751

hiddewie · 2017-02-24T16:08:27Z

This PR is a followup from #19648. That PR was closed because GitHub thought my branch contained no commits after a force push...

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

I have made a port of the Twig form theming code for Bootstrap 3 to the α-5 version of Bootstrap 4.

The (inheritance) structure of the form theming files has changed because a number of blocks are the same between BS 3 and 4. They have been migrated to bootstrap_base_layout.html.twig.
The new tree is as follows:

bootstrap_base_layout.html.twig
|-- bootstrap_3_layout.html.twig
|   `-- bootstrap_3_horizontal_layout.html.twig
`-- bootstrap_4_layout.html.twig
    `-- bootstrap_4_horizontal_layout.html.twig

Any occurances of .form-horizontal have been removed from the BS 4 code.
Checkboxes and radio buttons have been stacked using the .form-check, .form-check-label and .form-check-input classes. There is now no distinction between checkboxes and radio buttons in the markdown.
All layout tests have been added and updated for BS4. The inheritance tree is as follows:

AbstractLayoutTest
`-- AbstractBootstrap3LayoutTest
    |-- AbstractBootstrap3HorizontalLayoutTest
    `-- AbstractBootstrap4LayoutTest
        `-- AbstractBootstrap4HorizontalLayoutTest

All tests pass. The classes FormExtensionBootstrap4LayoutTest and FormExtensionBootstrap4HorizontalLayoutTest have been created similarly to the BS 3 versions.

~~The label coloring on an validation is not correct. I've made an issue (Form validation labels not coloring twbs/bootstrap#20535) of the problem.~~
No custom form elements have been used.
A docs PR can be created if this PR is accepted.
The new code might have to be updated if large changes occur in the BS 4 α.

Screenshot of BS3 and 4 comparison for the same form:

stof · 2017-02-24T16:11:44Z

Looks like you removed any commits once again...

stof · 2017-02-24T16:12:08Z

btw, you could push to the same branch name again and reopen the existing PR instead of creating a new one

hiddewie · 2017-02-24T22:45:03Z

I still have no idea why GitHub did not show my initial patch and did not allow me to reopen the previous PR.
In the previous PR I am using the same branch, for which the GitHub diff shows (and showed) the single commit which was my PR patch.
I will keep this one open, as the previous PR still shows 0 commits and no diff.

fabpot · 2017-03-05T20:13:52Z

Anyone willing to review this one? @symfony/deciders

fabwu · 2017-08-10T11:57:57Z

@hiddewie Is it possible to use custom checkboxes with this PR?

hiddewie · 2017-08-10T13:25:48Z

At the moment no, this should be a choice for the end user designing their web application using Bootstrap. However, I would be happy to create a follow-up PR if this has been accepted containing one extra template using the Bootstrap 4 form template, but with custom Bootstrap checkboxes/radio buttons.

fabwu · 2017-08-10T13:43:40Z

@hiddewie Ok thanks for your quick reply. Hope this gets merged soon!

hiddewie · 2017-08-10T13:45:20Z

I hope so too. The PR gets rebased every month or so, but originally it was submitted for 3.2.

javiereguiluz · 2017-08-11T07:17:55Z

Bootstrap 4 has just published its first beta version: https://github.com/twbs/bootstrap/releases/tag/v4.0.0-beta Could we please review if they made any change that affects to this pull request? Thanks!

hiddewie · 2017-08-14T20:22:11Z

Several small things have been updated for the beta version of Bootstrap 4:

The div layout had to be updated slightly for radio buttons and checkboxes
The form validation has been updated with the new classes. See Non-custom checkboxes don't change with .is-invalid twbs/bootstrap#23426.

I do not understand the single failing test in Travis CI, as the class seems to be defined correctly.

robfrawley · 2017-08-15T05:13:40Z

I do not understand the single failing test in Travis CI, as the class seems to be defined correctly.

@hiddewie That is the deps=low test; it is possible an older version of the twig-bridge is being pulled in. Possibly, you need to up the required dependency?

hiddewie · 2017-08-15T08:56:04Z

@robfrawley The deps=low build config confuses me. In this PR, I added content in the Twig Bridge and in the Form component (the base/abstract classes for the Bootstrap tests). However, it seems to download a stable version from Composer of the Form component, which of course does not contain my test classes, and causes the error for the Twig Bridge tests.

I don't see how upping the dependency would solve this problem, as it needs my updated PR code, both for the Twig Bridge and the Form component. How to solve this?

robfrawley · 2017-08-15T09:10:31Z

The dependencies do matter; I'm not 100% sure how this is handled internally but see #23451 (comment) where I had to deal with a similar issue, as well.

hiddewie · 2017-08-15T09:14:02Z

It'll be green when the PR will be merged.

Ah, so I guess that the test with the missing abstract class will have a green light after the PR is merged, because the downloaded dependency of symfony/form is version 3.4.x-dev, the same version for which this PR is proposed.

xabbuh · 2017-08-22T17:19:16Z

@hiddewie You need to rebase your PR against the 3.4 branch and then change the target path of the PR on GitHub too.

hiddewie · 2017-08-23T15:36:46Z

@xabbuh Done. The single failing test with deps=low is again the one which requires my new abstract class in the Forms bundle.

xabbuh · 2017-08-24T08:19:10Z

You need to update the version constraint for symfony/form in the require-dev section of the TwigBridge as well as the conflict rule like this:

"require-dev": {
    "symfony/form": "~3.4|~4.0"
},
"conflict": {
    "symfony/form": "<3.4"
}

hiddewie · 2017-08-24T09:15:09Z

@xabbuh Yes, thank you. Now the deps=high is failing, again because the abstract class is missing. Probably because the branch targets 3.4 and the 4.0 version of symfony/form is being installed.

MrMitch

The style for file inputs seems to be missing.

MrMitch · 2017-09-01T10:39:04Z

src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_base_layout.html.twig

+        {%- set attr = attr|merge({class: (attr.class|default('') ~ ' form-control')|trim}) -%}
+    {% endif %}
+    {{- parent() -}}
+{%- endblock form_widget_simple %}


For file inputs, boostrap recommends switching the class from .form-control to .form-control-file and this seems to be missing from this theme.
You can include this in by appending '-file' to the default 'form-control' class if the widget type is set to 'file' and slightly changing the type detection condition :

{% block form_widget_simple -%} {% if type is not defined or type != 'hidden' %} {%- set attr = attr|merge({class: (attr.class|default('') ~ ' form-control' ~ (type|default('') == 'file' ? '-file' : ''))|trim}) -%} {% endif %} {{- parent() -}} {%- endblock form_widget_simple %}

Thank you for the comment.

I've split that widget into a BS 3 and BS 4 variant, and used your suggestion. The layout test for the file input has also been updated.

…theme Fixed Bootstrap 4 error output Updated form errors to look correctly Cranked Twig Bridge Composer version to ~3.2 Added @ author PHPdoc tag to BS 4 test classes Fixed small items, and added fieldset/legend support - Fixed form class for File type - Added fieldset element for expanded form types - Added legend element as label for expanded form types - Fixed horizontal form classes for labels Added test for legend form label Fixed form label coloring on validation errors Fixed concrete Bootstrap layout test classes to use new code Fixed tests for form-control-label class on form labels Fixed a typo in using old Bootstrap 4 concrete test code Processed proposed changes from stof Processed proposed changes in bootstrap base layout from stof Updated margin-top for error list in the alpha-5 version of BS 4 Fixed {% endblock ... %} and fixed BS error class in test Fixed TwigRenderer => FormRenderer code change Minor fixed for radio/checkboxes and fixed validation feedback Added ~3.4 require of symfony/form (and <3.4 conflict) to Twig Bridge composer.json Updated Boostrap 4 file input to have class .form-control-file

javiereguiluz

@hiddewie you did a great work here. Thank you!

Considering that Bootstrap 4 is in beta since August 2017, this is now safe to be merged.

Nyholm · 2017-09-26T12:21:14Z

I've just had a quick look at the screenshots. I have a few things regarding accessibility and errors. But I guess that is no blocker for getting this merged.

silverbackdan · 2017-09-26T12:45:42Z

Just as a note on the custom options - I have an implementation when BS4 was in alpha6 which also required some form type extensions to give the options as to whether a user wants custom inputs or not.

Extensions are here: https://github.com/silverbackis/SilverbackPublishing/tree/master/src/SyliusUiBootstrapBundle/Form/Extension

This was my layout:
https://github.com/silverbackis/SilverbackPublishing/blob/master/src/SyliusUiBootstrapBundle/Resources/views/Form/bootstrap_4_layout.html.twig

I think I left bits out and this PR is far superior as a template, but I thought I may comment this to show how the custom options may be implemented in a follow-up PR after this is merged.

@hiddewie were you thinking to implement the custom option is a way where it could be configured with options?

fabpot · 2017-10-01T17:10:31Z

Thank you @hiddewie.

…ereguiluz) This PR was merged into the 3.4 branch. Discussion ---------- Bootstrap4 support for Twig form theme **This PR is a followup from #19648. That PR was closed because GitHub thought my branch contained no commits after a force push...** | Q | A | | --- | --- | | Branch? | master | | Bug fix? | no | | New feature? | yes | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | #16289 | | License | MIT | | Doc PR | - | I have made a port of the Twig form theming code for Bootstrap 3 to the α-5 version of Bootstrap 4. - The (inheritance) structure of the form theming files has changed because a number of blocks are the same between BS 3 and 4. They have been migrated to `bootstrap_base_layout.html.twig`. The new tree is as follows: ``` bootstrap_base_layout.html.twig |-- bootstrap_3_layout.html.twig | `-- bootstrap_3_horizontal_layout.html.twig `-- bootstrap_4_layout.html.twig `-- bootstrap_4_horizontal_layout.html.twig ``` - Any occurances of `.form-horizontal` have been removed from the BS 4 code. - Checkboxes and radio buttons have been stacked using the `.form-check`, `.form-check-label` and `.form-check-input` classes. There is now no distinction between checkboxes and radio buttons in the markdown. - All layout tests have been added and updated for BS4. The inheritance tree is as follows: ``` AbstractLayoutTest `-- AbstractBootstrap3LayoutTest |-- AbstractBootstrap3HorizontalLayoutTest `-- AbstractBootstrap4LayoutTest `-- AbstractBootstrap4HorizontalLayoutTest ``` All tests pass. The classes `FormExtensionBootstrap4LayoutTest` and `FormExtensionBootstrap4HorizontalLayoutTest` have been created similarly to the BS 3 versions. - ~~The label coloring on an validation is not correct. I've made an issue (twbs/bootstrap#20535) of the problem.~~ - No [custom form elements](http://v4-alpha.getbootstrap.com/components/forms/#custom-forms) have been used. - A docs PR can be created if this PR is accepted. - The new code might have to be updated if large changes occur in the BS 4 α. Screenshot of BS3 and 4 comparison for the same form: ![1](https://cloud.githubusercontent.com/assets/1073881/17732594/dfcb50d6-6472-11e6-8e96-c46987809322.PNG) Commits ------- f12e588 Removed unneeded wrapping quotes around a Twig key 709f134 Removed unneeded wrapping quotes around a Twig key 4222d54 Initial commit for Bootstrap 4 form theme, based on Bootstrap 3 form theme

hiddewie · 2017-10-09T09:46:29Z

Thank you @fabpot!

carsonbot added Status: Needs Review Feature labels Feb 24, 2017

hiddewie closed this Feb 24, 2017

hiddewie reopened this Feb 24, 2017

nicolas-grekas added this to the 3.x milestone Feb 25, 2017

javiereguiluz self-requested a review March 6, 2017 07:16

ro0NL mentioned this pull request May 3, 2017

Allow user to set input class attributes #22622

Closed

Guite mentioned this pull request Aug 11, 2017

Upgrade to Bootstrap 4.0 zikula/core#3530

Closed

hiddewie mentioned this pull request Aug 14, 2017

Non-custom checkboxes don't change with .is-invalid twbs/bootstrap#23426

Closed

hiddewie mentioned this pull request Aug 21, 2017

[Form] Add support for Bootstrap 4 forms #16289

Closed

hiddewie changed the base branch from master to 3.4 August 23, 2017 15:26

MrMitch reviewed Sep 1, 2017

View reviewed changes

hiddewie and others added 3 commits September 1, 2017 13:34

Removed unneeded wrapping quotes around a Twig key

709f134

Removed unneeded wrapping quotes around a Twig key

f12e588

javiereguiluz approved these changes Sep 25, 2017

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 1, 2017

fabpot approved these changes Oct 1, 2017

View reviewed changes

fabpot merged commit f12e588 into symfony:3.4 Oct 1, 2017

javiereguiluz mentioned this pull request Oct 3, 2017

[Form] Added form themes for Bootstrap 4 symfony/symfony-docs#8444

Closed

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

antalaron mentioned this pull request Oct 26, 2017

[TwigBridge] Bootstrap 3 horizontal layout not renders correctly the checkbox #24702

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bootstrap4 support for Twig form theme #21751

Bootstrap4 support for Twig form theme #21751

hiddewie commented Feb 24, 2017

stof commented Feb 24, 2017

stof commented Feb 24, 2017

hiddewie commented Feb 24, 2017

fabpot commented Mar 5, 2017

fabwu commented Aug 10, 2017

hiddewie commented Aug 10, 2017

fabwu commented Aug 10, 2017

hiddewie commented Aug 10, 2017

javiereguiluz commented Aug 11, 2017

hiddewie commented Aug 14, 2017 •

edited

Loading

robfrawley commented Aug 15, 2017

hiddewie commented Aug 15, 2017

robfrawley commented Aug 15, 2017

hiddewie commented Aug 15, 2017

xabbuh commented Aug 22, 2017

hiddewie commented Aug 23, 2017

xabbuh commented Aug 24, 2017

hiddewie commented Aug 24, 2017

MrMitch left a comment

MrMitch Sep 1, 2017

hiddewie Sep 1, 2017

javiereguiluz left a comment

Nyholm commented Sep 26, 2017

silverbackdan commented Sep 26, 2017

fabpot commented Oct 1, 2017

hiddewie commented Oct 9, 2017

Bootstrap4 support for Twig form theme #21751

Bootstrap4 support for Twig form theme #21751

Conversation

hiddewie commented Feb 24, 2017

stof commented Feb 24, 2017

stof commented Feb 24, 2017

hiddewie commented Feb 24, 2017

fabpot commented Mar 5, 2017

fabwu commented Aug 10, 2017

hiddewie commented Aug 10, 2017

fabwu commented Aug 10, 2017

hiddewie commented Aug 10, 2017

javiereguiluz commented Aug 11, 2017

hiddewie commented Aug 14, 2017 • edited Loading

robfrawley commented Aug 15, 2017

hiddewie commented Aug 15, 2017

robfrawley commented Aug 15, 2017

hiddewie commented Aug 15, 2017

xabbuh commented Aug 22, 2017

hiddewie commented Aug 23, 2017

xabbuh commented Aug 24, 2017

hiddewie commented Aug 24, 2017

MrMitch left a comment

Choose a reason for hiding this comment

MrMitch Sep 1, 2017

Choose a reason for hiding this comment

hiddewie Sep 1, 2017

Choose a reason for hiding this comment

javiereguiluz left a comment

Choose a reason for hiding this comment

Nyholm commented Sep 26, 2017

silverbackdan commented Sep 26, 2017

fabpot commented Oct 1, 2017

hiddewie commented Oct 9, 2017

hiddewie commented Aug 14, 2017 •

edited

Loading