[Django] #28712: Add ability to apply separate attributes to ChoiceWidget options
Django
-------------------------------------------+------------------------------
Reporter: Stephen Swatman | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords: ChoiceWidget
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------------+------------------------------
For `ChoiceWidget` objects the list of options is created in the
`create_option` method. Currently, options created by this method can
either inherit attributes from the parent widget or have no attributes at
all but there is no way to separately assign attributes to options as far
as I am aware. I would like to suggest introducing the ability to specify
separate option attributes in the `ChoiceWidget` constructor which are
applied only to the options in that widget.
Even more powerful would be the ability to pass callables as attribute
values which could dynamically compute attributes based on option
attributes such as names and values.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Stephen Swatman | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: ChoiceWidget | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
Changes (by Tim Graham):
* easy: 1 => 0
Comment:
Could you describe your use case in more detail? Is there a reason that
overriding `ChoiceWidget.create_option()` wouldn't work?
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:1>
Django
Reporter: Stephen Swatman | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: ChoiceWidget | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
Comment (by Stephen Swatman):
Replying to [comment:1 Tim Graham]:
> Could you describe your use case in more detail? Is there a reason that
overriding `ChoiceWidget.create_option()` wouldn't work?
Web developers may want to individually style each option in a
`ChoiceWidget`, be it by adding CSS classes or style attributes. This may
be done to add images to such a widget to make it more appealing to the
user. Developers may also want to add ARIA properties to the options or
add additional information to work with them via JavaScript.
It would absolutely be possible to override the `ChoiceWidget` class and
modifying the `create_option()` method. I imagine, however, that this is a
common enough use case to warrant a simpler method of doing this. Here are
some people on StackOverflow having the same issue:
* https://stackoverflow.com/questions/6477856/how-to-add-attributes-to-
option-tags-in-django
* https://stackoverflow.com/questions/43524214/django-add-attributes-to-
selectmultiple-option-tags
* https://stackoverflow.com/questions/5089396/django-form-field-choices-
adding-an-attribute
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:2>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Stephen Swatman):
* owner: nobody => Stephen Swatman
* status: new => assigned
* has_patch: 0 => 1
Comment:
Proposed PR: https://github.com/django/django/pull/9260
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:3>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
As for the case of styling, wouldn't it involve less markup to set a class
on the `<select>` and use a CSS selector (e.g. `select.super option
{...}`) for that? If anything, I'd think we'd want a way to specify a
different attribute value for each `<option>` value -- setting the same
key/value on every `<option>` doesn't seem like something to encourage.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:4>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Stephen Swatman):
Replying to [comment:4 Tim Graham]:
> As for the case of styling, wouldn't it involve less markup to set a
class on the `<select>` and use a CSS selector (e.g. `select.super option
{...}`) for that? If anything, I'd think we'd want a way to specify a
different attribute value for each `<option>` value -- setting the same
key/value on every `<option>` doesn't seem like something to encourage.
You're right that the implementation in the PR was not very powerful. I
have now expanded the PR to allow for callable attribute values. This
would allow developers a lot of control over the attributes of each
option.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:5>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I still don't find the example in the documentation particularly
compelling. Instead of adding CSS classes for that case, I would use CSS
[value=x] selectors.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:6>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Stephen Swatman):
Replying to [comment:6 Tim Graham]:
> I still don't find the example in the documentation particularly
compelling. Instead of adding CSS classes for that case, I would use CSS
[value=x] selectors.
The particular case that inspired me to create this issue is that I had a
model and I wanted to create a choice widget for that model. I wanted each
option to be styled such that it had a background image served from a
different server such that I needed to construct an attribute of the form
`style="background-image: url(https://somewhere.com/images/[id].png)"`
where the ID was the model's ID. The problem was solved by extending the
`Select` class and overriding the `create_option()` method. However, I
feel it would have been more easily solved had the pull request I created
been in place.
Still, I absolutely understand if you think this is too niche. Thus, if I
haven't convinced you please feel free to close the issue!
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:7>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I think the functionality could be useful but it would be nice to document
a more useful use case. Also, the patch documents the new functionality as
`Select.opt_attrs` but the implementation is on `ChoiceWidget`. Other
classes such as `RadioSelect` inherit from `ChoiceWidget`, not from
`Select`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:8>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
I have made an attempt to address your concerns: a more compelling example
is given and I have placed it in a more sensible section to cover the
entire `ChoiceWidget` family.
Jenkins is indicating non-passing tests but logs and the fact that only
minor documentation changes have been made and that that the docs test
still passes lead me to believe this is not an issue with the pull
request.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:9>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Tim Martin (added)
* needs_better_patch: 0 => 1
Comment:
I added a couple of minor comments on the documentation in the pull
request.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:10>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Replying to [comment:10 Tim Martin]:
> I added a couple of minor comments on the documentation in the pull
request.
Thank you! I have amended the mistakes you pointed out.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:11>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Bjorn Kristinsson):
I ran into a case the other day where this would have been extremely
useful. I needed to show a select with all options visible, but depending
on the user's permissions some of the options needed to be disabled. This
can be achieved by setting the disabled attribute, but since there was no
way to do that per option, I had to roll my own.
Looks like this would solve that, and if you're still looking for an
example to use in the docs, maybe you could show how to disable options?
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:12>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: ChoiceWidget | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
I've left a review on the patch.
My main concern is the documentation: we need to properly document
`ChoiceWidget` so that this new feature would make sense.
Beyond that (also on the PR) I think we should be advocating the subclass
and override `create_option` approach. I know people always want an extra
keyword arg, but such an approach is both lightweight and maintainable
(both for Django and user code).
That approach is under-documented currently. I would advocate closing this
as `wontfix` and creating a new ticket to properly document
`ChoiceWidget`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:13>
Django
Reporter: Stephen Swatman | Owner: Stephen
| Swatman
Component: Forms | Version: master
Severity: Normal | Resolution: wontfix
Keywords: ChoiceWidget | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => wontfix
--
Ticket URL: <https://code.djangoproject.com/ticket/28712#comment:14>