Opened 11 years ago
Closed 11 years ago
#20555 closed New feature (fixed)
Subwidgets not passed id attribute
Reported by: | 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 , 11 years ago
comment:2 by , 11 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → 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.
comment:3 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 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 , 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 , 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 , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.