Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#28176 closed New feature (fixed)

Expose real (i.e. not converted to string) value in widget option context

Reported by: Moritz Sichert Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Preston Timmons Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In #28075 option.value was changed to always be converted to a string before passing it to the template. I have a use case that doesn't work anymore because of this. I have this form:

from django.forms import Form, RadioSelect, TypedChoiceField


CHOICES = [
    (1, 'INFO'),
    (2, 'WARNING'),
    (3, 'SUCCESS'),
    (4, 'DANGER'),
]

CHOICES_DICT = {name: value for value, name in CHOICES}


class RadioSelectWithIcons(RadioSelect):
    template_name = 'radio_select_with_icons.html'

    def get_context(self, name, value, attrs):
        context = super().get_context(name, value, attrs)
        context['choices'] = CHOICES_DICT
        return context


class MyForm(Form):
    foo = TypedChoiceField(
        coerce=int,
        choices=CHOICES,
        widget=RadioSelectWithIcons
    )

and the following template (radio_select_with_icons.html):

{% for _, options, _ in widget.optgroups %}
  {% for option in options %}
    <label class="btn btn-default{% if option.selected %} active{% endif %}">
      <input type="radio" name="{{ widget.name }}" value="{{ option.value }}"{% if option.selected %} checked="checked"{% endif %}>
      {{ option.label }}
      {% if option.value == choices.INFO %}
        <span class="fa fa-info-circle text-info"></span>
      {% elif option.value == choices.WARNING %}
        <span class="fa fa-times-circle text-warning"></span>
      {% elif option.value == choices.SUCCESS %}
        <span class="fa fa-check-circle text-success"></span>
      {% elif option.value == choices.DANGER %}
        <span class="fa fa-exclamation-circle text-danger"></span>
      {% endif %}
    </label>
  {% endfor %}
{% endfor %}

Now, when I render this form, none of the <span class="fa ..."></span> are displayed. This is because option.value is a string and choices.X are integers so the equality will obviously always be false.

I can see two solutions to this that don't involve changing code in Django:

1.: Convert the integers in the choices to strings as well. I don't think this is a sensible solution since I know that both values are supposed to be integers but instead compare their string representations. This also becomes a problem if the string version of the value is ambiguous, e.g. translated strings.
2.: Pass the css classes I use in the <span> as context from the widget. I also think, that this is a bad approach, because I want to separate python code from display logic. This also wouldn't work for more complex things that don't change just a css class.

I think there is a good, simple solution to this: In ChoiceWidget.create_option() the returned context should also contain the un-stringified value. Maybe even rename the current value to str_value and store the actual value in value.

What do you think?

Change History (6)

comment:1 Changed 2 years ago by Tim Graham

Cc: Preston Timmons added

comment:2 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:3 Changed 2 years ago by cgons

A third option to deal with this issue within the template itself would be to use a django filter to convert the int into a string:

{{ value|stringformat:"i" }}

For example:

{% if option.value == choices.INFO|stringformat:"i" %}
    <span class="fa fa-info-circle text-info"></span>
    ...

Relevant docs: https://docs.djangoproject.com/en/dev/ref/templates/builtins/#stringformat\

It's definitely not ideal though.

comment:4 Changed 2 years ago by Tim Graham

Has patch: set

On the PR for #28303 John suggested, "If this lands, the option's input's value could be added to the attrs dict in Widget.create_option(), then value could resume to be the actual value, not the coerced value." I implemented that idea in the second commit of a PR, then simplified things to what I think is a simpler approach in the third commit.

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

Resolution: fixed
Status: newclosed

In 221e6e1:

Fixed #28176 -- Restored the uncasted option value in ChoiceWidget template context.

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

In a3b1319:

[1.11.x] Fixed #28176 -- Restored the uncasted option value in ChoiceWidget template context.

Backport of 221e6e18177516ac4ac95e40c344b93d14dd607b from master

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