Skip to content

Conversation

@io2
Copy link
Contributor

@io2 io2 commented Mar 23, 2021

in django forms.ModelForm types, the select > option value attribute is usually set to the primary key of the model object (typically an integer)

This commit converts field.value to string to ensure the if condition for adding selected to the <option> html is satisfied.

io2 and others added 6 commits March 19, 2021 17:48
When using any of the `formset_factory`s to generate multiple forms, the select fields lose their values if the form is returned with an error.

This happens because the template renders `field.name` instead of `field.html_name`.

`field.name` is the same for all forms, `field.html_name` is unique for each form instance
in forms.ModelForm types, the `select > option` value attribute
is usually set to the primary key of the model object (usually an int)

This commit converts `field.value` to string to ensure the `if` condition
    for adding selected to the <option> is met.

Added tests to better demonstrate the issue
@smithdc1
Copy link
Member

Hi @io2

Sorry for taking some time to get to this. There's a few things going on with this at the moment.

The first is that we should probably mirror the logic from checkboxes and radios. There are a few other edge cases we should look to capture. See this line here.

<input type="checkbox" class="{{ css_container.checkboxselectmultiple }}"{% if choice.0 in field.value or choice.0|stringformat:"s" in field.value or choice.0|stringformat:"s" == field.value|default_if_none:""|stringformat:"s" %} checked="checked"{% endif %} name="{{ field.html_name }}" id="id_{{ field.html_name }}_{{ forloop.counter }}" value="{{ choice.0|unlocalize }}" {{ field.field.widget.attrs|flatatt }}>

Secondly is that this can be improved using the new optgroups filter in django-crispy-forms which will simplify this logic here significantly. See django-crispy-forms/django-crispy-forms#1127

@io2
Copy link
Contributor Author

io2 commented Apr 26, 2021

@smithdc1 I'll take a look sometime and push an update.

@GitRon
Copy link
Contributor

GitRon commented Jan 5, 2024

Hi @io2!

@smithdc1 I'll take a look sometime and push an update.

Any update on this topic? 🙂

Best
Ronny

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants