Code

Opened 7 years ago

Closed 12 months ago

#4117 closed New feature (fixed)

Attributes for Widgets composed of more than one HTML element

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

Download all attachments as: .zip

Change History (22)

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

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 7 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 7 years ago by Gary Wilson <gary.wilson@…>

attributes for containers of RadioSelect and CheckboxSelectMultiple widgets

comment:3 Changed 7 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 7 years ago by SmileyChris

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

  • Cc jaywgraves@… added

comment:6 Changed 7 years ago by jaywgraves@…

for solving this for a RadioSelect widget see #4228

Changed 6 years ago by Alex

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 6 years ago by Alex

No more test failures

comment:7 Changed 6 years ago by Alex

  • Needs tests unset
  • Owner changed from nobody to Alex
  • Patch needs improvement unset
  • Status changed from new to assigned

Changed 6 years ago by Alex

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 6 years ago by CollinAnderson

  • Cc cmawebsite@… added

Is #5851 a duplicate of this?

Changed 6 years ago by Alex

Same patch, just updated to apply against forms

comment:9 Changed 6 years ago by Alex

  • milestone set to post-1.0

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

comment:10 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:11 Changed 4 years ago by Alex

  • Triage Stage changed from Design decision needed to Accepted

comment:12 Changed 3 years ago by lrekucki

  • Severity set to Normal
  • Type set to New feature

comment:13 Changed 3 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 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:15 Changed 12 months 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 12 months ago by bmispelon

  • Cc bmispelon@… added

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

Sorry about the noise.

comment:17 Changed 12 months ago by Claude Paroz <claude@…>

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

In c4186c2fec6f5418c81366a911792bf5295db494:

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

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.