Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Django 1.4 compatability and more #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

baldurthoremilsson
Copy link

I added compatability with Django 1.4 (ModelAdmin no longer has _media method, only media).

The URL to the jQuery UI JavaScript file is now generated by the staticfiles app if present. This makes it possible to use django-admin-jqueryui along with CachedStaticFilesStorage (or others) and get a proper URL for the JavaScript file.

I updated jQuery UI to 1.8.22. It looks like you have done this yourself, but it was not in the master branch, only in a tag. I also wrapped jQuery UI into a self-invoking function to keep jQuery inside the django namespace.

I didn' t touch setup.py, you should probably change the version to match the new jQuery UI :)

@piotrkilczuk
Copy link
Contributor

Thanks a lot, I have finally a chance to look at this...

@baldurthoremilsson
Copy link
Author

Great to see you haven't forgotten about the project :)

I looked at the code in the pull request again and realised that looking up the URL with the staticfiles app is redundant as the admin app already does that for all the media urls. The code in the pull request is also broken (it only includes the static function but never uses it - I must have forgotten that line in the interactive git add), so it's best to ignore that commit (66f7a34).

There is now a newer version of jQuery UI (v1.8.23) so you probably want to include that instead of the version in the pull request. I suggest you include it in a similar way, wrapped in a self invoking function to pass it the jQuery instance that comes with Django so people don't have to include another jQuery instance or make the Django jQuery instance global.

The Django 1.4 compatibility fix is still relevant though, can you merge only that part or do you need another pull request? Or you could just copy the changes and commit them yourself, it's only two lines of code...

@ALenfant
Copy link

This fixed Django 1.6 compatibility for me, it should really be merged! Thanks for your work baldurthoremilsson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants