Fix admin images with Django 1.9 and multi usage of file picker widget #670
Conversation
|
+1 |
|
@acatton This looks like great stuff, thanks. What is the current status of this PR? |
|
FE LGTM |
|
@acatton Also, can you rebase this against latest develop please? |
|
@mkoistinen, @julianandrews or @gavinwahl is supposed to take over this pull request. |
julianandrews
force-pushed
the
misc-fixes-for-1.9
branch
from
ceb898e
to
d6aca26
Compare
|
@mkoistinen, @Flight I've cherry picked the relevant commits from #683 and rebased this PR on top of develop. I decided to make some minor changes to the logic from 3f7e610 to mesh better with the |
|
Hey @mkoistinen -- this was rebased a couple weeks ago, but now changes to develop have broken again. This is the 3rd or 4th time we've had to do this. Can we get some kind of confirmation that this will be merged immediately if we do it again? Thanks |
| win.close(); | ||
| }; | ||
| })(django.jQuery); | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
Here is left a line from merging.
stefanfoulis
changed the title
|
@acatton @julianandrews this PR is awesome and fixes much more than the title indicates. I took the liberty to change the title and updated the description to reflect everything it fixes. I'd like to merge it asap. I you rebase it again, I'll merge it right away. If you don't have time I'll try to tackle the rebase tomorrow morning (CET). sidenote: does the js refactor happen to also address #736 (clash is other widgets on the same page use jQuery)? |
|
@stefanfoulis full disclosure, I left the company and the project that was extensively using this. I was hoping to start my new job with this pull request being merged, but it didn't work out 😉 . This is why I might have been a little passive aggressive in my comments towards the end. And I would like to apologize for this 😅 . No worries, @julianandrews took over the project in question and this pull request as well. I see that julian merged the two pull requests. I didn't do that because I felt that they were unrelated work, even though #683 was based on this. But julian's the boss now. What this does is making the javascript a lot more robust. It's difficult to explain, but it is relying on the structure on the page more than on some internal state held in a javascript variable. To answer your question, @julianandrews would have to test it (because I don't have the setup to test it anymore) but it could very likely also fix #736. But i wouldn't like to set these expectations too high. |
|
@acatton thanks for the quick response. Hopefully @julianandrews can do the rebase. If not, that fine as well and I'll give it a shot. Anyhow, the help of everyone involved is much appreciated 👍 🏆 |
Explicit is better than implicit. LTE_DJANGO_1_9 was added. Rebased by Julian Andrews <jandrews@fusionbox.com>. If something's broken, it's probably his fault!
Rebased by Julian Andrews <jandrews@fusionbox.com>. If something's broken, it's probably his fault!
This code was fragile. A HashedFilenameFileStorage for example would have broken it. Rebased by Julian Andrews <jandrews@fusionbox.com>. If something's broken, it's probably his fault!
This was breaking git's diff
This fixes two bugs:
* The error message "Dropzone already attached" when having multiple
FileField on one admin page.
* Supporting FileField in inlines in Admin.
* Make the javascript less fragile
These are the changes:
* Don't use prefixed id, but relative object with classes.
* Use django.jQuery for everything
* Remove the useless closures, by replacing:
(function ($) { $(function () { ... } }(jQuery)
with:
jQuery(function ($) { ... })
* Remove bad practices:
* $elem.removeAttr('value') -> $elem.val('')
* var func = ...; $elem.off(event, func); // It was just created
* Remove bad flow. (Was sometimes using hide(), on other time
addClass('hidden'))
julianandrews
force-pushed
the
misc-fixes-for-1.9
branch
from
96f3c85
to
9a657b7
Compare
|
I think this should do it. This at least seems to fix #683. I'm not sure if it addresses #736. #743 seems to be unfixed; I'm not sure if that's somehow the result of the rebase. I have a good idea at this point how to go about fixing #743, so I'll take a look, but if possible I'd rather get this PR merged in, and I'll submit a much smaller PR if I come up with a good fix. |
|
awesome @julianandrews . Thank you very much for the quick response and rebase. It's late at night here, so I'll postpone the review and merge to tomorrow morning. |
Fix admin images with Django 1.9 and multi usage of file picker widget
This pull request is kind of a melting pot. I just wanted to fix the static files in the admin center for Django 1.9. But I ended up cleaning up some code as well, and fixing some other things.
DJANGO_X_Xinutils/compatibility.py.LTE_DJANGO_X_XEdit: I would strongly advised to review each commit one by one instead of reviewing the whole thing.