search

mazelife / django-redactor

Project has been forked & renamed! See the README. Integrates the Redactor Javascript WYSIWYG editor in Django.
Other
63 stars 27 forks source link

Adding jquery to the media content may be wrong #12

Closed artscoop closed 11 years ago

artscoop commented 11 years ago

If you use Django admin, normally you already have jquery. Using AdminRedactorEditor imports jquery once again, which breaks every jQuery plugin on the page.

defbyte commented 11 years ago

Django admin ships with jQuery version 1.4.2, but it looks like Redactor needs version 1.7 minimum.

If Redactor can function in all capacity with jQuery 1.4.2, then yes, the jQuery import should be removed.

artscoop commented 11 years ago

Actually, several admin plugins may need versions greater than 1.4.2. If they all include different versions of jQuery in their js properties, nothing will work. I've hit this exact problem several times, and for all these plugins, I think it would be better to copy the admin page template and add the latest jquery in the head.

I've had to do this to prevent django-redactor from breaking Redactor pages. :/

defbyte commented 11 years ago

I feel your pain! I wish there was an implementation of the admin that smartly manages javascript dependencies. It would be really slick if the admin used require.js.

And with that, I agree, I think removing jquery media from AdminRedactorEditor would be a good idea, and shift the responsibility further up the line.

mazelife commented 11 years ago

Unfortunately Redactor won't work with jQuery 1.4.2. I'm reluctant to simply remove jQuery from the widget's media, though, because this would be a backwards-incompatible change for people who currently are using django-redactor and expect that it will just work without overriding specific admin templates.

It would be nice if the redactor authors did something along the lines of what Django does: putting jQuery within a "redactor" namespace. But they don't. However, I feel your pain and agree it would be a useful to be able to exclude jQuery from the media in cases where you want to handle javascript dependencies yourself. I don't think this should be the default, but I think it should be possible without forking this app. One solution I came up is something like this:

my_widget = RedactorEditor(no_jquery=True)

...in other words, exclude jQuery at widget initialization time. This would be very easy to implement and should meet your needs.

artscoop commented 11 years ago

Actually, what you did is nicer than just removing jQuery. Thank you !

mazelife commented 11 years ago

Ok, I've just pushed a fix for this (tagged v1.2.4). Apologies for the delay. I thought about the API a little bit and modified the language from what I originally proposed in the ticket to something--I think-- is a bit more easy-to-understand. 

my_widget = RedactorEditor(include_jquery=False)