Opened 9 years ago

Closed 8 years ago

Last modified 5 years ago

#5631 closed (fixed)

newforms-admin: 'classes': 'collapse' does not work (in one case)

Reported by: anonymous Owned by: nobody
Component: contrib.admin Version:
Severity: Keywords: newforms-admin collapse admin
Cc: aloysius.prayitno@…, andy@…, flori@…, email@…, djanger@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Attachments (1)

5631.diff (589 bytes) - added by kmtracey 8 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 9 years ago by awi <aloysius.prayitno@…>

  • Cc aloysius.prayitno@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 9 years ago by django@…

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

AFAIK the syntax has changed to 'classes': collapse?

comment:3 Changed 9 years ago by django@…

Let's try that again:

'classes': ['collapse']

comment:4 Changed 8 years ago by andybak

  • Cc andy@… added
  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Version newforms-admin deleted

This is actually still a problem post newforms-admin merge. What appears to happening is that:

  1. CollapsedFieldsets.js in admin-media checks to make sure that no fieldset with a class of 'errors' is collapsed.
  2. For some reason if you group any fields into a single line then that fieldset get's given a class of 'error'

I suspect #2 might be a workaround for some other problem if it's not an out and out mistake.


('Heading', {
  'classes': ('collapse',),
  'fields': ('field1','field2','field3')})

collapses successfully but...

('Heading', {
  'classes': ('collapse',),
  'fields': (('field1','field2'),'field3')})


Incidentally - setting the fieldset name to None also breaks 'collapse'.

comment:5 Changed 8 years ago by kmtracey

  • Has patch set
  • milestone set to 1.0
  • Summary changed from newforms-admin: 'classes': 'collapse' does not work to newforms-admin: 'classes': 'collapse' does not work (in one case)
  • Triage Stage changed from Unreviewed to Accepted

That's a very specific case for which collapse doesn't work, it does work in general. I'll attach a patch to fix the multi-field line case. The problem is that the errors() method for FieldLine joins the errors from each field with newlines, resulting in at least one newline if there is more than one field even if there are no errors. Stripping newlines after the join fixes this and does not change the visible output in cases where there are errors. (Note when there are errors, it may be difficult to determine what field a particular error message references, but that was the case both before and after the fix.)

As for setting the fieldset name to None breaking collapse, I cannot recreate that, unless by 'breaking' you mean that you get a collapsed fieldset that you cannot uncollapse because there is no header line on which to hang the "Show" link. IMO you just shouldn't do that. If you mean something else please open a different ticket to explain what you see as wrong and track fixing it.

Changed 8 years ago by kmtracey

comment:6 Changed 8 years ago by flosch

  • Cc flori@… email@… added

comment:7 Changed 8 years ago by Karen Tracey <kmtracey@…>

#8072 reported the same problem and suggested the same patch.

comment:8 Changed 8 years ago by niksite

  • Cc djanger@… added

comment:9 Changed 8 years ago by brosner

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

(In [8177]) Fixed #5631 -- When fieldsets contains a field that is on the same line, it is now collapsable when collapse is set in classes. Thanks andybak for some research and Karen Tracey for the patch.

comment:10 Changed 5 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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