Code

Opened 11 months ago

Closed 8 months ago

#20555 closed New feature (fixed)

Subwidgets not passed id attribute

Reported by: mdj2@… Owned by: mdj2
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 %}

Attachments (0)

Change History (7)

comment:1 Changed 11 months ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 11 months ago by bmispelon

  • Cc bmispelon@… added
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to New 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 months ago by bmispelon (next)

comment:3 Changed 11 months ago by mdj2

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

comment:4 Changed 11 months ago by mdj2

  • 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 Changed 10 months ago by anonymous

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 Changed 10 months ago by timo

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 Changed 8 months ago by Tim Graham <timograham@…>

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

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.