Django

Code

Ticket #4092 (new)

Opened 1 year ago

Last modified 3 months ago

django/contrib/localflavor/usa/forms.py USStateSelect missing blank options

Reported by: polpak <polpak@yahoo.com> Assigned to: dougn
Milestone: Component: django.contrib.localflavor
Version: SVN Keywords:
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description (Last modified by mtredinnick)

This field needs to have the default starting option of ('', '----------') so people don't accidentally forget to change it and have a bunch of submissions for Alabama.

I can submit a patch for this later on if needed

Chad Simmons

Attachments

4092_6194_simple.diff (20.3 kB) - added by dougn on 09/14/07 12:57:51.
Simple patch which does not include a new base widget, but adds an empty_label to each localflavor Select widget
4092_6248_nullselect.diff (139.7 kB) - added by dougn on 09/14/07 18:57:43.
New NullSelect? widget which is used as the base for all the localflavor select widgets, and replaces the ModelField? default widget,.
4093_6352_nullselect.diff (143.4 kB) - added by dougn on 09/16/07 01:00:33.
Updated patch with better NullSelect implementation which accepts empty_label on render method, and has full tests

Change History

04/19/07 13:14:42 changed by polpak <polpak@yahoo.com>

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

.... it stripped off the blank value ticktick

04/21/07 07:45:50 changed by Simon G. <dev@simon.net.nz>

  • owner changed from jacob to adrian.
  • component changed from Uncategorized to django.contrib.localflavor.
  • stage changed from Unreviewed to Accepted.

07/19/07 05:45:07 changed by mtredinnick

  • description changed.

(Fixed description formatting)

I'm not really sure what to do about this. The blank entry (if we include it), should be inserted by the widget, since it's not part of the data. And possibly only inserted if no choice is selected. We should also do the same thing for every other localflavor widget; nothing special about the US version here.

Patches welcome, certainly. Something to look at might make it easier to decide.

09/14/07 00:50:43 changed by dougn

I was looking at this and there are 20 Select widgets which would need to be changed. It feels like there should be a new base widget which deals with the empty choice (The same way there is for the ModelChoiceField.

I could just be over thinking things and making them more complex than necessary.

from itertools import chain

class NullSelect(Select):
    def __init__(self, attrs=None, choices=(), empty_label=u"---------"):
        empty_choice = ((u'', empty_label),)
        super(NullSelect, self).__init__(attrs, chain(empty_choice, choices))

With this addition, the newforms.models.ModelChoiceField would no longer need the empty_label, and would use the newforms.widgets.NullSelect widget instead.

09/14/07 00:59:38 changed by dougn

  • owner changed from nobody to dougn.

Working on a patch

09/14/07 12:34:19 changed by alagu

  • owner changed from dougn to alagu.

09/14/07 12:36:12 changed by alagu

  • owner changed from alagu to dougn.

Didn't notice that it was claimed. Sorry. Is there going to be a new base widget?

09/14/07 12:53:44 changed by dougn

I am making two different patches. One w/o the new base, and one with a new base, updated docs, and a regression test.

09/14/07 12:57:51 changed by dougn

  • attachment 4092_6194_simple.diff added.

Simple patch which does not include a new base widget, but adds an empty_label to each localflavor Select widget

09/14/07 13:03:06 changed by dougn

NOTE: holding off on setting the 'has_patch' flag until I get the second extended patch done, then will open up the discussion to the developer list.

09/14/07 18:57:43 changed by dougn

  • attachment 4092_6248_nullselect.diff added.

New NullSelect? widget which is used as the base for all the localflavor select widgets, and replaces the ModelField? default widget,.

09/14/07 19:01:18 changed by dougn

  • has_patch set to 1.

The nullselect patch includes documentation and regression tests. It is as up to date as I can make it.

The older simple patch is now out of data and does not include required test fixes. I can easily make a new simple patch if the NullSelect widget is rejected.

09/15/07 17:17:50 changed by jacob

This looks pretty good. I'm not 100% happy with the name "NullSelect?", but I'm not sure if there's a better color for that particular bikeshed.

Oh, and this needs a test for NullSelect? itself (there are tests for subclasses, but no the widget itself).

09/16/07 01:00:33 changed by dougn

  • attachment 4093_6352_nullselect.diff added.

Updated patch with better NullSelect implementation which accepts empty_label on render method, and has full tests

09/16/07 01:07:07 changed by dougn

I really don't care for NullSelect either. I thought of other names (EmptySelect, BlankSelect, etc) but NullSelect fit the meme of NullBooleanSelect and others.

I also considered adding the empty_label to the !Select widget it's self, but there is no good way of doing this without breaking most implementations out there. The nice part about NullSelect? is that it does not change any existing behavior.

One option would be to add it to !Select, defaulting to None, and providing a EMPTY_LABEL global for people to use (so that u'--------' is defined somewhere). This would be a rather verbose way of doing it though.

04/16/08 16:36:46 changed by tim.saylor@gmail.com

Without this patch I can't make an optional address in my model using USStateField. This should be committed soon.

Hmm, why is this still marked as new? It's been assigned and has a patch. Wierd.

04/16/08 20:31:18 changed by mtredinnick

Tim Saylor: if you want to help advance the ticket, feel free to fix the problems that were mentioned above. Complaining adds no value.

04/16/08 20:31:24 changed by mtredinnick

  • needs_better_patch set to 1.

04/16/08 20:33:11 changed by mtredinnick

  • needs_better_patch deleted.

04/16/08 21:29:48 changed by tim.saylor@gmail.com

What complaints? The only things complained about recently were the name NullSelect?, and Jacob said he can't think of anything better, and a lack of tests for the base NullSelect? class, which dougn appears to have fixed. I'd be glad to help, but I don't know what's needed.


Add/Change #4092 (django/contrib/localflavor/usa/forms.py USStateSelect missing blank options)




Change Properties
Action