Opened 10 years ago

Last modified 2 years ago

#494 new New feature

Collapse in admin interface for inline related objects

Reported by: jcstover@… Owned by: aptiko
Component: contrib.admin Version: master
Severity: Normal Keywords: edit_inline, nfa-someday
Cc: eric@…, schlaber@…, anthony@…, semente@…, msaelices@…, upadhyay@…, ewalstad@…, jgelens@…, andy@…, ramusus@…, seemant, glicerinu@…, kmike, intgr, rohit.aggarwal@…, jdchrist@…, carsten.fuchs@…, benzkji, pztrick, cmawebsite@…, riccardo.magliocchetti@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
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@…> 8 years ago.
Patch with alternative approach
494.2.patch (4.2 KB) - added by Antonis Christofides <anthony@…> 8 years ago.
Updated patch, fixing a documentation problem
494.3.patch (4.7 KB) - added by aptiko 7 years ago.
Quick and dirty solution for brainstorming
494.4.patch (7.2 KB) - added by aptiko 7 years ago.
Patch that fixes spotted errors
494.5.patch (7.3 KB) - added by jgelens 7 years ago.
The patch didn't apply correctly anymore to trunk. Updated with the right offsets.
494.6.patch (7.7 KB) - added by akaihola 6 years ago.
Updated offsets and doc file path for trunk r10680
494.7.patch (8.8 KB) - added by ramusus 6 years ago.
2 differences from 494.6.path described below
494.r12351.diff (7.0 KB) - added by ramusus 6 years ago.
works with 12351 rev in development trunk (updated)
494.r12661.diff (6.1 KB) - added by schinckel 6 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 schinckel 5 years ago.
A version that actually adds the classes properly. Works with r12835, and StackedInlines.
494.r12873.diff (10.8 KB) - added by schinckel 5 years ago.

Download all attachments as: .zip

Change History (78)

comment:1 Changed 10 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 10 years ago by jacob

  • milestone set to Version 1.0
  • Owner changed from adrian to jacob
  • Status changed from new to assigned

comment:3 Changed 10 years ago by jforcier@…

  • Cc jforcier@… added

comment:4 Changed 9 years ago by mdt@…

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

comment:5 Changed 9 years ago by anonymous

  • Cc jforcier@… removed

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

comment:6 Changed 9 years ago by URL

  • Type enhancement deleted

comment:7 Changed 9 years ago by eric@…

  • Cc eric@… added
  • Type set to defect

comment:8 Changed 9 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

comment:9 Changed 9 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 9 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 9 years ago by SmileyChris

  • Summary changed from Collapse in admin interface for related objects to Collapse in admin interface for related objects using meta.STACKED

Can someone confirm this?

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

  • Summary changed from Collapse in admin interface for related objects using meta.STACKED to Collapse in admin interface for related objects using models.STACKED
  • Triage Stage changed from Unreviewed to Design 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 9 years ago by Gary Wilson <gary.wilson@…>

  • Keywords edit_inline added

comment:14 Changed 8 years ago by anonymous

  • Summary changed from Collapse in admin interface for related objects using models.STACKED to Collapse in admin interface for inline related objects

comment:15 Changed 8 years ago by jacob

  • Version set to newforms-admin

-> newforms-admin

comment:16 Changed 8 years ago by jkocherhans

  • Owner changed from nobody to xian
  • Status changed from assigned to new

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

Patch with alternative approach

comment:17 Changed 8 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 8 years ago by Antonis Christofides <anthony@…>

Updated patch, fixing a documentation problem

comment:18 Changed 8 years ago by yserrano

  • Has patch set

comment:19 Changed 8 years ago by yserrano

  • 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 8 years ago by brosner

  • 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 8 years ago by brosner

  • Patch needs improvement set

comment:22 Changed 7 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 7 years ago by mrts

  • milestone set to 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 7 years ago by Bernd

  • Cc schlaber@… added

comment:25 Changed 7 years ago by aptiko

  • Cc anthony@… added

Changed 7 years ago by aptiko

Quick and dirty solution for brainstorming

comment:26 Changed 7 years ago by aptiko

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 7 years ago by aptiko

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

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

  • Cc semente@… added

comment:29 Changed 7 years ago by msaelices

  • Cc msaelices@… added

comment:30 Changed 7 years ago by amitu

  • Cc upadhyay@… added

comment:31 Changed 7 years ago by SmileyChris

  • Has patch set
  • Needs documentation set
  • Triage Stage changed from Design decision needed to Accepted

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 7 years ago by aptiko

  • Owner changed from xian to aptiko
  • Status changed from new to assigned

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

Changed 7 years ago by aptiko

Patch that fixes spotted errors

comment:33 Changed 7 years ago by aptiko

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

  • Cc ewalstad@… added

Changed 7 years ago by jgelens

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

comment:35 Changed 7 years ago by jgelens

  • Cc jgelens@… added

comment:36 Changed 7 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:37 Changed 6 years ago by andybak

  • Cc andy@… added

comment:38 Changed 6 years ago by mrts

  • Keywords gsoc09-admin-refactor added

comment:39 Changed 6 years ago by jacob

  • Keywords gsoc09-admin-refactor removed

Changed 6 years ago by akaihola

Updated offsets and doc file path for trunk r10680

comment:40 follow-up: Changed 6 years ago by Alex

  • Version changed from newforms-admin to SVN

classes should not be present on formsets.

comment:41 Changed 6 years ago by Alex

  • Patch needs improvement set

comment:42 Changed 6 years ago by Alex

  • Needs tests set

comment:43 in reply to: ↑ 40 Changed 6 years ago by akaihola

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

  • Cc ramusus added

Changed 6 years ago by ramusus

2 differences from 494.6.path described below

comment:45 Changed 6 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 6 years ago by ramusus

  • Cc ramusus@… added; ramusus removed

comment:47 Changed 6 years ago by seemant

  • Cc seemant added
  • Resolution set to fixed
  • Status changed from assigned to closed

comment:48 Changed 6 years ago by seemant

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:49 Changed 6 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 6 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 6 years ago by ramusus

works with 12351 rev in development trunk (updated)

Changed 6 years ago by schinckel

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 5 years ago by schinckel

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

Changed 5 years ago by schinckel

comment:51 Changed 5 years ago by adamnelson

  • Patch needs improvement unset

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

comment:52 follow-up: Changed 5 years ago by 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.

comment:53 Changed 5 years ago by adamnelson

  • Patch needs improvement set

In that case, reverting needs_better_patch

comment:54 Changed 5 years ago by glic3rinu

  • Cc glicerinu@… added

comment:55 Changed 5 years ago by anonymous

  • Cc kmike added

comment:56 in reply to: ↑ 52 Changed 5 years ago by 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 5 years ago by intgr

  • Cc intgr added

comment:58 Changed 5 years ago by anonymous

  • Cc rohit.aggarwal@… added

comment:59 Changed 5 years ago by jdchrist@…

  • Cc jdchrist@… added

comment:60 Changed 4 years ago by lrekucki

  • Severity changed from normal to Normal
  • Type changed from defect to New feature

comment:61 Changed 4 years ago by CarstenF

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

comment:62 Changed 4 years ago by benzkji

  • Cc benzkji added

comment:63 Changed 4 years ago by anonymous

  • UI/UX set

comment:64 Changed 3 years ago by pztrick

  • Cc pztrick added

comment:65 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:66 Changed 2 years ago by CollinAnderson

  • Cc cmawebsite@… added

comment:67 Changed 2 years ago by rm_

  • Cc riccardo.magliocchetti@… added
Note: See TracTickets for help on using tickets.
Back to Top