Opened 18 years ago
Closed 18 years ago
#3023 closed enhancement (fixed)
[patch] Newforms - widgets should use id and forms should use labels
Reported by: | Chris Beaven | Owned by: | Adrian Holovaty |
---|---|---|---|
Component: | Core (Other) | Version: | |
Severity: | normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Firstly, widgets should use id
if a name
is given. When I did this I cleaned up some un-DRYness in the widget code.
Secondly, forms should use labels when outputting.
The new Label
widget uses the name as the for
attribute.
I went through all the tests and fixed them up.
Attachments (1)
Change History (7)
by , 18 years ago
Attachment: | newforms.patch added |
---|
comment:1 by , 18 years ago
comment:2 by , 18 years ago
comment:3 by , 18 years ago
Thanks for the comments.
Fair enough about Label
not really being a widget, I just wanted somewhere to put it. I'd still say it could go in there without too much fuss, just broaden your view of a widget. ;)
Some id
comments:
- Ok, let's remove some magic, but we need to have a way to automatically add them in! Every elegant HTML form should use labels, and they are only supported properly across browsers by the use of
for="id"
. - How about a property of the form called
auto_id
. Ifauto_id == False
(which it is by default), no auto-ids. If set to a string with a%s
in it, it's the default id (based on name) for any widget, any other non-false value causes theid
to match thename
.
While you're reading, I have some other general comments:
- It would be much nicer if
as_table
and the like didn't actually include the<table>
wrapper (or<ul>
wrapper, etc). For most flexibility, the designer should be able to put that in so they can class it however they like. ErrorDict
should inheritdjango.utils.datastructures.SortedDict
.- An easy way of not using errors if it's a form with no passed data would be very helpful. My suggestion is that if
data is None
when in theForm
's initialized, then errors should be suppressed (this way, you could pass an empty dict still if you want the errors initially).
comment:5 by , 18 years ago
SmileyChris: I don't necessarily agree about using SortedDict
-- what's the use-case for needing the keys of the error dictionary to be ordered? The keys simply correspond to the field names, and the order of the fields is known, so it's possible to reconstruct the error dict in order if you really want to. Besides, I can't think of any time when you'd want to.
Thanks for opening #3025. Could you open a separate ticket for the "easy way of not using errors" issue and as_table
issue? Once those are opened, feel free to close this ticket.
comment:6 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
You're right, SortedDict
is not really needed.
Ticket closed since I've split out the other issues.
A couple of comments on this patch:
Label
as a subclass ofWidget
. AlthoughLabel
might share some behavior withWidget
coincidentally, they're fundamentally different things.id
attributes off of widgets because it struck me as a bit of magic. The framework should only add things that are absolutely necessary, and auto-addingid
attributes has some problems, namely that it assumes no other form on the page has that ID. Granted, Django's current form system puts in theid
s automatically, but that has struck me as slightly inelegant. I see this patch only puts in theid
if an ID hasn't been explicitly provided (which is nice), but it still has the problem of magically/automatically putting in the ID. I'm definitely open to hearing arguments for or against automatically putting inid
s, though; I'm not super passionate about the decision.