[Django] #31028: Use JavaScript classList API
Django
------------------------------------------------+------------------------
Reporter: Jon Dufresne | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
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 |
------------------------------------------------+------------------------
Some JavaScript code does string matching with the `.className`. This can
be simplified with the newer `classList` API designed to add, remove, and
check for classes in HTML elements.
https://developer.mozilla.org/en-US/docs/Web/API/Element/classList
--
Ticket URL: <https://code.djangoproject.com/ticket/31028>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jon Dufresne | Owner: nobody
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/12139
--
Ticket URL: <https://code.djangoproject.com/ticket/31028#comment:1>
Django
Reporter: Jon Dufresne | Owner: nobody
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
Jon, I like your cleanup work a lot!
However, about the admin JavaScript code, I think we should more clearly
state the minimal browser version supported. A quick search revealed this
[https://docs.djangoproject.com/en/2.2/faq/admin/#what-browsers-are-
supported-for-using-the-admin FAQ entry] which might be outdated by now.
Read also [https://groups.google.com/forum/#!msg/django-
developers/MN9nz5rJODY/MAIOEVKCAQAJ this django-developers thread]. I
think we should make it clearer in the docs before continuing the cleanup
work.
--
Ticket URL: <https://code.djangoproject.com/ticket/31028#comment:2>
Django
Reporter: Jon Dufresne | Owner: nobody
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
Thanks. The `classList` API is already in use by the Django admin.
{{{
django/contrib/admin/static/admin/js/collapse.js
21 elem.classList.add('collapsed');
39 if (fieldset.classList.contains('collapsed')) {
42 fieldset.classList.remove('collapsed');
46 fieldset.classList.add('collapsed');
}}}
The commit where this was introduced is
ba83378a7762c51be235b521aa5b48233d6c6c82. This shipped with 2.2.
So this change is not introducing a new, untested API.
I think being more explicit about support is a great idea and a solid
improvement. I just question why it should guard this particular change.
--
Ticket URL: <https://code.djangoproject.com/ticket/31028#comment:3>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* stage: Unreviewed => Accepted
Comment:
Fair, if it is already used, then let's go ahead.
I created #31032 for the docs issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/31028#comment:4>
Django
Reporter: Jon Dufresne | Owner: nobody
Cleanup/optimization |
Component: contrib.admin | Version: master
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 Claude Paroz):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31028#comment:5>
Django
Reporter: Jon Dufresne | Owner: nobody
Cleanup/optimization |
Component: contrib.admin | Version: master
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 Carlton Gibson <carlton.gibson@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"46a0edc3ba6c40d685b9d128c5f6d8723f9659ca" 46a0edc3]:
{{{
#!CommitTicketReference repository=""
revision="46a0edc3ba6c40d685b9d128c5f6d8723f9659ca"
Fixed #31028 -- Used classList API to check, add and remove DOM classes.
Thanks to Claude Paroz for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31028#comment:6>