Opened 5 years ago

Closed 5 years ago

#31028 closed Cleanup/optimization (fixed)

Use JavaScript classList API

Reported by: Jon Dufresne Owned by: nobody
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

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

Change History (6)

comment:2 by Claude Paroz, 5 years ago

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 FAQ entry which might be outdated by now.
Read also this django-developers thread. I think we should make it clearer in the docs before continuing the cleanup work.

comment:3 by Jon Dufresne, 5 years ago

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.

comment:4 by Claude Paroz, 5 years ago

Triage Stage: UnreviewedAccepted

Fair, if it is already used, then let's go ahead.

I created #31032 for the docs issue.

comment:5 by Claude Paroz, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:6 by Carlton Gibson <carlton.gibson@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 46a0edc3:

Fixed #31028 -- Used classList API to check, add and remove DOM classes.

Thanks to Claude Paroz for review.

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