Opened 18 years ago

Closed 17 years ago

Last modified 15 years ago

#2205 closed defect (invalid)

[patch] models.STACKED messes up the fields display in Admin

Reported by: vietyen@… 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)

Screenshot-Add person | Django site admin - Mozilla Firefox.png (47.7 KB ) - added by vietyen@… 18 years ago.
Correctly shown Person, not stacked
Screenshot-Change company | Django site admin - Mozilla Firefox.png (54.8 KB ) - added by vietyen@… 18 years ago.
Fields of stacked Person are not consistent
admin-stacked.patch (1.3 KB ) - added by treborhudson@… 18 years ago.
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?
admin-stacked.2.patch (1.3 KB ) - added by treborhudson@… 18 years ago.
I goofed and ran svn diff at the css directory, not the top level django directory

Download all attachments as: .zip

Change History (25)

by vietyen@…, 18 years ago

Correctly shown Person, not stacked

by vietyen@…, 18 years ago

Fields of stacked Person are not consistent

comment:1 by treborhudson@…, 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 treborhudson@…, 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 treborhudson@…, 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 treborhudson@…, 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 treborhudson@…, 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:4 by treborhudson@…, 17 years ago

Cc: treborhudson@… added

Adding myself to CC

comment:5 by treborhudson@…, 17 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 Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedReady for checkin

comment:7 by Malcolm Tredinnick, 17 years ago

Triage Stage: Ready for checkinAccepted

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 Rob Hudson <treborhudson@…>, 17 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 Chris Beaven, 17 years ago

Triage Stage: AcceptedReady for checkin

Lets get this in at least then.

comment:10 by Jacob, 17 years ago

Resolution: fixed
Status: newclosed

(In [4590]) Small fix to admin CSS. Fixes #2205; thanks, treborhudson@…

comment:11 by matt <matt.barry@…>, 17 years ago

Resolution: fixed
Status: closedreopened

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.

comment:12 by matt <matt.barry@…>, 17 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 anonymous, 17 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

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

above comment was me

comment:15 by brooks.travis@…, 17 years ago

The field and label display issues after 4590 are a real showstopper. Perhaps the patch should be rolled back and re-visited?

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

Just noting that [4590] was reverted in [4652] (#3596).

in reply to:  12 ; comment:17 by Rob Hudson <treborhudson@…>, 17 years ago

Has patch: unset
Triage Stage: AcceptedDesign decision needed

Seems like this is complicated enough that it could just be held off until newforms-admin.

in reply to:  17 comment:18 by Gary Wilson <gary.wilson@…>, 17 years ago

Patch needs improvement: unset
Triage Stage: Design decision neededAccepted

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 patrick@…, 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 James Bennett, 17 years ago

Resolution: invalid
Status: reopenedclosed

This is made obsolete by newforms-admin, and the method used there to handle this situation is already working properly.

Note: See TracTickets for help on using tickets.
Back to Top