#2205 closed defect (invalid)
[patch] models.STACKED messes up the fields display in Admin
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | contrib.admin | Version: | |
Severity: | normal | Keywords: | |
Cc: | treborhudson@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have made a Person model that has its fields display ordered as follows:
class Admin:
fields = (
(None, {
'fields' : ('company', 'function', ('first_name', 'last_name'), 'gender', ('phone','phone2'), 'email', ('postal_code', 'streetnumber', 'streetnumber_extension'))
}),
)
That field ordering is not adhered to when a Person is stacked into the edit form of another model. I would expect that the field ordering would be respected even when models.STACKED is used.
Attachments (4)
Change History (25)
by , 18 years ago
Attachment: | Screenshot-Add person | Django site admin - Mozilla Firefox.png added |
---|
by , 18 years ago
Attachment: | Screenshot-Change company | Django site admin - Mozilla Firefox.png added |
---|
Fields of stacked Person are not consistent
comment:1 by , 18 years ago
Just a note to say that I discovered this as well. I didn't see that the field grouping was broken but saw that the spacing of the input fields wasn't aligned like it should be.
by , 18 years ago
Attachment: | admin-stacked.patch added |
---|
Not a fix, but this shows which CSS declarations breaks the stacked alignment. Possibly the inline CSS needs to account for both STACKED and TABULAR?
by , 18 years ago
Attachment: | admin-stacked.2.patch added |
---|
I goofed and ran svn diff at the css directory, not the top level django directory
comment:2 by , 18 years ago
FYI: With the removal of the 2 inline CSS declarations, the models.TABULAR still looks the same on Firefox.
comment:3 by , 18 years ago
Summary: | models.STACKED messes up the fields display in Admin → [patch] models.STACKED messes up the fields display in Admin |
---|
I've been using the last patch on this with not visual defects. I'm going to call it a patch.
comment:5 by , 18 years ago
Ticket #2719 has an alternate fix:
- label.inline { margin-left:20px; }
+ label.inline { font-weight:normal !important; color:#666; font-size:12px; }
- .aligned label.inline { display:inline; float:none; }
+ .aligned label.inline { display:block; padding:0 1em 3px 0; float:left; width:8em; }
comment:6 by , 18 years ago
Triage Stage: | Unreviewed → Ready for checkin |
---|
comment:7 by , 18 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
Before checking in any version of this patch, I would like to hear some feedback that it works without breaking anything on both IE6/Win and Safari (which means, also doesn't break TABULAR display).
comment:8 by , 18 years ago
We've been using the patch since I submitted it. (Sorry it's named as a .diff file but I can upload a new one if need be.)
Just today I verified the following:
- Admin STACKED looks the same in: Firefox/Win and Firefox/Mac, IE6/Win and IE7/Win, and Safari.
- TABULAR display doesn't output the CSS class "inline" so this patch won't affect TABULAR display, but I checked it any way and the tabular display wasn't affected.
There's probably more that could be done with this. For example, there is a CSS declaration for labels inside a ul with class "radiolist" that could possibly also be removed but I'm not sure what triggers that output in the admin and so I haven't removed it from the patch. There are also "inline" class declarations on other elements but I'm guessing that since they aren't labels that are aligned along the left, they are supposed to be there.
But removing those 2 lines in the forms.css file is a drastic improvement in how it looks that it seems like it was a mistake that they got in there in the first place. My guess is those declarations were mistakenly added to the label tags.
Help me figure out how to get a radiolist to display and I'll test (and possibly fix) how that works.
Thanks,
Rob
comment:9 by , 18 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Lets get this in at least then.
comment:10 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:11 by , 18 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Unless I'm mistaken, the CSS patch only fixes the formatting for inline-stacked models; it seems to break some other formatting (see ticket #3596), and doesn't fix the ordering problem at all.
follow-up: 17 comment:12 by , 18 years ago
At a glance, it seems like the the alignment isn't actually a CSS problem, but rather that the "inline" class should not be applied to labels in stacked form-rows. (IIUC, "inline" should not be applied to the first label in a form row anyway..)
comment:13 by , 18 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:15 by , 18 years ago
The field and label display issues after 4590 are a real showstopper. Perhaps the patch should be rolled back and re-visited?
follow-up: 18 comment:17 by , 18 years ago
Has patch: | unset |
---|---|
Triage Stage: | Accepted → Design decision needed |
Seems like this is complicated enough that it could just be held off until newforms-admin.
comment:18 by , 18 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
Replying to Rob Hudson <treborhudson@gmail.com>:
Seems like this is complicated enough that it could just be held off until newforms-admin.
I agree that we could consider this not having a patch, but it's stage is Accepted since this is something that we have decided we want to fix.
comment:19 by , 17 years ago
The problem was reported over a year ago. Is it really that complicated fixing this (no offense, I´m just curious)?
comment:20 by , 17 years ago
Resolution: | → invalid |
---|---|
Status: | reopened → closed |
This is made obsolete by newforms-admin, and the method used there to handle this situation is already working properly.
Correctly shown Person, not stacked