Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[TwigBridge] Bootstrap 4 form theme fixes #24703

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

vudaltsov
Copy link
Contributor

@vudaltsov vudaltsov commented Oct 27, 2017

Q A
Branch? 3.4 and higher
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Some fixes for compound forms and their labels.

  1. Do not add .form-control-label and .col-form-legend at the same time. It's enough to have only one of them.
  2. Use legends instead of labels for compound fields. I think it makes more sense to have nested fieldsets than labels without for in a compound form. An example of nested fieldsets is given at the bottom of this page.

Do not add .form-control-label and .col-form-legend at the same time. It's enough to have only one of them.
I think it makes more sense to have nested `fieldset`s than `label`s without `for`.

An example of nested fieldsets is given at the bottom of [this page](https://dev.w3.org/html5/spec-preview/the-fieldset-element.html).
@vudaltsov
Copy link
Contributor Author

Btw, can we safely remove the compound is defined check?

@fabpot
Copy link
Member

fabpot commented Oct 29, 2017

Thank you @vudaltsov.

@fabpot fabpot closed this Oct 29, 2017
fabpot added a commit that referenced this pull request Oct 29, 2017
This PR was squashed before being merged into the 3.4 branch (closes #24703).

Discussion
----------

[TwigBridge] Bootstrap 4 form theme fixes

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

Some fixes for compound forms and their labels.

1. Do not add `.form-control-label` and `.col-form-legend` at the same time. It's enough to have only one of them.
1. Use legends instead of labels for compound fields. I think it makes more sense to have nested `fieldsets` than `labels` without `for` in a compound form. An example of nested fieldsets is given at the bottom of [this page](https://dev.w3.org/html5/spec-preview/the-fieldset-element.html).

Commits
-------

e55c67a [TwigBridge] Bootstrap 4 form theme fixes
@vudaltsov vudaltsov deleted the bs4-fix branch October 29, 2017 21:03
@nicolas-grekas
Copy link
Member

@vudaltsov could you please check tests? They are failing in relation to this PR I suppose.

@arkste
Copy link
Contributor

arkste commented Oct 30, 2017

i fixed them in #24728

@vudaltsov
Copy link
Contributor Author

@arkste , thanx a lot.

This was referenced Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.