Opened 10 years ago

Closed 4 years ago

#4117 closed New feature (fixed)

Attributes for Widgets composed of more than one HTML element

Reported by: Iwan Vosloo Owned by: Alex Gaynor
Component: Forms Version: master
Severity: Normal Keywords: attrs, RadioSelect, MultiWidget
Cc: gary.wilson@…, jaywgraves@…, cmawebsite@…, bmispelon@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As suggested in #3870, I'm adding this ticket.

With widgets (such as {{RadioSelect}} or {{MultiWidget}}) that are rendered as several HTML elements/inputs one sometimes need a way to attach HTML attributes to the widget as a whole so that you can style its constituent parts using a stylesheet.

Please see the reason for this request in http://groups.google.com/group/django-developers/browse_thread/thread/e55be1c11893c069

A naïve implementation would be that if you construct the widget with attrs= keyword argument, they should be used as HTML attributes on a suitable element (as in #4080). It seems, however, as if the norm is as in #3870.

I believe a design decision is needed here: #4080 does one thing with attrs, #3870 does another thing with attrs. For general styling, I think #4080 is the way to go, but it appeared to me that #3870 makes sense in other cases. (For example if you want to set readonly attribute on the entire widget, it should be set on all inputs it is composed of).

-i

Attachments (5)

4117.diff (1.7 KB) - added by Gary Wilson <gary.wilson@…> 10 years ago.
attributes for containers of RadioSelect and CheckboxSelectMultiple widgets
widget-ul.diff (6.6 KB) - added by Alex Gaynor 8 years ago.
Here's an up to date patch, it adds the id attr, however it causes one test error(not failure), which is a UnicodeDecodeError
widget-ul.2.diff (8.1 KB) - added by Alex Gaynor 8 years ago.
No more test failures
widget-ul-id.diff (8.3 KB) - added by Alex Gaynor 8 years ago.
This version only adds the id attr to the ul, there is a seperate ticket about making newforms more CSS friendsly so we can defer the rest of the attrs to that ticket.
widget-ul-id.2.diff (8.3 KB) - added by Alex Gaynor 8 years ago.
Same patch, just updated to apply against forms

Download all attachments as: .zip

Change History (22)

comment:1 Changed 10 years ago by Simon G. <dev@…>

Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Please note that #4080 is a subset of what is asked for here and has a (slightly outdated) patch for fixing RadioSelect container attributes.

Changed 10 years ago by Gary Wilson <gary.wilson@…>

Attachment: 4117.diff added

attributes for containers of RadioSelect and CheckboxSelectMultiple widgets

comment:3 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Has patch: set
Needs tests: set

MultiWidget doesn't have a containing element so there is nothing to do there. CheckboxSelectMultiple was the only other widget I noticed besides RadioSelect that used a container element.

comment:4 Changed 10 years ago by Chris Beaven

Patch needs improvement: set

Gary, your patch sets the attributes on the lis as well as the containing ul for the RadioSelect but not the CheckboxSelectMultiple widget.

I'm not sure you need to set it on the lis, do you?

comment:5 Changed 9 years ago by anonymous

Cc: jaywgraves@… added

comment:6 Changed 9 years ago by jaywgraves@…

for solving this for a RadioSelect widget see #4228

Changed 8 years ago by Alex Gaynor

Attachment: widget-ul.diff added

Here's an up to date patch, it adds the id attr, however it causes one test error(not failure), which is a UnicodeDecodeError

Changed 8 years ago by Alex Gaynor

Attachment: widget-ul.2.diff added

No more test failures

comment:7 Changed 8 years ago by Alex Gaynor

Needs tests: unset
Owner: changed from nobody to Alex Gaynor
Patch needs improvement: unset
Status: newassigned

Changed 8 years ago by Alex Gaynor

Attachment: widget-ul-id.diff added

This version only adds the id attr to the ul, there is a seperate ticket about making newforms more CSS friendsly so we can defer the rest of the attrs to that ticket.

comment:8 Changed 8 years ago by Collin Anderson

Cc: cmawebsite@… added

Is #5851 a duplicate of this?

Changed 8 years ago by Alex Gaynor

Attachment: widget-ul-id.2.diff added

Same patch, just updated to apply against forms

comment:9 Changed 8 years ago by Alex Gaynor

milestone: post-1.0

It is with great regret that I say this will have to wait until after 1.0.

comment:10 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:11 Changed 6 years ago by Alex Gaynor

Triage Stage: Design decision neededAccepted

comment:12 Changed 6 years ago by Łukasz Rekucki

Severity: Normal
Type: New feature

comment:13 Changed 6 years ago by patchhammer

Easy pickings: unset
Patch needs improvement: set

widget-ul-id.2.diff fails to apply cleanly on to trunk

comment:14 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:15 Changed 4 years ago by anonymous

Patch needs improvement: unset

I've created a pull request based on Alex's last patch.

Half of the work was already done in #19874.

comment:16 Changed 4 years ago by Baptiste Mispelon

Cc: bmispelon@… added

For some reason, trac decided i was spamming and it logged me out.

Sorry about the noise.

comment:17 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In c4186c2fec6f5418c81366a911792bf5295db494:

Fixed #4117: Apply id attribute to the outer <ul> of RadioSelect

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