#33328 closed Cleanup/optimization (fixed)
Use native JS events to trigger 'formset:added'/'formset:removed'
Reported by: | Claude Paroz | Owned by: | Claude Paroz |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The 'formset:added'/'formset:removed' events trigger in admin inlines.js are currently using jQuery trigger functionality, hence the events can not be catched by pure JS code.
I suggest to trigger those events with native JS events instead, so custom libs are not forced to use jQuery to catch those events. We've also been blocked by this on #30049.
However, the difference in passing complementary event data might make this move backwards incompatible. I'm not sure there is a clean migration path.
Change History (19)
comment:1 by , 3 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I'll work on this. Converting jquery functions into plain javascript should be fun
comment:4 by , 3 years ago
Replying to Claude Paroz:
Compatibility issues are not always fun :-)
Started working on it today, and I can say, you're right : )
comment:5 by , 3 years ago
I'm currently working in admin/static/js/inline.js
. Should I also change formset:added
and formset:removed
in admin/javascript.txt
, admin/inline.test.js
, admin/static/js/autocomplete.js
?
comment:6 by , 3 years ago
It is difficult to answer your question without knowing your strategy here (maybe show some preliminary code or PR?).
comment:7 by , 3 years ago
Oh, of course😄. Here's the PR ​https://github.com/django/django/pull/15181)
comment:8 by , 3 years ago
I'm really sorry for not communicating. Actually I've caught a cold, and require rest. So I don't think I'll be able to work on this now.
comment:9 by , 3 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:10 by , 3 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:12 by , 3 years ago
Needs tests: | set |
---|
comment:13 by , 3 years ago
Needs tests: | unset |
---|
comment:14 by , 3 years ago
Hi. Not sure if this is helpful but here is a library that allows for jQuery events to be converted to standard DOM events and visa versa.
​https://leastbad.com/jquery-events-to-dom-events
​https://www.npmjs.com/package/jquery-events-to-dom-events
I am sure there are others out there but this may be a good reference for a generic solution that still keeps the jQuery events present for backwards compatibility.
comment:15 by , 3 years ago
Thanks for the suggestion. I'm not sure that it's worth the complication as only one event is concerned. If someone wants to try, why not? As for me, I'd rather switch "brutally" and document the backwards incompatibility.
comment:16 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
This is in line with the previous moves away from the jQuery dependency, so +1.
Just imagining, one could listen for the native event and then dispatch a jQuery equivalent in the handler I would think. 🤔
Something to consider on review for the release notes.