Opened 19 years ago

Closed 8 years ago

Last modified 8 years ago

#494 closed New feature (fixed)

Collapse in admin interface for inline related objects

Reported by: jcstover@… Owned by: Tim Graham <timograham@…>
Component: contrib.admin Version: dev
Severity: Normal Keywords: edit_inline, nfa-someday
Cc: eric@…, schlaber@…, anthony@…, semente@…, msaelices@…, upadhyay@…, ewalstad@…, jgelens@…, andy@…, ramusus@…, Seemant Kulleen, glicerinu@…, Mikhail Korobov, rohit.aggarwal@…, jdchrist@…, carsten.fuchs@…, Ben Stähli, patrick, cmawebsite@…, riccardo.magliocchetti@…, toracle@…, ramezashraf@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

It would be nice if there is a property to tell the admin interface to collapse the related object.

eg:
organisatie = meta.ForeignKey(Organisatie, edit_inline=meta.STACKED, edit_inline_style='collapse')

I changed line 610 in the django/views/admin/main.py to
t.append('<fieldset class="module%s">\n' % ((rel_field.rel.edit_inline != meta.TABULAR) and ' aligned collapse' or ' collapse'))
and it works. But using a property would be better.

Thanks

Attachments (11)

494.patch (4.2 KB ) - added by Antonis Christofides <anthony@…> 16 years ago.
Patch with alternative approach
494.2.patch (4.2 KB ) - added by Antonis Christofides <anthony@…> 16 years ago.
Updated patch, fixing a documentation problem
494.3.patch (4.7 KB ) - added by Antonis Christofides 16 years ago.
Quick and dirty solution for brainstorming
494.4.patch (7.2 KB ) - added by Antonis Christofides 15 years ago.
Patch that fixes spotted errors
494.5.patch (7.3 KB ) - added by Jeffrey Gelens 15 years ago.
The patch didn't apply correctly anymore to trunk. Updated with the right offsets.
494.6.patch (7.7 KB ) - added by Antti Kaihola 15 years ago.
Updated offsets and doc file path for trunk r10680
494.7.patch (8.8 KB ) - added by ramusus 15 years ago.
2 differences from 494.6.path described below
494.r12351.diff (7.0 KB ) - added by ramusus 14 years ago.
works with 12351 rev in development trunk (updated)
494.r12661.diff (6.1 KB ) - added by Matthew Schinckel 14 years ago.
I've updated the patch so that it doesn't use an empty list as the default argument for classes: this doesn't necessarily do what you expect it to. A second or tertiary call to the same function will use the same list, even if you have added things to it in the meantime, meaning that classes may get added to other objects that were not supposed to. At this stage, that is unlikely anyway, but it is bad form to create a function definition with an empty list as the default argument to anything. This patch works with r12661.
494.r12835.diff (10.8 KB ) - added by Matthew Schinckel 14 years ago.
A version that actually adds the classes properly. Works with r12835, and StackedInlines.
494.r12873.diff (10.8 KB ) - added by Matthew Schinckel 14 years ago.

Download all attachments as: .zip

Change History (86)

comment:1 by jctover@…, 19 years ago

Since I changed already a few things in the admin view for translation and I'm not experienced with svn yet here are my changes I made to create an inline_style property to make it collapse

in django/core/meta/fields.py line nr 585

  • raw_id_admin=kwargs.pop('raw_id_admin', False)) + raw_id_admin=kwargs.pop('raw_id_admin', False), inline_style=kwargs.pop('inline_style', None))

in django/core/meta/fields.py line nr 654

  • related_name=None, limit_choices_to=None, lookup_overrides=None, raw_id_admin=False): + related_name=None, limit_choices_to=None, lookup_overrides=None, raw_id_admin=False, inline_style=None):

in django/views/admin/main.py line 610

  • t.append('<fieldset class="module%s %s">\n' % ((rel_field.rel.edit_inline != meta.TABULAR) and ' aligned' or )) + t.append('<fieldset class="module%s %s">\n' % ((rel_field.rel.edit_inline != meta.TABULAR) and ' aligned' or , rel_field.rel.inline_style or ))

For using it just include inline_style='collapse' and the related objects are collapsed.
Hope you include a better solution to this problem

Johan

comment:2 by Jacob, 18 years ago

milestone: Version 1.0
Owner: changed from Adrian Holovaty to Jacob
Status: newassigned

comment:3 by jforcier@…, 18 years ago

Cc: jforcier@… added

comment:4 by mdt@…, 18 years ago

even the collapse in the related object has no effect. is that a bug?

comment:5 by anonymous, 18 years ago

Cc: jforcier@… removed

Removed myself from the Cc list (no longer that interested =))

comment:6 by URL, 18 years ago

Type: enhancement

comment:7 by eric@…, 18 years ago

Cc: eric@… added
Type: defect

comment:8 by (none), 17 years ago

milestone: Version 1.0

Milestone Version 1.0 deleted

comment:9 by Marc Fargas <telenieko@…>, 17 years ago

Wy don't you use the method provied by http://www.djangoproject.com/documentation/model_api/#id3?
It provides almost this, I think.

If it's enought for the reporter, please close the ticket as "worksforme" ;)

comment:10 by jcstover@…, 17 years ago

I didn't use django for a while, but from what I remember. For a tablular view of related objects you can collapse that view, but not when it is stacked. That was it about. If it is working for stacked objects now its okay.

comment:11 by Chris Beaven, 17 years ago

Summary: Collapse in admin interface for related objectsCollapse in admin interface for related objects using meta.STACKED

Can someone confirm this?

comment:12 by Marc Fargas <telenieko@…>, 17 years ago

Summary: Collapse in admin interface for related objects using meta.STACKEDCollapse in admin interface for related objects using models.STACKED
Triage Stage: UnreviewedDesign decision needed

Confirmed, it has to do with the way [source:django/trunk/django/contrib/admin/templates/admin/change_form.html] works, it can handle the "classes" attribute for fields (line 36), but not for "edit_inline" relations (stacked or inline) on line 57. And I could not find any easy way to get it working. Maybe [source:django/trunk/django/contrib/admin/templates/admin/edit_inline_stacked.html] should get the classes for the related objects but dunno how to pass it.

As a side note, it's models.STACKED not meta.STACKED right now, changing the summary to reflect this.

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

Keywords: edit_inline added

comment:14 by anonymous, 17 years ago

Summary: Collapse in admin interface for related objects using models.STACKEDCollapse in admin interface for inline related objects

comment:15 by Jacob, 17 years ago

Version: newforms-admin

-> newforms-admin

comment:16 by jkocherhans, 17 years ago

Owner: changed from nobody to xian
Status: assignednew

by Antonis Christofides <anthony@…>, 16 years ago

Attachment: 494.patch added

Patch with alternative approach

comment:17 by Antonis Christofides <anthony@…>, 16 years ago

I just uploaded a patch. You append a string with classes to edit_inline, and it uses these classes. For example:

edit_inline = models.TABULAR + 'collapse extrapretty'

by Antonis Christofides <anthony@…>, 16 years ago

Attachment: 494.2.patch added

Updated patch, fixing a documentation problem

comment:18 by Yves Serrano, 16 years ago

Has patch: set

comment:19 by Yves Serrano, 16 years ago

Has patch: unset

The patch fails on the newforms-admin branch.
The file structure has changed from
django/contrib/admin/templates/admin/edit_inline_stacked.html
to
django/contrib/admin/templates/admin/edit_inline/stacked.html

comment:20 by Brian Rosner, 16 years ago

Keywords: nfa-someday added

Tagging with nfa-someday as this is functionality that is not critical to the merge of the newforms-admin branch.

comment:21 by Brian Rosner, 16 years ago

Patch needs improvement: set

comment:22 by Antonis Christofides <anthony@…>, 16 years ago

Could you please point me to a description of how the newforms-admin branch is managed and what is the policy for patching admin/newforms-admin? Do I need to write two patches, one for old admin and one for new admin?

The second question is whether a better patch in the same spirit would be accepted.

comment:23 by mrts, 16 years ago

milestone: post-1.0

Non-essential for 1.0.

Antonis, you could write a similar patch against trunk when newforms-admin gets merged (and that is really soon now). It will be eventually merged, but probably not before 1.0.

comment:24 by Bernd, 16 years ago

Cc: schlaber@… added

comment:25 by Antonis Christofides, 16 years ago

Cc: anthony@… added

by Antonis Christofides, 16 years ago

Attachment: 494.3.patch added

Quick and dirty solution for brainstorming

comment:26 by Antonis Christofides, 16 years ago

I just uploaded another patch (not including documentation), as proof of concept. It works, but I've done it by doing quite some voodoo debugging. The idea is to add a 'classes' InlineModelAdmin option, with a list of classes. For example:

class BookInline(admin.TabularInline):
    model = Book
    classes = ['collapse', 'extrapretty']

If we like this interface, then please review my patch and help me find out what I didn't get right. However, I think that ideally I'd like another interface: the "fieldsets" ModelAdmin option should be reworked in order to somehow include inline objects. This would also give the flexibility to change the order, allowing them to intermingle with other fieldsets, and so on. But this also means that some conceptual changes should be made, because inline objects are not fieldsets, they are formsets. The "fieldsets" option might need to be renamed to "formsections" or so and its structure enhanced or modified.

comment:27 by Antonis Christofides, 15 years ago

Note: Apparently the patch I submitted only works if the model contains another fieldset that specifies "classes".

comment:28 by Guilherme M. Gondim <semente@…>, 15 years ago

Cc: semente@… added

comment:29 by Manuel Saelices, 15 years ago

Cc: msaelices@… added

comment:30 by amitu, 15 years ago

Cc: upadhyay@… added

comment:31 by Chris Beaven, 15 years ago

Has patch: set
Needs documentation: set
Triage Stage: Design decision neededAccepted

aptiko's patch looks good. It adds useful functionality and I like the addition of a fieldset to the StackedInline template - I always thought it should have it for consistency.

The patch does incorrectly close <fieldset> with </div>.
It also needs to change generic_inlineformset_factory in django.contrib.contenttypes.generic.

comment:32 by Antonis Christofides, 15 years ago

Owner: changed from xian to Antonis Christofides
Status: newassigned

SmileyChris, thanks for reviewing my patch. I'm working on it.

by Antonis Christofides, 15 years ago

Attachment: 494.4.patch added

Patch that fixes spotted errors

comment:33 by Antonis Christofides, 15 years ago

Needs documentation: unset
Patch needs improvement: unset

I just uploaded an updated patch, with the following differences from the previous one:

  1. Previous patch would only work if the model admin contained another fieldset that specified "classes". I fixed this.
  2. It fixes the incorrect closing of <fieldset> with </div>.
  3. It includes the necessary changes in generic_inlineformset_factory.
  4. It includes documentation.

Please note, however, that I have not tested generic_inlineformset_factory. I merely made the change and checked that the file compiles.

comment:34 by __ew__, 15 years ago

Cc: ewalstad@… added

by Jeffrey Gelens, 15 years ago

Attachment: 494.5.patch added

The patch didn't apply correctly anymore to trunk. Updated with the right offsets.

comment:35 by Jeffrey Gelens, 15 years ago

Cc: jgelens@… added

comment:36 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:37 by Andy Baker, 15 years ago

Cc: andy@… added

comment:38 by mrts, 15 years ago

Keywords: gsoc09-admin-refactor added

comment:39 by Jacob, 15 years ago

Keywords: gsoc09-admin-refactor removed

by Antti Kaihola, 15 years ago

Attachment: 494.6.patch added

Updated offsets and doc file path for trunk r10680

comment:40 by Alex Gaynor, 15 years ago

Version: newforms-adminSVN

classes should not be present on formsets.

comment:41 by Alex Gaynor, 15 years ago

Patch needs improvement: set

comment:42 by Alex Gaynor, 15 years ago

Needs tests: set

in reply to:  40 comment:43 by Antti Kaihola, 15 years ago

Replying to Alex:

classes should not be present on formsets.

Do you mean that the API of the patch is good, but it shouldn't touch django.forms and django.contrib.contenttypes?

comment:44 by ramusus, 15 years ago

Cc: ramusus added

by ramusus, 15 years ago

Attachment: 494.7.patch added

2 differences from 494.6.path described below

comment:45 by ramusus, 15 years ago

.... added property

"classes": self.classes 

to the defaults dictionary inside GenericInlineModelAdmin for activating ability to collapse generic inline admin forms.

And also added ability to define closed by default fieldset without any stumble while page is loading this way:

class BookInline(admin.TabularInline):
    model = Book
    classes = ['collapse', 'collapsed']

comment:46 by ramusus, 15 years ago

Cc: ramusus@… added; ramusus removed

comment:47 by Seemant Kulleen, 15 years ago

Cc: Seemant Kulleen added
Resolution: fixed
Status: assignedclosed

comment:48 by Seemant Kulleen, 15 years ago

Resolution: fixed
Status: closedreopened

comment:49 by anonymous, 14 years ago

I didn't use django for a while, but from what I remember. 
For a tablular view of related objects you can collapse that 
view, but not when it is stacked. That was it about. If it is
working for stacked objects now its okay.


How can I do that? (aka, is there a work around for doing this w/o patching code?)

comment:50 by anonymous, 14 years ago

I patched manually the current svn with the 494.7 patch, but it seems that no longer work. Why doesn't submit to milestone?

by ramusus, 14 years ago

Attachment: 494.r12351.diff added

works with 12351 rev in development trunk (updated)

by Matthew Schinckel, 14 years ago

Attachment: 494.r12661.diff added

I've updated the patch so that it doesn't use an empty list as the default argument for classes: this doesn't necessarily do what you expect it to. A second or tertiary call to the same function will use the same list, even if you have added things to it in the meantime, meaning that classes may get added to other objects that were not supposed to. At this stage, that is unlikely anyway, but it is bad form to create a function definition with an empty list as the default argument to anything. This patch works with r12661.

by Matthew Schinckel, 14 years ago

Attachment: 494.r12835.diff added

A version that actually adds the classes properly. Works with r12835, and StackedInlines.

by Matthew Schinckel, 14 years ago

Attachment: 494.r12873.diff added

comment:51 by Adam Nelson, 14 years ago

Patch needs improvement: unset

The patch itself seems to satisfy people's comments, but tests are still needed.

comment:52 by Alex Gaynor, 14 years ago

For the record (and I thought I'd left this comment before, but apparently not) I'm opposed to formsets having some knowledge of "classes", there's no precedent for it in the forms API, and it further munges the output with the server side issues.

comment:53 by Adam Nelson, 14 years ago

Patch needs improvement: set

In that case, reverting needs_better_patch

comment:54 by Marc Aymerich, 13 years ago

Cc: glicerinu@… added

comment:55 by anonymous, 13 years ago

Cc: Mikhail Korobov added

in reply to:  52 comment:56 by Matthew Schinckel, 13 years ago

Replying to Alex:

For the record (and I thought I'd left this comment before, but apparently not) I'm opposed to formsets having some knowledge of "classes", there's no precedent for it in the forms API, and it further munges the output with the server side issues.

In hindsight, I think I agree. I'm not sure if I got that code from someone else, and just updated it, but I was definitely following a pattern of some sort.

Having said that, BoundField in django.forms.forms.py contains a method css_classes, which looks for a property called extra_classes, which it uses to add classes to a field element. Why could the same not apply to a Form?

comment:57 by Marti Raudsepp, 13 years ago

Cc: Marti Raudsepp added

comment:58 by anonymous, 13 years ago

Cc: rohit.aggarwal@… added

comment:59 by jdchrist@…, 13 years ago

Cc: jdchrist@… added

comment:60 by Łukasz Rekucki, 13 years ago

Severity: normalNormal
Type: defectNew feature

comment:61 by Carsten Fuchs, 13 years ago

Cc: carsten.fuchs@… added
Easy pickings: unset

comment:62 by Ben Stähli, 13 years ago

Cc: Ben Stähli added

comment:63 by anonymous, 13 years ago

UI/UX: set

comment:64 by patrick, 11 years ago

Cc: patrick added

comment:65 by Aymeric Augustin, 11 years ago

Status: reopenednew

comment:66 by Collin Anderson, 11 years ago

Cc: cmawebsite@… added

comment:67 by rm_, 11 years ago

Cc: riccardo.magliocchetti@… added

comment:68 by Karen Tracey, 8 years ago

Owner: changed from Antonis Christofides to Karen Tracey
Status: newassigned

comment:69 by Karen Tracey, 8 years ago

Needs tests: unset
Owner: Karen Tracey removed
Patch needs improvement: unset
Status: assignednew

comment:70 by Jeongsoo, Park, 8 years ago

Cc: toracle@… added

comment:71 by Tim Graham, 8 years ago

comment:72 by Ramez Issac, 8 years ago

Cc: ramezashraf@… added

comment:73 by Marti Raudsepp, 8 years ago

Cc: Marti Raudsepp removed

comment:74 by Tim Graham <timograham@…>, 8 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In 5399ccc0:

Fixed #494 -- Added ability to specify classes on admin inline fieldsets.

This includes the ability to collapse inlines by specifying a class named
'collapse'.

comment:75 by Tim Graham <timograham@…>, 8 years ago

In 99d2469e:

Refs #494 -- Fixed a flaky admin_inlines tests.

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