Opened 14 years ago

Last modified 4 years ago

#13564 new New feature

Provide class attributes for form fields

Reported by: h3 Owned by:
Component: Forms Version: 1.2-beta
Severity: Normal Keywords:
Cc: Gabriel Grant, trebor74hr@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I work a lot with Django forms and I'm also working on grappelli and I've come to find that there's one thing that is cruelly missing from django's forms..

Meaningful class names on form inputs.

As now if I want to change the class name of an input I either have to write the forms explicitly or do something like this for *every* forms:

from django import forms
from django.forms import widgets

class MyForm(forms.Form):
    title = forms.CharField(required=True, widget=widgets.TextInput(attrs={
        'class': 'charfield required'
    }))

Whereas it would be easy to simply use the field type's name to generate a meaningful class;

<input type="text" value="" class="django-charfield django-required" />

The "required" is not crucial, but would be really useful for JavaScript enhanced validation.
That said, just a class hinting the field type would be a 100% improvement.

Finally, it's not just for JavaScript. It would also be useful for CSS styling. For example when
I use a PositiveInteger field I never need it to be the same width as a Char field. It would
be nice to be able to style them without having to write many lines of Python or HTML codes.

Attachments (1)

13564_form_get_field_css_classes.diff (5.6 KB ) - added by Robert Lujo 14 years ago.
New method Form.get_css_classes - with tests and docs

Download all attachments as: .zip

Change History (20)

comment:1 by Russell Keith-Magee, 14 years ago

Owner: changed from nobody to Russell Keith-Magee
Status: newassigned
Triage Stage: UnreviewedAccepted

Agreed on the use case.

My only concern is the extent to which this masks the bigger issue of providing an easy way for end-users to customize the exact class names that are used at time of rendering. However, I suspect that is a much bigger discussion.

comment:2 by Russell Keith-Magee, 14 years ago

Owner: Russell Keith-Magee removed
Status: assignednew

comment:3 by h3, 14 years ago

My only concern is the extent to which this masks the bigger issue of providing an easy way for end-users to customize the exact class names that are used at time of rendering.

I think this is another issue which is not really an issue. There must be an easier way to do it, but this is not my point.

If the form fields would be more explicit, for the vast majority of use cases, this need wouldn't exist altogether. And this step is not that hard to take, it only
requires to add the field type of the rendered field to its class name.

It's backward compatible, it doesn't brake anything and it solve most needs for custom class names.

comment:4 by Joshua Ginsberg <jag@…>, 14 years ago

So in the case where a widget was explicitly specified and its class attribute was explicitly specified, would that override the auto class names? Supplement them?

comment:5 by Robert Lujo, 14 years ago

Owner: set to Robert Lujo
Status: newassigned

by Robert Lujo, 14 years ago

New method Form.get_css_classes - with tests and docs

comment:6 by Robert Lujo, 14 years ago

In patch uploaded you can find new Form method *get_field_css_classes* that
can be overriden and you can specify any css class(es) you want based on
bound_field object (and current css classes). Added doc-tests and updated
documentation.

    def get_field_css_classes(self, bf, css_classes):
        """
        Returns the bound field's css classes as set() of strings. Called for
        each bounded field (parameter *bf*). Parameter *css_classes* holds
        current field's set of css classes. 

        Subclasses may wish to override.

        See also form class attributes *error_css_class* and
        *required_css_class*.
        """
        return css_classes

comment:7 by Robert Lujo, 14 years ago

Has patch: set

comment:8 by Robert Lujo, 14 years ago

Related ticket #14710 Form css classes for bound fields are not rendered {{ form.field_name }}

comment:9 by Gabriel Grant, 14 years ago

Cc: Gabriel Grant added

comment:10 by Julien Phalip, 14 years ago

milestone: 1.3

OK, it looks like this ticket has diverged into 2 different ones: (1) Automatically add a CSS class to a form field based on the field's type; and (2) Have a simple way of overriding or adding CSS classes to a form field.

(1) seems alright and useful, but only if the class name is made very specific to avoid backwards incompatibility (something along the lines of "django-charfield" seems reasonable but I'd like to see something even more specific to avoid any clashes).

(2) looks really useful as I've myself needed this is many projects before. It looks like a different (though related) issue as the one reported in the initial ticket description, so I'd encourage one to create a separate ticket for it.

Also, we're very close to releasing 1.3 and therefore this new feature would not be considered in this release, so I'm removing this ticket from the 1.3 milestone.

comment:11 by Robert Lujo, 14 years ago

I agree with you, but one can notice that the patch I made solves only (2) and solution for (1) is given as an example in unit-test.
In django documentation for this new feature one can put that example:

def get_field_css_classes(self, bf, css_classes): 
    if "required" not in css_classes: 
        css_classes.add("not_required") 
    if "error" not in css_classes: 
        css_classes.add("not_error") 
    css_classes.add("django-%s" % bf.field.__class__.__name__.lower()) 
    return css_classes 

Thus, I would suggest not to solve (1) at all, but only (2) (whether through this ticket or new one).

in reply to:  10 comment:12 by Robert Lujo, 14 years ago

Replying to julien:
I just wrote a comment and suggestion but didn't used trac's "reply".

comment:13 by Julien Phalip, 14 years ago

@ trebor74hr: Feel free to create a new ticket for (2) and mention #13564 as a related ticket. This ticket suggests that a CSS class reflecting the form field type is added *automatically*, so that you don't have to do it manually for each of your forms. So (1) and (2) are essentially related but different, and therefore could both be addressed separately (i.e. (1) only may eventually be fixed, or (2) only, or both, or none).

in reply to:  13 comment:14 by Robert Lujo, 14 years ago

Replying to julien:

Feel free to create a new ticket for (2) and mention #13564 as a related ticket.

I'm still not convinced that (2) should be created as ticket at all. Automatically filling css properties of fields/widgets is backward incompatible change, and therefore could cause problems. On the other hand, I use django.forms a lot, and in the cases like (2) is, I create my own base Form class (by inheriting django.forms.Form) and put all common functionality in there. Then I inherit this Form. Example for (2):

from django import forms

class Form(forms.Form):
    def get_field_css_classes(self, bf, css_classes): 
        css_classes.add("django-%s" % bf.field.__class__.__name__.lower()) 
        return css_classes 

class UserRegistrationForm(Form):
    ...

My suggestion is to solve only (1) and (2) could serve as example in documentation like explained.

btw. I don't know if regular django user takes advantages of creating own customized Forms/Widgets/Fields + inheritance, but I have a feeling that this is not very often the case. If this is true, then I suggest stronger promotion of this powerful django feature. For the start, such approach could be (more) emphasized in the official django documentation (with examples).

comment:15 by Julien Phalip, 14 years ago

Severity: Normal
Type: New feature

comment:16 by patchhammer, 14 years ago

Easy pickings: unset
Patch needs improvement: set

13564_form_get_field_css_classes.diff fails to apply cleanly on to trunk

comment:17 by Robert Lujo, 13 years ago

Cc: trebor74hr@… added

comment:18 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:19 by Mariusz Felisiak, 4 years ago

Owner: Robert Lujo removed
Status: assignednew
Note: See TracTickets for help on using tickets.
Back to Top