Code

Opened 3 years ago

Closed 3 years ago

#15773 closed New feature (needsinfo)

MultiValueField could be more convenient

Reported by: Aryeh Leib Taurog <vim@…> Owned by: nobody
Component: Forms Version: 1.3
Severity: Normal Keywords:
Cc: vim@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

It seems to me that the MultiValueField could be a lot more useful. I'm assuming the SplitDateTimeField code exemplifies the normal use case, and I see two things here that bother me.

  1. To use MultiValueField, coder must create and assign a widget class. Maybe I'm just lazy because the other Field classes seem to provide reasonable defaults, but couldn't this field provide a reasonable default as well?
  1. compress must be defined on the Field class, but decompress must be defined on the Widget class. I don't know if I'd call this a violation of DRY, but it seems to me it would be much nicer to put these two together, since one is the inverse of the other. Obviously there's nothing preventing me from putting my MultiWidget subclass and MultiValueField subclass together, but if I separate the Widgets from the Fields, as the django source has done, then I introduce room for maintenance errors.

I guess this is mainly about convenience. We do like making good development practice more convenient.

My suggestion for addressing these two points is below. Thanks for reading.

from django.forms import fields
from django.forms import widgets

class MultiValueFieldWidgetDescriptor(object):
    def __get__(self, instance, owner):
        widget_base = getattr(owner, 'widget_base', widgets.MultiWidget)
        class MyWidget(widget_base):
            def __init__(self, attrs=None):
                widgets = [f.widget for f in instance.fields]
                super(MyWidget, self).__init__(widgets, attrs)

            decompress = owner.decompress.__get__(self)

        return MyWidget

class MultiValueField(fields.MultiValueField):
    widget = MultiValueFieldWidgetDescriptor()

Attachments (0)

Change History (2)

comment:1 Changed 3 years ago by jacob

  • Easy pickings unset
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I'm not sure I get it. Can you share a specific use case or a problem you have that this solves?

comment:2 Changed 3 years ago by jacob

  • Resolution set to needsinfo
  • Status changed from new to closed

Without more info I don't see what the problem here is. If you can provide the use case or the problem that prompts this patch, please feel free to reopen.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.