Skip to content

[Form] Remove extra .form-group wrapper around file widget in bootstrap 4 #28115

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 7, 2018
Merged

[Form] Remove extra .form-group wrapper around file widget in bootstrap 4 #28115

merged 1 commit into from
Aug 7, 2018

Conversation

MrMitch
Copy link

@MrMitch MrMitch commented Aug 2, 2018 •
edited
Loading

Q A
Branch? 4.1
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

This is a follow-up to #27958 and #27919 by @apfelbox .

It fixes an extra space between the help text of a FileType widget and the widget itself. The extra space was caused by a .form-group wrapper in the file_widget block.

@MrMitch
Copy link
Author

MrMitch commented Aug 2, 2018

I based this on master, let me know if it should be based on something else.

@apfelbox apfelbox mentioned this pull request Aug 2, 2018
Closed
@apfelbox
Copy link
Contributor

apfelbox commented Aug 2, 2018

You should rebase it, so that it can be merged into 4.1 – as bugfix. 👍

[Form] Remove extra .form-group wrapper around file widget in bootstr…
01e7fe4 
…ap 4

This is a follow-up to #27958 and
#27919.

It fixes an extra space between the help text of a FileType widget and the
widget itself. The extra space was cause by a .form-group wrapper in the
file_widget block.
@MrMitch
Copy link
Author

MrMitch commented Aug 2, 2018

@apfelbox rebase done, thank you. Should this PR target 4.1 instead of master, too ?

@MrMitch MrMitch changed the base branch from master to 4.1 August 2, 2018 12:26
@MrMitch
Copy link
Author

MrMitch commented Aug 2, 2018

rebased on 4.1, PR target changed to 4.1 and description updated

@MrMitch
Copy link
Author

MrMitch commented Aug 2, 2018

Not sure why appveyor build fails, this seems unrelated (it's in the SecurityBundle, which this PR doesn't change) :
image
ref: https://ci.appveyor.com/project/fabpot/symfony/build/1.0.39156#L1059

@nicolas-grekas nicolas-grekas added this to the 4.1 milestone Aug 3, 2018
@nicolas-grekas
Copy link
Member

Is this legit at the semantic level? Could this be a BC break? Can't we get rid of the whitespace by removing the blank characters between tags?

@apfelbox
Copy link
Contributor

apfelbox commented Aug 7, 2018

@nicolas-grekas see my comments in #28116 that should explain why this is a bugfix (I sent the same changes in a PR just minutes after this one).

@nicolas-grekas
Copy link
Member

Thank you @MrMitch.

@nicolas-grekas nicolas-grekas merged commit 01e7fe4 into symfony:4.1 Aug 7, 2018
nicolas-grekas added a commit that referenced this pull request Aug 7, 2018
@nicolas-grekas
bug #28115 [Form] Remove extra .form-group wrapper around file widget…
3e47827 
… in bootstrap 4 (MrMitch)

This PR was merged into the 4.1 branch.

Discussion
----------

[Form] Remove extra .form-group wrapper around file widget in bootstrap 4

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        |  n/a

This is a follow-up to #27958 and #27919 by @apfelbox .

It fixes an extra space between the help text of a FileType widget and the widget itself. The extra space was caused by a `.form-group` wrapper in the `file_widget` block.

Commits
-------

01e7fe4 [Form] Remove extra .form-group wrapper around file widget in bootstrap 4
@fabpot fabpot mentioned this pull request Aug 28, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@xabbuh xabbuh xabbuh approved these changes

@vudaltsov vudaltsov vudaltsov approved these changes

Assignees
No one assigned
Labels
Bug Form Status: Reviewed
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
6 participants
@MrMitch @apfelbox @nicolas-grekas @xabbuh @vudaltsov @carsonbot