Opened 19 years ago
Closed 19 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 , 19 years ago
| Attachment: | newforms.patch added |
|---|
comment:1 by , 19 years ago
comment:2 by , 19 years ago
comment:3 by , 19 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%sin it, it's the default id (based on name) for any widget, any other non-false value causes theidto match thename.
While you're reading, I have some other general comments:
- It would be much nicer if
as_tableand 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. ErrorDictshould 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 Nonewhen 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 , 19 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 , 19 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:
Labelas a subclass ofWidget. AlthoughLabelmight share some behavior withWidgetcoincidentally, they're fundamentally different things.idattributes off of widgets because it struck me as a bit of magic. The framework should only add things that are absolutely necessary, and auto-addingidattributes has some problems, namely that it assumes no other form on the page has that ID. Granted, Django's current form system puts in theids automatically, but that has struck me as slightly inelegant. I see this patch only puts in theidif 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 inids, though; I'm not super passionate about the decision.