Admin DateTimeShortcuts performance (feature request / improvement suggestion)

[Fabian Büchler]

Hello Django developers,

as this is my first post here, let me shortly introduce myself:
I'm Fabian Büchler, webdeveloper from Vienna, Austria; relatively new to Django and just thinking about hacking on Django itsself ...
I've read all your contribution guidelines and I believe this to be the correct place for my concern - please correct me if I'm wrong.


I have a suggestion for improvement to the Django admin, namely the DateTimeShortcuts - especially in combination with model inlines.

The big picture
In a project I'm working on an event management tool for the Vienna tourist board and one of our models is heavily date based:
Event 1<n EventDate (used in a tabular inline within Event change forms).

Each EventDate has two date fields and two time fields, that means four date/time pickers per inline row.
When adding new rows using the "Add new row" link, this is already quite slow. The more inline rows are on the page, the slower it gets.

In addition to that, I've built a generator tool for these inlines which generates EventDates by requested patterns (per weekday, for a certain date range,...).
When generating more than a hand full of inline rows at a time, the browser slows down to a halt - continuous JS timeouts included.

All this happens on multiple, relatively new computers (all OSes), in the newest browsers (FF3.6, FF4b10,...).
After some JS profiling I found, that the problem seems to be burried within the reinitialization of all .datetimeshortcuts every time a new inline row is being added.
The processing of that takes about 95% of the overall JavaScript processing time.

My improvement suggestion
As a solution I would suggest to rewrite the complete DateTimeShortcuts.js using jQuery (it is not jQuery based at all atm.), especially jQuery's new (1.4.2) event delegation functionality [0] to improve performance. With event delegation, no reinitialization of newly added .datetimeshortcuts elements would be necessary at all.
As an addition, I think it would make sense to integrate the jQuery UI datepicker (maybe with a timepicker extension [1]) instead of the existing one, because that might resolve some problems, as with the datepicker opening outside of the viewport when the calendar icon is too far on the right side of the screen.

I am willing to try to implement this myself (and, of course, provide any outcomes to the community).
But before starting, I first wanted to hear your opinions to my suggestion and - if accepted - any pointers to how you would like this to be implemented.

Please, feel free to ask for more information about what exactly I'm doing. I could also set up a little demo page to illustrate the situation.

Looking forward to any input and feedback!
Regards,
Fabian

[0] http://api.jquery.com/delegate/
[1] http://trentrichardson.com/examples/timepicker/

[Russell Keith-Magee]

On Fri, Jan 28, 2011 at 10:18 PM, Fabian Büchler
<fabian....@gmail.com> wrote:
> Hello Django developers,
>
> as this is my first post here, let me shortly introduce myself:
> I'm Fabian Büchler, webdeveloper from Vienna, Austria; relatively new to
> Django and just thinking about hacking on Django itsself ...
> I've read all your contribution guidelines and I believe this to be the
> correct place for my concern - please correct me if I'm wrong.

You've found the right place - and welcome!

For some background -- Django's admin started as a hand-crafted
Javascript solution -- mostly because it predates the existence of
jQuery as a widely accpted javascript framework. We only added jQuery
to the admin around a year ago.

As a result, there is an admin javascript cleanup needed. Updating
date/time handling is part of that.

The best way to approach this is to Just Do It. Pick some specific
aspect of the Javascript where you think a jQuery-based cleanup is
called for, prepare a patch, then open a ticket and upload your patch
to that ticket.

Try to constrain the scope of your changes if you can. Don't just drop
a "I've fixed all your Javascript" patch on our laps -- if you start
smaller, it's a lot easier for us to give feedback.

As for using JQueryUI -- we don't currently have that dependency, so
you'll have to make your case for why we should add the new
dependency. If you can show a sample implementation that saves X% of
code, or is Y% faster, or has new features ABC, that will go a long
way to making your case.

So - thanks for offering to help out! Can't wait to see the
improvements you can make.

Yours,
Russ Magee %-)

[Fabian Büchler]

Thanks for the super fast reply, Russ!

the way you put it makes sense to me, so I guess I'm going to start over with replacing just the DateTimeShortcuts script and trying to use that from within the inlines script.

Regards,
Fabian

2011/1/28 Russell Keith-Magee <rus...@keith-magee.com>

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.

[Ramiro Morales]

Fabian,

On Jan 28, 11:18 am, Fabian Büchler <fabian.buech...@gmail.com> wrote:
> [...]

> As an addition, I think it would make sense to integrate the jQuery UI
> datepicker (maybe with a timepicker extension [1]) instead of the existing
> one, because that might resolve some problems, as with the datepicker
> opening outside of the viewport when the calendar icon is too far on the
> right side of the screen.
>

While your jQuery UI dependency inclusion proposal is discussed I'd
like to
focus in the specific datepicker calendar outside of viewport problem
you describe.

We fixed a related problem regarding negative vertical positions in
[1]r15143
closing ticket #11414. You seem to be proficient in JavaScript and
hopefully have
a way to reproduce the issue so if you can identify the exact problem
maybe we
can fix it in the short term. Please feel free to open a ticket with
your findings,
bonus points for a patch fixing it :)

Thanks for taking the time to help us with all those issues and
enhancements.

Regards,

--
Ramiro

1. http://code.djangoproject.com/changeset/15143
2. http://code.djangoproject.com/ticket/11414

[Fabian Büchler]

2011/2/7 Ramiro Morales <cra...@gmail.com>

Fabian,

On Jan 28, 11:18 am, Fabian Büchler <fabian.buech...@gmail.com> wrote:
> [...]

> As an addition, I think it would make sense to integrate the jQuery UI
> datepicker (maybe with a timepicker extension [1]) instead of the existing
> one, because that might resolve some problems, as with the datepicker
> opening outside of the viewport when the calendar icon is too far on the
> right side of the screen.
>

While your jQuery UI dependency inclusion proposal is discussed I'd
like to
focus in the specific datepicker calendar outside of viewport problem
you describe.

We fixed a related problem regarding negative vertical positions in
[1]r15143
closing ticket #11414. You seem to be proficient in JavaScript and
hopefully have
a way to reproduce the issue so if you can identify the exact problem
maybe we
can fix it in the short term. Please feel free to open a ticket with
your findings,
bonus points for a patch fixing it :)

 
Hej Ramiro,

yesterday I've created a ticket #15231 [1] containing a patch that also includes a fix for the horizontal out-of-viewport problem.
In the attached patch [2], have a look at line 324 of datetimeshortcuts.js

For now I have rewritten the whole DateTime widget things without jQuery UI and I guess that will do for now.

Regards,
Fabian

[1] http://code.djangoproject.com/ticket/15231
[2] http://code.djangoproject.com/attachment/ticket/15231/patch_admin_datetimeshortcuts.diff

Thanks for taking the time to help us with all those issues and
enhancements.

Regards,

--
Ramiro

1. http://code.djangoproject.com/changeset/15143
2. http://code.djangoproject.com/ticket/11414

--

[Fabian Büchler]

Just wanted to point you to the ticket I've opened for this topic: http://code.djangoproject.com/ticket/15231
Since the patch is in for two months now, I thought it was ok to kindly ask if someone was willing to do a review on this.

Happy about every review and improvement suggestion!

Thanks and regards,
Fabian

2011/1/28 Fabian Büchler <fabian....@gmail.com>