Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed bug which caused bound forms to lose selected option #89

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

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.

None yet

3 participants