jonasundderwolf / django-image-cropping

Django helper application to easily and non-destructively crop arbitrarily large images in admin and frontend.
Other
552 stars 131 forks source link

Don't load jQuery if IMAGE_CROPPING_JQUERY_URL is set None or "" or "None" #98

Closed ramast closed 8 years ago

ramast commented 8 years ago

Many users get confused and put IMAGE_CROPPING_JQUERY_URL = None in their python settings and since this is translated to javascript null and since null != "None" then the script assume its loading different version of jQuery and call noConflict(true). This fix address this problem

anrie commented 8 years ago

@ramast We currently test if setting IMAGE_CROPPING_JQUERY_URL to None prevents the inclusion of the jQuery that is bundled with image cropping. This the documented and intended behavior: https://github.com/jonasundderwolf/django-image-cropping/blob/master/README.rst#custom-jquery

ramast commented 8 years ago

@anrie, I know that's how it should work. For me, when I set IMAGE_CROPPING_JQUERY_URL to None the bundled jquery is still included. to stop the inclusion of bundled jquery, I must set IMAGE_CROPPING_JQUERY_URL to "None" not None.

this patch intends to fix this bug

anrie commented 8 years ago

Hm, okay. But then it would be good to have a test case that demonstrates the issue. Could you provide one along with your fix?

ramast commented 8 years ago

I agree. However since posting the fix we started switching away from this project because we want to use "imgcrop" javascript library for cropping. Therefore, I can't justify the time I'd spend implementing the test case.

anrie commented 8 years ago

Ah, do you have a link to imgcrop? Is it vanilla JS? Switching to a non jQuery solution would be interesting for django-image-cropping too.

ramast commented 8 years ago

Unfortunetly it still depends on jquery. The main reason we chose it, is because it offer more flexibility for thumbnail selection.

For example, lets say you have a square logo that you want to fit into a rectangle thumbnail. You can do it by zooming out the logo until it can fit the thumbnail, the empty space to the right and left logo will be filled by easythumbnails (black by default)

http://requtize.github.io/cropimg/

ramast commented 8 years ago

If you want to switch your project to the new library, please do let me know. I'd be happy to help with the migration. I'd rather avoid creating yet another django thumbnail app

anrie commented 8 years ago

@ramast Thanks for the offer. I have to investigate cropimg a bit closer but ultimately I'd prefer a non jQuery solution as this should simplify the code in a few places.