8000 bootstrap 3 layout adds "form-control" on hidden inputs / csrf token inputs · Issue #16925 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

bootstrap 3 layout adds "form-control" on hidden inputs / csrf token inputs #16925

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
Closed
michaelPf85 opened this issue Dec 9, 2015 · 7 comments
Closed

bootstrap 3 layout adds "form-control" on hidden inputs / csrf token inputs #16925

michaelPf85 opened this issue Dec 9, 2015 · 7 comments
Labels
Enhancement Form

Comments

@michaelPf85
Copy link

Hello,

I've been using Bootlint recently to fix some mistakes on an application we're building. Most errors were on our side and we were able to fix, but one seems to come from symfony : Bootlint complains that our "form__token" hidden inputs use "form-control" class that is only available for text inputs.

I think this is the CSRF token input that SF automatically adds, I might be wrong on some point or term.

I located in bootstrap_3_layout.html.twig the block form_widget_simple that adds the "form-control" class on everything but "file" inputs. I modified it from or file != type to or ('file' != type and 'hidden' != type) and it works well. But I'm afraid this could miss some other errors, but other widgets seem to cover about everything (date, datetime, money...), or worse it could not be wanted :)

Would this be alright ? I suppose I might try to write some pull request if it's OK (this would be my first).

We're using Symfony 2.6.12 but I checked the master branch and this part was not changed.

Thanks for your feedback.
@xabbuh xabbuh added the Form label Dec 10, 2015
@xabbuh
Copy link
Member
xabbuh commented Dec 10, 2015

@michaelPf85 It would be great if you could point us to some Bootstrap documentation which explain when to use and when not to use the form-control class.

@michaelPf85
Copy link
Author

That's a challenge... Precise Bootstrap documentation !

Up to now I've found those :

It never exactly tells in Bootstrap that form-controlshould not be used outside of the cases listed in the code documentation, but the Bootlink rule does say so. As such it's not completely satisfying but IMHO it looks like sufficient doc.

@javiereguiluz
Copy link
Member

@michaelPf85 thanks for investigating the origin of this recommendation. @xabbuh given these proofs, I'd say that the change is legitimate.

@xabbuh
Copy link
Member
xabbuh commented Dec 10, 2015

Seems so. Are there other types we should exclude too?

@michaelPf85
Copy link
Author

Here is the complete list of input types where the form-control class applies (from bootstrap source code).

Date and datetime seem to be covered by oher blocks, but I do not know what form_widget_simple covers exactly so I can't filter the list for you unfortunately.

  • input[type="text"]
  • input[type="password"]
  • input[type="datetime"]
  • input[type="datetime-local"]
  • input[type="date"]
  • input[type="month"]
  • input[type="time"]
  • input[type="week"]
  • input[type="number"]
  • input[type="email"]
  • input[type="url"]
  • input[type="search"]
  • input[type="tel"]
  • input[type="color"]

@satanasdiabolo
Copy link

It looks like form_widget_simple covers the following form type:

  • TextType
  • EmailType
  • IntegerType
  • MoneyType
  • NumberType
  • PasswordType
  • PercentType
  • SearchType
  • UrlType
  • RangeType
  • DateType with option widget set to single_text
  • DateTimeType with option widget set to single_text
  • TimeType with option widget set to single_text
  • BirthdayType with option widget set to single_text
  • FileType

The form-control CSS class is also applied to choice_widget_collapsed

@javiereguiluz javiereguiluz mentioned this issue Jan 27, 2016
Closed
@javiereguiluz
Copy link
Member

@michaelPf85 thanks for the information provided to solve this issue and thanks to @satanasdiabolo too.

I've created a new pull request to solve this issue (#17568). @michaelPf85 you mentioned in your first comment that this would be your first pull request. If you want to try to submit a pull request next time, you can read this guide and you can ask us for any help that you might need.

fabpot added a commit that referenced this issue Mar 1, 2016
@fabpot
bug #17568 Improved Bootstrap form theme for hidden fields (javieregu…
df4cc40 
…iluz)

This PR was squashed before being merged into the 2.7 branch (closes #17568).

Discussion
----------

Improved Bootstrap form theme for hidden fields

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

Commits
-------

ba5d7f9 Improved Bootstrap form theme for hidden fields
@fabpot fabpot closed this as completed Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement Form
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @javiereguiluz @michaelPf85 @satanasdiabolo @xabbuh
0