Code

Opened 4 years ago

Last modified 3 years ago

#13564 assigned New feature

Provide class attributes for form fields

Reported by: h3 Owned by: trebor74hr
Component: Forms Version: 1.2-beta
Severity: Normal Keywords:
Cc: gg, 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 trebor74hr 4 years ago.
New method Form.get_css_classes - with tests and docs

Download all attachments as: .zip

Change History (19)

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to russellm
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 4 years ago by russellm

  • Owner russellm deleted
  • Status changed from assigned to new

comment:3 Changed 4 years ago by h3

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 Changed 4 years ago by Joshua Ginsberg <jag@…>

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 Changed 4 years ago by trebor74hr

  • Owner set to trebor74hr
  • Status changed from new to assigned

Changed 4 years ago by trebor74hr

New method Form.get_css_classes - with tests and docs

comment:6 Changed 4 years ago by trebor74hr

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 Changed 4 years ago by trebor74hr

  • Has patch set

comment:8 Changed 4 years ago by trebor74hr

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

comment:9 Changed 4 years ago by gg

  • Cc gg added

comment:10 follow-up: Changed 3 years ago by julien

  • milestone 1.3 deleted

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 Changed 3 years ago by trebor74hr

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).

comment:12 in reply to: ↑ 10 Changed 3 years ago by trebor74hr

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

comment:13 follow-up: Changed 3 years ago by julien

@ 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).

comment:14 in reply to: ↑ 13 Changed 3 years ago by trebor74hr

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 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:16 Changed 3 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

13564_form_get_field_css_classes.diff fails to apply cleanly on to trunk

comment:17 Changed 3 years ago by trebor74hr

  • Cc trebor74hr@… added

comment:18 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from trebor74hr to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.