Opened 11 years ago

Closed 12 months ago

Last modified 11 months 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: master
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@…, benzkji, Patrick Paul, 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@…> 9 years ago.
Patch with alternative approach
494.2.patch (4.2 KB) - added by Antonis Christofides <anthony@…> 9 years ago.
Updated patch, fixing a documentation problem
494.3.patch (4.7 KB) - added by Antonis Christofides 8 years ago.
Quick and dirty solution for brainstorming
494.4.patch (7.2 KB) - added by Antonis Christofides 8 years ago.
Patch that fixes spotted errors
494.5.patch (7.3 KB) - added by Jeffrey Gelens 8 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 8 years ago.
Updated offsets and doc file path for trunk r10680
494.7.patch (8.8 KB) - added by ramusus 8 years ago.
2 differences from 494.6.path described below
494.r12351.diff (7.0 KB) - added by ramusus 7 years ago.
works with 12351 rev in development trunk (updated)
494.r12661.diff (6.1 KB) - added by Matthew Schinckel 7 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 7 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 7 years ago.

Download all attachments as: .zip

Change History (86)

comment:1 Changed 11 years ago by jctover@…

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 Changed 11 years ago by Jacob

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

comment:3 Changed 11 years ago by jforcier@…

Cc: jforcier@… added

comment:4 Changed 11 years ago by mdt@…

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

comment:5 Changed 11 years ago by anonymous

Cc: jforcier@… removed

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

comment:6 Changed 10 years ago by URL

Type: enhancement

comment:7 Changed 10 years ago by eric@…

Cc: eric@… added
Type: defect

comment:8 Changed 10 years ago by (none)

milestone: Version 1.0

Milestone Version 1.0 deleted

comment:9 Changed 10 years ago by Marc Fargas <telenieko@…>

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 Changed 10 years ago by jcstover@…

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 Changed 10 years ago by Chris Beaven

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

Can someone confirm this?

comment:12 Changed 10 years ago by Marc Fargas <telenieko@…>

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 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Keywords: edit_inline added

comment:14 Changed 9 years ago by anonymous

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

comment:15 Changed 9 years ago by Jacob

Version: newforms-admin

-> newforms-admin

comment:16 Changed 9 years ago by jkocherhans

Owner: changed from nobody to xian
Status: assignednew

Changed 9 years ago by Antonis Christofides <anthony@…>

Attachment: 494.patch added

Patch with alternative approach

comment:17 Changed 9 years ago by Antonis Christofides <anthony@…>

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'

Changed 9 years ago by Antonis Christofides <anthony@…>

Attachment: 494.2.patch added

Updated patch, fixing a documentation problem

comment:18 Changed 9 years ago by Yves Serrano

Has patch: set

comment:19 Changed 9 years ago by Yves Serrano

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 Changed 9 years ago by Brian Rosner

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 Changed 9 years ago by Brian Rosner

Patch needs improvement: set

comment:22 Changed 9 years ago by Antonis Christofides <anthony@…>

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

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

Cc: schlaber@… added

comment:25 Changed 8 years ago by Antonis Christofides

Cc: anthony@… added

Changed 8 years ago by Antonis Christofides

Attachment: 494.3.patch added

Quick and dirty solution for brainstorming

comment:26 Changed 8 years ago by Antonis Christofides

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

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

comment:28 Changed 8 years ago by Guilherme M. Gondim <semente@…>

Cc: semente@… added

comment:29 Changed 8 years ago by Manuel Saelices

Cc: msaelices@… added

comment:30 Changed 8 years ago by amitu

Cc: upadhyay@… added

comment:31 Changed 8 years ago by Chris Beaven

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

Owner: changed from xian to Antonis Christofides
Status: newassigned

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

Changed 8 years ago by Antonis Christofides

Attachment: 494.4.patch added

Patch that fixes spotted errors

comment:33 Changed 8 years ago by Antonis Christofides

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

Cc: ewalstad@… added

Changed 8 years ago by Jeffrey Gelens

Attachment: 494.5.patch added

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

comment:35 Changed 8 years ago by Jeffrey Gelens

Cc: jgelens@… added

comment:36 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:37 Changed 8 years ago by Andy Baker

Cc: andy@… added

comment:38 Changed 8 years ago by mrts

Keywords: gsoc09-admin-refactor added

comment:39 Changed 8 years ago by Jacob

Keywords: gsoc09-admin-refactor removed

Changed 8 years ago by Antti Kaihola

Attachment: 494.6.patch added

Updated offsets and doc file path for trunk r10680

comment:40 Changed 8 years ago by Alex Gaynor

Version: newforms-adminSVN

classes should not be present on formsets.

comment:41 Changed 8 years ago by Alex Gaynor

Patch needs improvement: set

comment:42 Changed 8 years ago by Alex Gaynor

Needs tests: set

comment:43 in reply to:  40 Changed 8 years ago by Antti Kaihola

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

Cc: ramusus added

Changed 8 years ago by ramusus

Attachment: 494.7.patch added

2 differences from 494.6.path described below

comment:45 Changed 8 years ago by ramusus

.... 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 Changed 7 years ago by ramusus

Cc: ramusus@… added; ramusus removed

comment:47 Changed 7 years ago by Seemant Kulleen

Cc: Seemant Kulleen added
Resolution: fixed
Status: assignedclosed

comment:48 Changed 7 years ago by Seemant Kulleen

Resolution: fixed
Status: closedreopened

comment:49 Changed 7 years ago by anonymous

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

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

Changed 7 years ago by ramusus

Attachment: 494.r12351.diff added

works with 12351 rev in development trunk (updated)

Changed 7 years ago by Matthew Schinckel

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.

Changed 7 years ago by Matthew Schinckel

Attachment: 494.r12835.diff added

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

Changed 7 years ago by Matthew Schinckel

Attachment: 494.r12873.diff added

comment:51 Changed 7 years ago by Adam Nelson

Patch needs improvement: unset

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

comment:52 Changed 7 years ago by Alex Gaynor

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 Changed 7 years ago by Adam Nelson

Patch needs improvement: set

In that case, reverting needs_better_patch

comment:54 Changed 6 years ago by Marc Aymerich

Cc: glicerinu@… added

comment:55 Changed 6 years ago by anonymous

Cc: Mikhail Korobov added

comment:56 in reply to:  52 Changed 6 years ago by Matthew Schinckel

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 Changed 6 years ago by Marti

Cc: Marti added

comment:58 Changed 6 years ago by anonymous

Cc: rohit.aggarwal@… added

comment:59 Changed 6 years ago by jdchrist@…

Cc: jdchrist@… added

comment:60 Changed 6 years ago by Łukasz Rekucki

Severity: normalNormal
Type: defectNew feature

comment:61 Changed 6 years ago by Carsten Fuchs

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

comment:62 Changed 6 years ago by benzkji

Cc: benzkji added

comment:63 Changed 6 years ago by anonymous

UI/UX: set

comment:64 Changed 4 years ago by Patrick Paul

Cc: Patrick Paul added

comment:65 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:66 Changed 4 years ago by Collin Anderson

Cc: cmawebsite@… added

comment:67 Changed 3 years ago by rm_

Cc: riccardo.magliocchetti@… added

comment:68 Changed 13 months ago by Karen Tracey

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

comment:69 Changed 13 months ago by Karen Tracey

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

comment:70 Changed 13 months ago by Jeongsoo, Park

Cc: toracle@… added

comment:71 Changed 13 months ago by Tim Graham

comment:72 Changed 13 months ago by Ramez Issac

Cc: ramezashraf@… added

comment:73 Changed 13 months ago by Marti

Cc: Marti removed

comment:74 Changed 12 months ago by Tim Graham <timograham@…>

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 Changed 11 months ago by Tim Graham <timograham@…>

In 99d2469e:

Refs #494 -- Fixed a flaky admin_inlines tests.

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