[Django] #33328: Use native JS events to trigger 'formset:added'/'formset:removed'
Django
------------------------------------------------+------------------------
Reporter: Claude Paroz | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33328>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Claude Paroz | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:1>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by SwastikTripathi):
* owner: nobody => SwastikTripathi
* status: new => assigned
Comment:
I'll work on this. Converting jquery functions into plain javascript
should be fun
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:2>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
Compatibility issues are not always fun :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:3>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by SwastikTripathi):
Replying to [comment:3 Claude Paroz]:
> Compatibility issues are not always fun :-)
Started working on it today, and I can say, you're right : )
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:4>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by SwastikTripathi):
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}}}?
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:5>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
It is difficult to answer your question without knowing your strategy here
(maybe show some preliminary code or PR?).
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:6>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by SwastikTripathi):
Oh, of course😄. Here's the PR
https://github.com/django/django/pull/15181)
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:7>
Django
Reporter: Claude Paroz | Owner:
Type: | SwastikTripathi
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by SwastikTripathi):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:8>
Django
Reporter: Claude Paroz | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by SwastikTripathi):
* owner: SwastikTripathi => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:9>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* owner: (none) => Claude Paroz
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:10>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15233 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:11>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:12>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:13>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by lb-):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:14>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:15>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:16>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by GitHub <noreply@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"eabc22f919e6c1774842e628400b87ac56c47bce" eabc22f9]:
{{{
#!CommitTicketReference repository=""
revision="eabc22f919e6c1774842e628400b87ac56c47bce"
Fixed #33328 -- Transformed formset:added/removed to native JS events.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:17>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson <carlton@…>):
In [changeset:"981615c6b55bdd2860c6281d4521627de90274dd" 981615c]:
{{{
#!CommitTicketReference repository=""
revision="981615c6b55bdd2860c6281d4521627de90274dd"
Refs #33328 -- Added some advice regarding handling formset:added/removed
in 3rd party libraries
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:18>
Django
Reporter: Claude Paroz | Owner: Claude
Type: | Paroz
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"fe7cb345448822528ce0447141810c27f9e56cff" fe7cb345]:
{{{
#!CommitTicketReference repository=""
revision="fe7cb345448822528ce0447141810c27f9e56cff"
Refs #33328 -- Corrected JS check for event.detail presence in docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33328#comment:19>