Admin DateTimeShortcuts + Inlines performance

[Fabian Büchler]

In the admin, stacked or tabular inlines reinitialize the DateTimeShortcuts widgets for date or time fields whenever an inline row/form is added or removed.

This has serious performance problems if your inline has a couple of date/time fields per row.
About 95% of JS execution time is spent on the reinitialization of DateTimeShortcuts resulting in continious JS timeouts in even the most modern browsers.

A couple of days ago I've offered to fix this in the django-devleopers list:
​http://groups.google.com/group/django-developers/browse_thread/thread/ff460085a3bfb12e/

I've rewritten admin/DateTimeShortcuts.js and calendar.js and combinded them in a new datetimeshortcuts.js (+ minified version), which is jQuery based and uses 1.4.2 event delegation.
This way, inlines do not need to reinitialize the DateTimeShortcuts widgets at all.

The usage of the new datetimeshortcuts.js is as follows:

By default, all .fieldsets are dts-enabled by the following call at the bottom of datetimeshortcuts.js

  $("fieldset.module").datetimeshortcuts();

Users can use the plugin as well, for their own fields, specifing field selectors and enabling or disabling the today, calendar, now and clock widgets separately:

  $('my-selector').datetimeshortcuts({
      date_fields: "input:text.date_from, input:text.date_to",
      time_fields: "input:text.time_from, input:text.time_to",
      enable_calendar: true,
      enable_today: true,
      enable_clock: false,
      enable_now: false
  });

In the admin's base.html, I've defined a basic set of options for DateTimeShortcuts, which override the default options of datetimeshortcuts.js:

  $('body').data('datetimeshortcuts_defaults', {
      enable_calendar: true,
      enable_today: true,
      enable_clock: true,
      enable_now: true
  });

It would be great to add a variable to Django's settings to define these in your settings.py and include the values via a template tag in the base.html.
I don't know if you are willing to do this, so I didn't prepare it for this patch.

A patch is ready and attached.

[Andy Terra]

Personally, I couldn't get the admin media prefix to work until I made a small change to the javascript. Maybe there's something wrong with my setup, but now I have:

(function($) {
    
    $.fn.datetimeshortcuts = function(options) {
        // variables set in base.html
        var datetimeshortcuts_defaults = $('body').data('datetimeshortcuts_defaults');
        
        // extend default settings with user options
        var o = $.extend({
                // admin_media_prefix: $('body').data('admin_media_prefix')
                admin_media_prefix: window.__admin_media_prefix__
            },

        // (...)

And it finally works for me.

In any way, this is an excellent patch and I can see a lot of users benefiting from it right away. Hopefully we can make it to 1.3!

[Fabian Büchler]

Replying to airstrike:

Personally, I couldn't get the admin media prefix to work until I made a small change to the javascript. Maybe there's something wrong with my setup, but now I have:

   admin_media_prefix: $('body').data('admin_media_prefix'

the admin_media_prefix you refer to is being set in the base.html template. Before it was set as window.__admin_media_prefix__, with my patch it is $('bdoy').data('admin_media_prefix').

Can you check if the patch modified your base.html template or if you have overwritten it?

Otherwise, thanks for reviewing!

[Andy Terra]

I have indeed overwritten the base.html template. Actually, I just patched my own app to see if it would work, rather than starting a new app from the patch. Perhaps the case is that it's not particularly clear to me (and maybe other users) how to set admin_media_prefix The Right Way. window.admin_media_prefix is what I found googling around.

On a side note, +1 on making the move to jQuery UI. Readability and flexibility are two key items that come to mind as strong arguments in favor of that decision.

(Meanwhile, I'd love to use jQuery UI even if it never makes it to Django officially, so if you happened to make one for yourself (that you didn't submit as patch), please share! My javascript skills are lackluster at best, so adapting your code would take hours that I can't afford to spend at the moment.)

---

EDIT: missing a couple of words

[Fabian Büchler]

Replying to airstrike:

I have indeed overwritten the base.html template. Actually, I just patched my own app to see if it would work, rather than starting a new app from the patch. Perhaps the case is that it's not particularly clear to me (and maybe other users) how to set admin_media_prefix The Right Way. window.admin_media_prefix is what I found googling around.

Just for clarification:
The admin_media_prefix is being set in the base.html template, because that is the least likely to be overwritten (usually it is just being extended).
It is necessary to set this variable in a Django template, because there you can create the link between the variables known in Python code and the JavaScript which knows nothing about these variables.

Since I've rewritten the DateTimeShortcuts thingy using jQuery, I've also changed the way the admin_media_prefix variable is stored from window.__admin_media_prefix__ to jQuery('body').data('admin_media_prefix').

Maybe this would require some backwards compatibility fix - what do you think?

On a side note, +1 on making the move to jQuery UI. Readability and flexibility are two key items that come to mind as strong arguments in favor of that decision.

I'd vote for that too, but I'll better bring that topic up in the dev mailinglist some day.
The jQuery UI datepicker is propably far more advanced by now (in terms of browser compatibility, keyboard navigation, etc.) and the time-picker widget is far less useful than it could possibly be, imho.

(Meanwhile, I'd love to use jQuery UI even if it never makes it to Django officially, so if you happened to make one for yourself (that you didn't submit as patch), please share! My javascript skills are lackluster at best, so adapting your code would take hours that I can't afford to spend at the moment.)

I have not integrated jQuery UI by now because I wanted to make the move to jQuery first and separately, as I thought that would give the patch greater changes to be accepted. :)
Once a descision has been made to integrate it, I would definitely like to help.

[Fabian Büchler]

Update patch to add some minor improvements to calendar/clock closing behaviour (duplicate patch due to me forgetting to check "replace attachment, sorry).
Unset "Patch needs improvement" flag because airstrike's problems are based upon his overwritten base.html template.

[Fabian Büchler]

patch for current trunk (>= r16402)

[Fabian Büchler]

patch for 1.3 tag

[Fabian Büchler]

Just fixed a minor bug in the calendar popup.
Updated patches for 1.3 (patch_admin_datetimeshortcuts__1.3.diff) and current trunk (>= r16402, patch_admin_datetimeshortcuts.diff).

[Fabian Büchler]

Patch for 1.4 master

[Fabian Büchler]

Patch for 1.4 master

[Fabian Büchler]

Patch for stable/1.4.x

[Fabian Büchler]

Attached new patches for 1.4 stable branch and 1.4 master.

[anonymous]

Updated patch on request of apollo13 for current master (1.5+). See ​https://github.com/django/django/pull/618

[Tim Graham]

Patch no longer applies cleanly.