-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Allow user to set input class attributes #22622
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
Conversation
When a user sets a form input attribute class, we shouldn't force-add the form-control class.
This will break stuff.. yes :) For being a bootstrap theme it makes sense to add the proper classes by definition, otherwise it wouldnt be a valid bootstrap theme. Meaning you need another extension point ;-) Perhaps refactor to something like #21751 so that classes are put in a separate block, which you can override. |
That still seems rather bizarre to me. Here's the basic issue: Say I have
I don't get an input with I get class="some classes form-control". I cannot in my form set the class. I agree that by default, the class should be added, but I disagree that it should be added to any input that has any class set, since as a dev I know what should be there. I do realize that this will break others, but it really seems very bizarre to override a form theme so that my classes are applied. |
I don't understand this. Your class will be set anyway. It's just the Merging the PR as is cannot be done as it will break every other use where people just want to add additional classes. |
I realize this can't be merged as is for many reasons. Here's the issue. If I specify a form class I have no way to not have the form control class. This may seem insignificant but having to override a form theme to simply remove a class is the wrong solution in my opinion. I can still use the bootstrap theme (css wise not symfony form theme) without having form-control on the input. For example the specific reason I found this issue is because we have a theme built on top of bootstrap. It has a bunch of input sizing classes, they need to be applied without form-control to work. I can't do that without overriding the theme. Unfortunately I think the way the default class is applied for this theme is wrong and changing it would cause a BC but really, creating a form theme is just weird to solve an issue that the form system should just do. |
You can still override Bootstrap's CSS for the |
yes - I completely and totally understand there are multiples ways around this issue. We've worked around it internally already. You can probably close this - though I still think this should be fixed in a future backwards breaking way/branch. Its nonsense to override it in the other places to fix this. I filed this bug because when I do this (regardless of the theme involved):
I think it is a bug to get anything but 'some classes' in the output. Its nice to have defaults, but it seems wrong that explicitly setting an option doesn't get exactly that option. |
IMHO it's not a bug. The theme is specific to a bootstrap framework and the framework specifies how the theme should look like. |
The theme provides the defaults. If you take bootstrap and build a site with it, you are not required to always use class=form-control on your inputs. If you have a situation for whatever reason where you want fine grained control of the class used on the input. It is logical that you would set the class on the symfony form via the attr->class entry. The fact that you cannot remove a class when you explicitly set the class is the issue. The theme has a default yes, but the bug I am reporting is that I cannot remove that without doing crazy amounts of work for overriding themes. If I have a form with 3 inputs and one of them can't have the form-control class applied I have to jump through crazy hoops to override the class I set in the form for that one field. That is a bug IMO. |
Well, there are ways to achieve your goal. Either by writing CSS that does not need the class to be absent or by creating a custom theme that extends the built-in one and overrides this widget. Introducing a new config option (which we would need to be backward compatible) IMO is to much work in core to support this edge case. |
I still think you either opt-in or opt-out from form theming; if you opt-in and use bootstrap as a theme; this means inputs needs a form-control class (from the theme pov). It's weird only one field should follow a different theme; but in that case there are solutions already (like @xabbuh mentioned). If overriding becomes a mess we could stil make themes more flexible like mentioned in #22622 (comment) |
alrighty |
2nd thing that's documented is that if you need this for one field only you can overwrite it on the spot (same template). or prepare the external template file and include it where needed. IMO you are over-complicating things. |
When a user sets a form input attribute class, we shouldn't force-add the
form-control class.
We are using a theme built on bootstrap 3. However we have cases where the input
class should be simply =col-xs-X for width control. When we set this on the form
we still get form-control and so the input is wide and ignores the other class.
It took awhile to find this where everything gets the form-control regardless of
having a class already set. It seems to me that if you are setting the class you
should set all classes required for what you are doing. This is a proof of concept
change at the moment as I didn't touch the textarea/buttons or any other widget
that uses the same syntax. Making this change locally keeps all other input fields
properly styled and allows us to override the class attribute as I would expect.
Our only other choice is to override the bootstrap_3_layout.html.twig if this
type of change is unwanted. If it is wanted, then I can make the rest of the changes.