Opened 2 years ago

Closed 2 years ago

#20649 closed New feature (fixed)

Add an easier way to override the blank option for Fields with choices

Reported by: brillgen Owned by: alexcouper
Component: Forms Version: master
Severity: Normal Keywords:
Cc: timograham@…, info@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

CHAR_FIELD_CHOICES = ((, "Empty Choice"), ('S', "Non Empty Choice"))
char_field = models.CharField(blank=True, default=
, max_length=1, choices=CHAR_FIELD_CHOICES)

Expected:
<select id="id_char_field" name="char_field">
<option value="" selected="selected">Confirmed</option>
<option value="P">Provisional</option>
</select>

Actual Output:
<select id="id_category" name="category">
<option value="" selected="selected">---------</option>
<option value="">Confirmed</option>
<option value="P">Provisional</option>
</select>

Change History (19)

comment:1 Changed 2 years ago by wim@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

Hi, if you use blank=False in your Charfield definition, the blank option will not show.

comment:2 Changed 2 years ago by brillgen

  • Resolution worksforme deleted
  • Status changed from closed to new

That is correct. However I have an option which is blank (please see example above). blank=False does not allow that option to be selected, saying that this field is required.

comment:3 Changed 2 years ago by timo

  • Cc timograham@… added
  • Component changed from Uncategorized to Forms
  • Keywords admin removed
  • Summary changed from unable to override the blank option '--------' in django admin in charfield with choices to Add an easier way to override the blank option for CharFields with choices
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to New feature
  • Version changed from 1.5 to master

I wouldn't consider this a bug, but rather a new feature request.

BaseModelAdmin.formfield_for_choice_field has an example of calling db_field.get_choices which allows customizing the blank option, but this is probably not a public API at this point.

Accepting on the basis that an easier way to customize the blank choice would be helpful (or documenting it if one already exists that I'm not thinking of). Adding an empty_label argument to a CharField with choices may be a possible approach.

comment:4 Changed 2 years ago by Alex Couper <info@…>

  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

comment:5 Changed 2 years ago by brillgen

IMHO, its a bug because the documentation for choices does not say anywhere that the blank is a special case etc. While designing a solution, I would also suggest that you keep in mind this use case (we're not affected by this one yet)

For a Nullable char field with choices, the select options created should be as below:
Null should be represented lets say by the default --------- (and overridable)
"" should also be represented lets say by the default "" (and overridable)
<....remaining options...>

These are all database valid options which should be possible. How its done keeping backward compatibility is why you guys chose to be the core team :)

comment:6 Changed 2 years ago by Alex Couper <info@…>

I am implementing an API change for CharFields with choices such that an empty_label can be specified.

CHOICES = (
    ('p', 'Python'),
    ('j', 'Java')
)

class DummyModel(models.Model):
    language = models.CharField(choices=CHOICES, blank=True, max_length=2,
                                 empty_label="Please select one")

Which would result in

<p><label for="id_language">Language:</label> <select id="id_language" name="language">
<option value="" selected="selected">Please select one</option>
<option value="p">Python</option>
<option value="j">Java</option>
</select></p>

I'd be interested to hear of a use case where you would want to tell the difference between a null and a blank string on a choice field.

comment:7 Changed 2 years ago by alexcouper

  • Owner changed from anonymous to alexcouper

comment:8 Changed 2 years ago by alexcouper

I have submitted a PR to attempt to address this problem, using my understanding of what timo suggested.

https://github.com/django/django/pull/1315

I have a slight concern that we are breaking MVC somewhat, in that we're defining presentation properties on the model as a matter of convenience.
Although having said that, we already do that somewhat with choices, so perhaps this is seen as OK.

As this is my first contribution to Django, I thought it would be a good idea to get feedback at this point before making any documentation changes.

comment:9 Changed 2 years ago by alexcouper

  • Cc info@… added
  • Has patch set

comment:10 Changed 2 years ago by alexcouper

  • Needs documentation set

comment:11 Changed 2 years ago by timo

On second thought, rather than empty_label, how about doing what the OP did and just making sure that the Django blank choice isn't added if a blank choice is already defined in choices?:

CHOICES = (
    ('', 'Please select one'),
    ('p', 'Python'),
    ('j', 'Java')
)

Opinions?

comment:12 Changed 2 years ago by charettes

@timo I also think it would make more sense than an empty_label option however both alternatives introduce a form specific option at the ORM level.

IMHO this belongs at the form level.

comment:13 Changed 2 years ago by charettes

@timo I see that most of the choice logic is unfortunately already defined at the db field level thus you can ignore my last concern.

You suggested approach should work and is way less invasive.

comment:14 Changed 2 years ago by alexcouper

@timo - I prefer that approach too.

The empty_label option felt wrong while I was implementing it TBH.

I'll be away for the next week or so but after that I'll amend my fix (unless I'm beaten to it by someone else).

comment:15 Changed 2 years ago by alexcouper

I have managed to find some time to give the new solution a go.

Let me know if you've got any questions about it. It's on this PR:

https://github.com/django/django/pull/1383

*holds breath*

comment:16 Changed 2 years ago by brillgen

I'm not core or reviewing the patch (just the reporter) but it looks good (without having tested the codebase though ...)

comment:17 Changed 2 years ago by alexcouper

  • Needs documentation unset

comment:18 Changed 2 years ago by timo

  • Summary changed from Add an easier way to override the blank option for CharFields with choices to Add an easier way to override the blank option for Fields with choices

Updated title to reflect that the fact that this isn't specific to CharField but works for any field that defines choices.

comment:19 Changed 2 years ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 1123f4551158b7fc65d3bd88c375a4517dcd0720:

Fixed #20649 -- Allowed blank field display to be defined in the initial list of choices.

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