Fix admin widget in django 3.0 #137
Conversation
|
Also fixes #136 |
| try: | ||
| from django.contrib.admin.templatetags import admin_static | ||
| except ImportError: | ||
| class admin_static: | ||
| def static(path): | ||
| return path |
There was a problem hiding this comment.
It seems to me that rewriting something like this would be better:
try:
from django.templatetags.static import static
except ImportError:
# compatibility with django < 2.1
from django.contrib.admin.templatetags.admin_static import static
There was a problem hiding this comment.
Yes, I see that the class definition is not the most elegant solution. However, I think using templatetags in python code should be avoided as well.
According to https://docs.djangoproject.com/en/3.0/topics/forms/media/#paths-in-asset-definitions it seems that form media assets paths will be prefixed correctly. Therefore just using the plain path would be nicer.
The class was necessary to satisfy the following conditions:
- Use plain paths, without templatetags (at least for newer code)
- Keeping the code backwards compatible
- Only introduce changes around the import lines of the file (non-intrusive)
I think the best solution would be to eliminate the use of templatetags everywhere. But that would be more intrusive.
Anyway, I think we can close the issue because @payamnj merged a solution already.
There was a problem hiding this comment.
@saemideluxe I just liked your solution and removed my PR.
|
In fact, with adding support for django 3.0, we need to drop support for Py2.
This PR is completely unprepared for a merge. Also, django 3.0 docs:
|
|
I completely agree with @Mogost Adding Django 3 to the test matrix should hopefully show all the things that need to be considered. It would we great if this PR could be enhanced to add the missing pieces for a release that is compatible with Django 3.0. For me it would be okay to follow easy-thumbnails and issue a release that drops support for Django < 2.2. If I see it correctly that would also render the initial issue in this PR obsolete. |
Sorry, the title must have been a bit misleading (will change that in a minute). My intention was not to migrate the project to django 3.0 but to fix the issue with the admin-widget in django 3.0. @anrie Right now I think I have no time to work on this on my own. I quickly tried tox with django30 and at least one other issue comes up (which is related to django-webtest). Is there a way other people can contribute to a PR? @Mogost could update the tox/travis configuration which I am not used to. Then we could work on fixing the remaining issues, I guess it is mostly dependency issues. |
|
In fact, we are not in a hurry, in any case, the release with Django 3.0 support of easy-thumbnails has not yet taken place at the moment. |
|
#139 |
|
I merged @Mogost compatibility branch and issued a new release: https://pypi.org/project/django-image-cropping/ Hope that solves all the issues :-) |
This is a small patch in order to allow using the admin widget with django 3.0. The intention was to change as little as possible while keeping backward compatibility.