Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#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 Carlton Gibson, 3 years ago

Triage Stage: Unreviewed → Accepted

This is in line with the previous moves away from the jQuery dependency, so +1.

However, the difference in passing complementary event data might make this move backwards incompatible. I'm not sure there is a clean migration path.

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.

comment:2 by SwastikTripathi, 3 years ago

Owner: changed from nobody to SwastikTripathi
Status: new → assigned

I'll work on this. Converting jquery functions into plain javascript should be fun

comment:3 by Claude Paroz, 3 years ago

Compatibility issues are not always fun :-)

in reply to:  3 comment:4 by SwastikTripathi, 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 SwastikTripathi, 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 Claude Paroz, 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 SwastikTripathi, 3 years ago

Oh, of course😄. Here's the PR ​https://github.com/django/django/pull/15181

Last edited 3 years ago by SwastikTripathi (previous) (diff)

comment:8 by SwastikTripathi, 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 SwastikTripathi, 3 years ago

Owner: SwastikTripathi removed
Status: assigned → new

comment:10 by Claude Paroz, 3 years ago

Owner: set to Claude Paroz
Status: new → assigned

comment:11 by Claude Paroz, 3 years ago

Has patch: set

comment:12 by Carlton Gibson, 3 years ago

Needs tests: set

comment:13 by Claude Paroz, 3 years ago

Needs tests: unset

comment:14 by LB (Ben Johnston), 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 Claude Paroz, 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 Carlton Gibson, 3 years ago

Triage Stage: Accepted → Ready for checkin

comment:17 by GitHub <noreply@…>, 3 years ago

Resolution: → fixed
Status: assigned → closed

In eabc22f9:

Fixed #33328 -- Transformed formset:added/removed to native JS events.

comment:18 by Carlton Gibson <carlton@…>, 3 years ago

In 981615c:

Refs #33328 -- Added some advice regarding handling formset:added/removed in 3rd party libraries

comment:19 by GitHub <noreply@…>, 3 years ago

In fe7cb345:

Refs #33328 -- Corrected JS check for event.detail presence in docs.

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