Opened 6 years ago

Closed 15 months ago

#15760 closed New feature (fixed)

Feature: JS Hooks for Dynamic Inlines

Reported by: Mark Lavin Owned by: Tim Graham <timograham@…>
Component: contrib.admin Version: master
Severity: Normal Keywords: inlines jquery callback
Cc: dev@…, Jaap Roes, cmawebsite@…, Giorgos Stratakis, Ramez Issac Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

It is difficult to access the newly added row when working with dynamic inlines. This makes it difficult to use widgets which require javascript bindings (such as auto-complete widgets) in the inlines. I believe this is the issue that someone was trying to raise in #15693. While the formset plugin does allow for passing method to be called when a new row is added, this is always already populated by admin/edit_inline/stacked.html and admin/edit_inline/tabular.html. If you want to include another method to be run you need to override these templates completely.

I've attached a patch which adds two events to the formset plugin: formsetadd and formsetdelete which are fired by the add and delete rows respectively. The example usage would be

django.jQuery('.add-row a').live('formsetadd', function(e, row) {
    console.log('Formset add!');
    console.log($(row));
});

Attachments (1)

inlines.diff (639 bytes) - added by Mark Lavin 6 years ago.
Admin inlines with events

Download all attachments as: .zip

Change History (28)

Changed 6 years ago by Mark Lavin

Attachment: inlines.diff added

Admin inlines with events

comment:1 Changed 6 years ago by Mark Lavin

Needs documentation: set

comment:2 Changed 6 years ago by Julien Phalip

Triage Stage: UnreviewedAccepted

comment:3 Changed 6 years ago by Brillgen Developers

Cc: dev@… added
Easy pickings: unset

comment:4 Changed 5 years ago by Julien Phalip

UI/UX: set

comment:5 Changed 5 years ago by Jaap Roes

Cc: Jaap Roes added

comment:6 Changed 5 years ago by anonymous

Keywords: callback added
Owner: changed from nobody to Mark Lavin

comment:7 Changed 5 years ago by Mark Lavin

Owner: Mark Lavin deleted

comment:8 Changed 5 years ago by Collin Anderson

Cc: cmawebsite@… added

I ran into this issue two years ago, and I am running into it again today. I ended up having to clone django's add link, hide the original, add my own click handler, then call django's click handler on the original element.

It looks like a few others have ran into this issue too.

I believe the posted patch would solve the problem for me, maybe put a django namespace prefix on it just to be safe.

Another option would be to globally add a callback or override options.added and options.removed.

Last edited 5 years ago by Collin Anderson (previous) (diff)

comment:9 Changed 5 years ago by Mark Lavin

I ended up working around this issue in my own project by monkey patching django.jQuery.fn.formset. I still think that adding these events would be helpful but I think that https://github.com/django/django/pull/24 largely reduces the need for them if it is accepted.

comment:10 Changed 5 years ago by Collin Anderson

Ahh, yes, #18241 would also solve this for me.

comment:11 Changed 4 years ago by Michał Pasternak

I like the inlines.diff patch, because it is more explicit this way. I don't think there's anything wrong with having clearly named events on the javascript side, is there?

comment:12 Changed 4 years ago by Per Rosengren

Needs documentation: unset

Updated this patch to current master, with some alterations. Added documentation.

Patch: https://github.com/django/django/pull/542

This fix also fixes accepted duplicate #19314

Example usage:

django.jQuery(document).bind('formset_add.admin.my_widget', function(event, row_element) {
    my_widget_setup($('.my_widget', row_element));
});

comment:13 Changed 4 years ago by Julien Phalip

Patch needs improvement: set

Thanks for your work on the patch.

In the example usage above, you bind the event 'formset_add.admin.my_widget'. Is that a typo (i.e. the 'my_widget' part)?

Also, what would happen when the page would contain multiple types of inlines? How would you discriminate between the various inlines? If you could please elaborate on the approach a developer would have to follow, that would be helpful — and in fact, this could also be added to the documentation.

Finally, to include this patch in Django core, some tests would have to be written. Would you like to have a go at writing some Selenium tests demonstrating the use of this new feature?

Thanks!

comment:14 Changed 3 years ago by Giorgos Stratakis

Cc: Giorgos Stratakis added

comment:15 Changed 18 months ago by Ramez Issac

How About to extend the current plugin defaults to accept say 'addedCallback' and 'deletedCalback' which can be assigned in to the django.jQuery.fn.formset.defaults . Those options are left for the user to fill.
The Tabular & Stacked inline js can callback those function along with their 'added' and 'deleted' function

And as answer to julien question about what if many inline existed on a page , we can send both the created form and the formset name.

For using this new feature, a change_form template is extended for the model you want to work with, and we add the javascript function of desire inside the HTML template. Mainly in the change_form_document_ready block or maybe a new block (where missing block.super is not gonna mess things up)

I can do this if idea is accepted.

Last edited 18 months ago by Ramez Issac (previous) (diff)

comment:16 Changed 18 months ago by Ramez Issac

Adding a formset defaults in the document_ready block is too late for the defaults to get picked up.
The plugin is Initiated by the edit_inline/tabular.html (or stacked) , at the end of inline_field_sets block

The other option i see is on the extrahead block

comment:17 Changed 18 months ago by Ramez Issac

Cc: Ramez Issac added

PR created 4883

comment:18 Changed 16 months ago by Ramez Issac

Needs documentation: set

Any Idea on where this feature documentation should go in the Django admin documentation.

Thanks.

Last edited 16 months ago by Ramez Issac (previous) (diff)

comment:19 Changed 16 months ago by Ramez Issac

Patch needs improvement: unset

comment:20 Changed 16 months ago by Tim Graham

Maybe a new page at docs/ref/contrib/admin/javascript.txt would make sense. Anyway, the content is more important than the location at this point.

comment:21 Changed 16 months ago by Ramez Issac

Indeed , thank you Tim.
I added an 'initial documentation' , there is a small problem in docs/ref/contrib/admin/javascript line 19 , I couldn't figure out how to make a proper link. :-)
Waiting for feedback regarding this and any other.

Best Regards.

Last edited 16 months ago by Ramez Issac (previous) (diff)

comment:22 Changed 16 months ago by Ramez Issac

Needs documentation: unset

comment:23 Changed 15 months ago by Tim Graham

Patch needs improvement: set

comment:24 Changed 15 months ago by Ramez Issac

Patch needs improvement: unset

Added JavaScript tests along with documentation enhancement.

comment:25 Changed 15 months ago by Tim Graham

Patch needs improvement: set

Reviewed the updated patch.

comment:26 Changed 15 months ago by Ramez Issac

Patch needs improvement: unset

Improvements made. Kindly review. Thanks.

comment:27 Changed 15 months ago by Tim Graham <timograham@…>

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

In 1335aa2:

Fixed #15760 -- Added JavaScript events for admin inline forms.

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