Opened 11 years ago

Closed 11 years ago

#20555 closed New feature (fixed)

Subwidgets not passed id attribute

Reported by: mdj2@… Owned by: Matt Johnson
Component: Forms Version: 1.6-alpha-1
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

If an id is not defined in a widget's attrs dictionary, and the form.auto_id variable is True, the BoundField class will generate an id for the widget automatically (via the auto_id property). The id is typically used when rendering the markup for the input element and the label tag.

If a BoundField has subwidgets, the automatically generated id is NOT passed to the subwidget. This makes it difficult to construct a label tag that references the subwidget. Consider this example:

{% for subwidget in form.some_field_with_subwidgets %}
    {{ subwidget.tag }} <label for="????">{{ subwidget.choice_label }}</label>
{% endfor %}

As far as I'm aware, there is no way to access the subwidget's automatically generated id for use in the label's "for" attribute. One must resort to something like:

<label for="{{ form.form.some_field_with_subwidgets.auto_id }}_{{ forloop.counter0 }}">

To fix this, I replaced BoundField.iter with:

    def __iter__(self):
        """
        Yields rendered strings that comprise all widgets in this BoundField.

        This really is only useful for RadioSelect widgets, so that you can
        iterate over individual radio buttons in a template.
        """
        id_ = self.field.widget.attrs.get('id') or self.auto_id
        attrs = {'id': id_} if id_ else {}
        for subwidget in self.field.widget.subwidgets(self.html_name, self.value(), attrs=attrs):
            yield subwidget

This allows me to do something like:

{% for subwidget in form.some_field_with_subwidgets %}
    {{ subwidget.tag }} <label for="{{ subwidget.attrs.id }}">{{ subwidget.choice_label }}</label>
{% endfor %}

Change History (7)

comment:1 by anonymous, 11 years ago

I just discovered my little patch won't work unless subwidget.tag is called first in the template, since that method modifies the subwidget's id to include the index number. Perhaps the SubWidget or ChoiceInput classes should have an id, or auto_id property.

comment:2 by Baptiste Mispelon, 11 years ago

Cc: bmispelon@… added
Triage Stage: UnreviewedAccepted
Type: BugNew feature

This seems like a useful feature to add.

I think the fact that ChoiceInput.tag mutates self.attrs should be fixed. I haven't looked into the problem but I don't see why this couldn't be done in ChoiceInput.init instead (and then your patch could be used).

Regarding your patch, the idea seems sound but I think the logic of passing the id to attrs should be moved into a separate function so that it can be shared with as_widget which does the same thing.

Version 0, edited 11 years ago by Baptiste Mispelon (next)

comment:3 by Matt Johnson, 11 years ago

Owner: changed from nobody to Matt Johnson
Status: newassigned

comment:4 by Matt Johnson, 11 years ago

Has patch: set

Here's my first attempt at a patch: https://github.com/django/django/pull/1244

It seems like BoundField.id_for_label ought to be used in most places where the widget's id attribute is needed. It handles the logic of getting the ID from the widget's attributes or from the auto_id property. The problem is that RendererMixin.id_for_label appends a _0 to the widget's ID:

# Widgets using this RendererMixin are made of a collection of
# subwidgets, each with their own <label>, and distinct ID.
# The IDs are made distinct by y "_X" suffix, where X is the zero-based
# index of the choice field. Thus, the label for the main widget should
# reference the first subwidget, hence the "_0" suffix.

I'm not sure why the main widget's label should reference the first subwidget (MultiWidget appends _0 to the id as well). But if that feature was removed, then accessing BoundField.id_for_label could become the canonical way to get a widget's ID.

comment:5 by anonymous, 11 years ago

I would be grateful for any feedback on the feature/patch. I think it would be a useful addition to 1.6 especially with the introduction of subwidgets on the CheckboxSelectMultiple widget.

comment:6 by Tim Graham, 11 years ago

The patch looks good to me. Unfortunately, it's too late to add this to 1.6 (beta marks the feature freeze), but we can commit it to master.

Would a documentation update be helpful here? For example, in the discussion of iterating over RadioSelect choices: https://docs.djangoproject.com/en/dev/ref/forms/widgets/#radioselect

I don't think this is backwards-incompatible, but just want to confirm.

comment:7 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 907ef9d0d157c47c66bf265dca93a0bee8664ea3:

Fixed #20555 -- Make subwidget id attribute available

In BoundField.__iter__, the widget's id attribute is now passed to
each subwidget. A new id_for_label property was added to ChoiceInput.

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