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:

Description


Attachments (1)

5631.diff (589 bytes) - added by Karen Tracey 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: invalid
Status: newclosed

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 Andy Baker

Cc: andy@… added
Resolution: invalid
Status: closedreopened
Version: newforms-admin

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.

So:

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

collapses successfully but...

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

doesn't.

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

comment:5 Changed 8 years ago by Karen Tracey

Has patch: set
milestone: 1.0
Summary: newforms-admin: 'classes': 'collapse' does not worknewforms-admin: 'classes': 'collapse' does not work (in one case)
Triage Stage: UnreviewedAccepted

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 Karen Tracey

Attachment: 5631.diff added

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 Nikolay

Cc: djanger@… added

comment:9 Changed 8 years ago by Brian Rosner

Resolution: fixed
Status: reopenedclosed

(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

Milestone 1.0 deleted

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