Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only use django jQuery #270

Closed
wants to merge 1 commit into from
Closed

Conversation

leibowitz
Copy link
Contributor

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
Copy link

Coverage Status

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

@leibowitz
Copy link
Contributor Author

leibowitz commented Aug 13, 2018

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
Copy link
Member

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

Can we close this PR?

@leibowitz
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

Sure, going to create a new PR as this is quite different
#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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants