The trouble is, if I want to add this class I have to override django/forms/div.html and friends, and this template contains a lot already, see django/django/forms/templates/django/forms/div.html at main · django/django · GitHub – nothing tricky, but definitely something which has to be kept up-to-date when upgrading Django in the project. Since BoundField.css_classes exists already and is documented I have opened a ticket with a proposal to add Form.field_css_class and include that CSS class in the wrapping element if it is truthy, see #35521 (Make it easy to add CSS classes to a `BoundField`) – Django
Does anyone have any concerns or opinions on this? Thanks in advance!
Django’s div.html template is artificially complicated. It’s only the way it is in order to match the exact historical output along different branches, to avoid any breaking issues when moving to the template rendering.
In my own projects I simplifying it a lot. I move the actual <div> into my field.html so as to make that easier to customise (and enhance) per-field. But if I didn’t do that it would look like:
{{ errors }}
{% for field, errors in fields %}
<div{% with classes=field.css_classes %}{% if classes %} class="{{ classes }}"{% endif %}{% endwith %}>
{{ field.as_field_group }}
</div>
{% endfor %}
{% for field in hidden_fields %}{{ field }}{% endfor %}
That’s a much simpler thing to override.
But, yes, looking at it, don’t you just want to add to css_classes there! I feel this should be settable per-field though (and maybe via that per form) — I wonder what the right API is for that?
Now that we’ve got template based rendering as well as other improvements such as the addition of as_field_group() I’ve been thinking about what else would ease form rendering. css_classes is clearly one example but there are others too. One other example is the ability to ease rendering of classes on the widget especially when they depend on state. (e.g. something like form-invalid when there are errors), another is to allow a reference to the form from the widget.
There’s different ways to solve these. Template customisation has been suggested as one, and we can of course also add additional API for each of these needs. css_classes is one proposal, here was another. Template tags (such as what’s available in django-widget-tweaks) are also an option – crispy forms also makes use of this approach.
What these API proposals generally have in common is that they are customisations to the BoundField. Django allows custom BoundFields today, but I think they are a little onerous to use. You have to override get_bound_field() for each field in your form.
Maybe we should ease customisation of BoundFields at the field/form/project level. A PR along these lines was opened recently, but has no accepted ticket.
This would allow a general approach to customisation of BoundFields to allow folk to do what they want rather than add a specific API for each use case.
Thanks! That does indeed solve my issue nicely. I forgot about as_field_group and I only now remember the discussion about keeping the HTML the same in the default case.
The default field template still contains a lot of useful stuff such as the fieldset and the ARIA logic, but copying that seems doesn’t feel as bad since breakage would probably be more obvious.
Overriding the BoundField sounds interesting since it could also open the door for e.g. changing the order of inputs and labels in the HTML in the case of checkboxes or other, more involved changes to the output without having to use a third party form rendering package.
But, yes, looking at it, don’t you just want to add to css_classes there! I feel this should be settable per-field though (and maybe via that per form) — I wonder what the right API is for that?
My proposed field_css_class (analogous to required_css_class etc.) doesn’t feel flexible enough when we’re talking about adding new API; overriding the BoundField class would allow third party code to add to the CSS class quite easily without having to copy-paste many Django inetrnals.
When I override this per-field then, it ends up looking like this:
<div data-my-customisations=true>
{% include 'django/forms/field.html#field-group; %}
</div>
… where all the work is done in the opening <div> tag, and we don’t have to rewrite the field logic each time. (A quick check once per-major Django version is sufficient to make sure we didn’t get that out of sync.)
Hi! Bumping this discussion with the same question as Christophe above. I’d love for form fields customization to be easier, it’s my worst pain point with Django currently.
For the record, I’m quite happy with the template solution Carlton lined out above.
There’s one thing:
A quick check once per-major Django version is sufficient to make sure we didn’t get that out of sync.
That’s true, but those quick checks quickly add up when you have ~100 websites using Django and you have to upgrade a few of them. Experience shows that I’m a very sloppy person when it comes to things such as upgrading and I always forget parts, especially when they are not very obvious. Checklists help somewhat but…
Overridden templates have bitten me once too much.
Also, I should really add more unit tests to my projects.
But, if I had a way to add a CSS class to each form field’s wrapping div or fieldset I wouldn’t even have to start doing that
Yep, I too am happy with the template override solution, but if I could just set attributes on the outer div there (by customising the BoundField somehow) then I wouldn’t need it either in quite a few cases.
I’d be +1 to doing something here. I assume @matthiask would too (from the comment).
@smithdc1 You too? (I’d support your plan David. We probably need a fresh ticket?)
Then I don’t know if @christophehenry is interested in working on it? If not someone else can pick it up, either with credit if based on the old PR, or not if a different approach is followed.
As far as I can see, we should be able to do this.
So, I’m not against more classes and options at all. But, the combination of just class="field" and :has() (and of course the existing error_css_class and required_css_class) would probably cover 90% of all use cases I ever had for this. The BoundField cleanup makes sense anyway (I think) but it seems like it isn’t directly related to this more basic issue.
I think it would be great if a small change like this, which only adds to an already existing case in the code, could be landed without having to do the larger thing. The perfect isn’t always the enemy of the good, but in a world of limited attention and time it probably is.
Then I don’t know if @christophehenry is interested in working on it?
Are you talking about my BF customization PR? Well I’m quite happy to make it progress. The documentation and tests are written. If everyone is OK with this solution, it’s ready for merging.
Right now it’s closed because I got fed up with the ticket tracker triage team blocking my work because nobody would respond here. But if we can achieve consensus about the solution, I’m too happy to reopen.
(Still, I think there’s a problem with the process of adding features in Django. If community consensus can’t be achieved here because of one grumpy user or even because… just nobody responds, there’s no way to push ideas forward. I think it’s a real issue)
Yes, it can take a while to get to the right consensus. That can be frustrating. Sorry if you’ve felt that. Do try not to take it out on the triage team though. They’re doing the right thing.
Zooming out these things have been how they are for a long time now. This current proposal only arises because of the form changes in the last couple of versions. So I’d expect there to be some deliberation before we make changes.
(And yes, that does need people to reply. Those people are volunteers who do what they can in the time they have.)
My only slight concern is that I’d liken it to a multipurpose tool. It’ll be useful in many cases but won’t add a specific switch for each use case so won’t be exactly what folk are looking for.
In your proposition settings.FORM_RENDERER would become a dictionnary, right?
I suggest we leave the setting being a string to the import location of your FORM_RENDERER. The BaseRenderer could learn a new attribute for a boundfield class. This can be subclassed – and the setting updated – to allow a project wide setting. It’s the same pattern being followed as for changing the default form templates.
Here’s an example I put together of what I was thinking. It builds upon your proposal with the addition of the attribute on the BaseRenderer. I hope the tests show how this could allow users to customise the BoundField per field, per form and per project.
It’s rather draft though – I’m nervous about that circular import.
I’ve been reading the tickets and opinions from this thread. I have paid special attention to the commit shown by David in the previous comment. I feel that the whole necessity to define the bound_field_class across multiple classes to pass it from the renderer all way down to the form.Field is a code smell.
My first gut feeling is that the FORM_RENDERER setting would, ideally, be added (somehow!) to the TEMPLATES settings in a way that we could also configure the customized BoundField import path class. Has anyone considered something similar to this approach?
We begin defining it per-Field, so an attribute there seems OK, no?
Then the question is, can we define it per-Form, to avoid the need to override every field? That seems a reasonable request, so we add an attribute to the Form, and have Field check that if its own attribute is not set. Again, that seems OK.
Finally then, we ask, can we set this at the project level? For that we have the form renderer which encapsulates these defaults, and again we allow the proxying up from the Form.
At each step, that seems a logical approach, and it matches the existing patterns for the template names for form and field group.
We have to do something like this to allow per-field, per-form, and per-project adjustments no?
—-
For me, the form renderer is still kind of new. I haven’t thought about moving it elsewhere, but that would be a separate adjustment no?
@boxed I think that’s a massive distraction from this topic. “How do we adjust the forms API to allow…?” is not well answered by “Use something else entirely”.
I know you’re keen to promote Iommi, but please don’t hijack unrelated thread. That’s just spammy.
