search

charettes / django-colorful

Extension to the Django web framework that provides database and form color fields
https://pypi.python.org/pypi/django-colorful
MIT License
169 stars 58 forks source link

Staticfiles support + South support + javascript issue fix #3

Closed kmike closed 13 years ago

kmike commented 13 years ago

Just noticed that js error occurs less often but is not properly fixed.

charettes commented 13 years ago

Great fixes, thanks kmike. Everything looks great but i'd like to have more details on the widget creation issues: DOMContentLoaded (aka jQuery.ready) should work just fine. Do you mind providing details? Maybe i can help tackling the issue.

kmike commented 13 years ago

I'm using this widget in django admin (with django-admin-tools installed) and sometimes it fails with this error: https://skitch.com/mikekorobovmike/fa6g5/line . Reloading page does help.

charettes commented 13 years ago

Hmm I use django-admin-tools also. I think it might have something to do with the two version of jQuery (contrib.admin and django-admin-tools) living together in the admin.

I see that django-admin tools loads a second version of jQuery asynchronously and we might fall in a race condition because of the (jQuery || django.jQuery).

What happens if you replace: (jQuery || django.jQuery) by (django.jQuery || jQuery) at https://github.com/charettes/django-colorful/blob/master/colorful/widgets.py#L32 ?

I think it might fix the issue.

2011/9/10 Mikhail Korobov < reply@reply.github.com>

I'm using this widget in django admin (with django-admin-tools installed) and sometimes it fails with this error: https://skitch.com/mikekorobovmike/fa6g5/line . Reloading page does help.

Reply to this email directly or view it on GitHub: https://github.com/charettes/django-colorful/pull/3#issuecomment-2058851

charettes commented 13 years ago

What happens if you replace (jQuery || django.jQuery) in widgets.py by (django.jQuery || jQuery)?

I think it should fix the issue.

kmike commented 13 years ago

Yes, this fix seems correct, I'm not able to reproduce the issue with (django.jQuery || jQuery). I'm not sure it will work if there is no 'django.jQuery' (e.g. in public site) and won't blow up with an exception in IE though.

charettes commented 13 years ago

You're right about the IE part. Let's do it 'django' in window ? django.jQuery : jQuery.

Do you mind committing the changes in this branch while back using $(document).ready? This way i could just pull your request with everything at the same time.

kmike commented 13 years ago

Done!