search

jazzband / django-smart-selects

chained and grouped selects for django forms
https://django-smart-selects.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
1.1k stars 348 forks source link

Only use django jQuery #270

Closed leibowitz closed 4 years ago

leibowitz commented 5 years ago

The bindfields.js seems to fail to receive the formset:added event, probably because those events are triggered using the django.jQuery. https://docs.djangoproject.com/en/1.11/ref/contrib/admin/javascript/#inline-form-events

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 89.809% when pulling fa739507f1635f34bc5dc60f308f2b8c5b8dd23c on leibowitz:fix_jquery into ca55e0bc8ad378dc7a3d1bb5e68e18e8fc51c546 on digi604:master.

leibowitz commented 5 years ago

I just realised this is working as well simply by adding USE_DJANGO_JQUERY = True to the settings. Makes me wonder, what's the rational behind depending on a separate version of jquery and not re-using the django version? https://docs.djangoproject.com/en/2.1/ref/contrib/admin/#jquery

manelclos commented 4 years ago

@leibowitz I can imagine the setting is there because at times you would need a different jQuery version.

Can we close this PR?

leibowitz commented 4 years ago

Even if that breaks anything that relies on the formset:added and formset:removed events? I'm not sure it's a good idea to allow to customise the jQuery version, if that will lead to other issues. Or am I missing something?

manelclos commented 4 years ago

I agree. This can be a source of problems. If we apply your PR though, it also needs to contain removal of the two options that allow jQuery version customisation: USE_DJANGO_JQUERY and JQUERY_URL. And the documentation should state that these options have been removed since version x.y.z.

Could you please add that to the PR?

leibowitz commented 4 years ago

Sure, going to create a new PR as this is quite different https://github.com/jazzband/django-smart-selects/pull/301

Regarding this,

And the documentation should state that these options have been removed since version x.y.z.

It should appear in the changelog, but there's none. Time to create one?