Opened 7 years ago

Closed 7 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, 7 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, 7 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, 7 years ago

Triage Stage: Unreviewed → Accepted

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

I created #31032 for the docs issue.

comment:5 by Claude Paroz, 7 years ago

Triage Stage: Accepted → Ready for checkin

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

Resolution: → fixed
Status: new → closed

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