Opened 17 years ago

Closed 11 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: dev
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@…> 17 years ago.
attributes for containers of RadioSelect and CheckboxSelectMultiple widgets
widget-ul.diff (6.6 KB ) - added by Alex Gaynor 16 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 16 years ago.
No more test failures
widget-ul-id.diff (8.3 KB ) - added by Alex Gaynor 16 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 16 years ago.
Same patch, just updated to apply against forms

Download all attachments as: .zip

Change History (22)

comment:1 by Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedDesign decision needed

comment:2 by Gary Wilson <gary.wilson@…>, 17 years ago

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

by Gary Wilson <gary.wilson@…>, 17 years ago

Attachment: 4117.diff added

attributes for containers of RadioSelect and CheckboxSelectMultiple widgets

comment:3 by Gary Wilson <gary.wilson@…>, 17 years ago

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 by Chris Beaven, 17 years ago

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 by anonymous, 17 years ago

Cc: jaywgraves@… added

comment:6 by jaywgraves@…, 17 years ago

for solving this for a RadioSelect widget see #4228

by Alex Gaynor, 16 years ago

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

by Alex Gaynor, 16 years ago

Attachment: widget-ul.2.diff added

No more test failures

comment:7 by Alex Gaynor, 16 years ago

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

by Alex Gaynor, 16 years ago

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 by Collin Anderson, 16 years ago

Cc: cmawebsite@… added

Is #5851 a duplicate of this?

by Alex Gaynor, 16 years ago

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

Same patch, just updated to apply against forms

comment:9 by Alex Gaynor, 16 years ago

milestone: post-1.0

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

comment:10 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:11 by Alex Gaynor, 14 years ago

Triage Stage: Design decision neededAccepted

comment:12 by Łukasz Rekucki, 13 years ago

Severity: Normal
Type: New feature

comment:13 by patchhammer, 13 years ago

Easy pickings: unset
Patch needs improvement: set

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

comment:14 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:15 by anonymous, 11 years ago

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 by Baptiste Mispelon, 11 years ago

Cc: bmispelon@… added

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

Sorry about the noise.

comment:17 by Claude Paroz <claude@…>, 11 years ago

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