Opened 4 years ago

Last modified 14 hours ago

#34034 assigned New feature

Adding a class on ChoiceWidget subwidgets is excessively difficult

Reported by: Claude Paroz Owned by: Wasif Shahzad
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Unless someone shows the magical way, adding a class on a subwidget of ChoiceWidget (and not on the widget itself) is too difficult.

Maybe adding option_attrs (either as class attribute or as __init__ parameter) could be a solution.

Change History (16)

comment:1 by Mark Walker, 4 years ago

Could you add an example of the difficulty?

Looking at the Select widget (​source), it defines an attribute multiple;

Code highlighting: 

    def get_context(self, name, value, attrs):
      context = super().get_context(name, value, attrs)
      if self.allow_multiple_selected:
          context["widget"]["attrs"]["multiple"] = True
      return context

So I'm assuming this then renders with <select multiple>. If that's the level you want to add a class to, context["widget"]["attrs"]["class"] = "my-select-class" should then render your classes.

Version 0, edited 4 years ago by Mark Walker (next)

comment:2 by Claude Paroz, 4 years ago

That means that you have to subclass the Django widget. Then for subwidgets, you have to override create_option, not get_context. Then you have to take care not overwriting any existing class content. So in some code of mine, I had to write:

    def create_option(self, *args, attrs=None, **kwargs):
        attrs = attrs.copy() if attrs else {}
        if 'class' in attrs:
            attrs['class'] += ' form-check-input'
        else:
            attrs.update({'class': 'form-check-input'})
        return super().create_option(*args, attrs=attrs, **kwargs)

and note that the class name is hardcoded, so if you need another class for another part of a form, you have to set the class name as a subclass attribute, and create other widget subclasses. And as that can touch RadioSelect, CheckboxSelectMultiple, etc., you'll have to create subclasses for all of them (if used, of course).

That's what I mean by "excessingly difficult". But it's doable, of course.

comment:3 by Carlton Gibson, 4 years ago

Summary: Adding a class on ChoiceWidget subwidgets is excessingly difficult → Adding a class on ChoiceWidget subwidgets is excessively difficult
Triage Stage: Unreviewed → Accepted

Hey Claude,

Maybe adding option_attrs…

So I'm looking at this in ChoiceWidget: 

    def create_option(
        self, name, value, label, selected, index, subindex=None, attrs=None
    ):
        ...
        option_attrs = (
            self.build_attrs(self.attrs, attrs) if self.option_inherits_attrs else {}
        )
        ...

Do you think pulling that into a property (that could then be overridden) would be sufficient? Maybe the else would pull from an __init__ arg. … 🤔

(Implementation TBD)

Seems a reasonable suggestion anyhow. Thanks.

comment:4 by Claude Paroz, 4 years ago

I'm not yet sure about the implementation (didn't find time until now), but basically, we should be able to handle the following use cases:

  • attrs for parent widget, not on subwidgets (already OK with option_inherits_attrs set to False)
  • same attrs for parent widget AND subwidgets (already OK with option_inherits_attrs set to True)
  • attrs for subwidgets but NOT for parent widget (<-- TODO)

Ideally I would like to be able to do that without subclassing, e.g. by Widget(attrs={...}, option_attrs={...}). I guess experiments will tell if it's doable wrt option attrs inheritance.

comment:5 by David Smith, 4 years ago

Seems a reasonable suggestion to me.

One other thought I had was about using templates. A custom template could be provided to option_template_name or "django/forms/widgets/select_option.html" could be overridden. This may help to avoid Widget(attrs={...}, option_attrs={...}) each time this type of widget is used in a project?

comment:6 by Claude Paroz, 4 years ago

The problem with templates is that it's also very difficult to add classes, as they may conflict with other classes already present in attrs. If there was a mean to complement widget/subwidget classes from attrs with custom classes, then templates could be a viable solution.

comment:7 by Kees Hink, 3 years ago

Owner: changed from nobody to Kees Hink
Status: new → assigned

comment:8 by Kees Hink, 3 years ago

Has patch: set

​PR

Last edited 3 years ago by Natalia Bidart (previous) (diff)

comment:9 by Natalia Bidart, 3 years ago

Needs tests: set
Patch needs improvement: set

comment:10 by Mariusz Felisiak, 3 years ago

Owner: Kees Hink removed
Status: assigned → new

comment:11 by Mariana, 2 years ago

Owner: set to Mariana
Status: new → assigned

comment:12 by Mariana, 2 years ago

Needs tests: unset
Patch needs improvement: unset

comment:13 by Sarah Boyce, 23 months ago

Patch needs improvement: set

comment:14 by Mariana, 19 months ago

Patch needs improvement: unset

​PR

comment:15 by Sarah Boyce, 19 months ago

Patch needs improvement: set

comment:16 by Wasif Shahzad, 14 hours ago

Owner: changed from Mariana to Wasif Shahzad

I will try to look at it in the next 3 to 4 weeks. If someone doesn't hear anything from me by then, then feel free to assign it to yourself.

Note: See TracTickets for help on using tickets.
Back to Top