Opened 9 years ago

Closed 8 years ago

Last modified 6 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: UI/UX:

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@… 9 years ago.
Correctly shown Person, not stacked
Screenshot-Change company | Django site admin - Mozilla Firefox.png (54.8 KB) - added by vietyen@… 9 years ago.
Fields of stacked Person are not consistent
admin-stacked.patch (1.3 KB) - added by treborhudson@… 9 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@… 9 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)

Changed 9 years ago by vietyen@…

Correctly shown Person, not stacked

Changed 9 years ago by vietyen@…

Fields of stacked Person are not consistent

comment:1 Changed 9 years ago by treborhudson@…

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.

Changed 9 years ago by treborhudson@…

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?

Changed 9 years ago by treborhudson@…

I goofed and ran svn diff at the css directory, not the top level django directory

comment:2 Changed 9 years ago by treborhudson@…

FYI: With the removal of the 2 inline CSS declarations, the models.TABULAR still looks the same on Firefox.

comment:3 Changed 9 years ago by treborhudson@…

  • Summary changed from models.STACKED messes up the fields display in Admin to [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 Changed 9 years ago by treborhudson@…

  • Cc treborhudson@… added

Adding myself to CC

comment:5 Changed 9 years ago by treborhudson@…

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 Changed 8 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Ready for checkin

comment:7 Changed 8 years ago by mtredinnick

  • Triage Stage changed from Ready for checkin to 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 Changed 8 years ago by Rob Hudson <treborhudson@…>

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 Changed 8 years ago by SmileyChris

  • Triage Stage changed from Accepted to Ready for checkin

Lets get this in at least then.

comment:10 Changed 8 years ago by jacob

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

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

comment:11 Changed 8 years ago by matt <matt.barry@…>

  • Resolution fixed deleted
  • Status changed from closed to 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.

comment:12 follow-up: Changed 8 years ago by matt <matt.barry@…>

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

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

comment:14 Changed 8 years ago by Gary Wilson <gary.wilson@…>

above comment was me

comment:15 Changed 8 years ago by brooks.travis@…

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

comment:16 Changed 8 years ago by Gary Wilson <gary.wilson@…>

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

comment:17 in reply to: ↑ 12 ; follow-up: Changed 8 years ago by Rob Hudson <treborhudson@…>

  • Has patch unset
  • Triage Stage changed from Accepted to Design decision needed

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

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

  • Patch needs improvement unset
  • Triage Stage changed from Design decision needed to 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 Changed 8 years ago by patrick@…

The problem was reported over a year ago. Is it really that complicated fixing this (no offense, I´m just curious)?

comment:20 Changed 8 years ago by ubernostrum

  • Resolution set to invalid
  • Status changed from reopened to closed

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