-
-
Notifications
You must be signed in to change notification settings - Fork 351
Open
Description
Currently what is stated in the docs is not aligned with what the code does:
django-smart-selects/docs/settings.md
Lines 3 to 10 in 1f3ac7c
| `JQUERY_URL` | |
| : jQuery 2.2.0 is loaded from Google's CDN if this is set to `True`. If you would prefer to | |
| use a different version put the full URL here. Set `JQUERY_URL = False` | |
| to disable loading jQuery altogether. | |
| `USE_DJANGO_JQUERY` | |
| : By default, `smart_selects` loads jQuery from Google's CDN. However, it can use jQuery from Django's | |
| admin area. Set `USE_DJANGO_JQUERY = True` to enable this behaviour. |
django-smart-selects/smart_selects/widgets.py
Lines 16 to 51 in 1f3ac7c
| USE_DJANGO_JQUERY = getattr(settings, "USE_DJANGO_JQUERY", False) | |
| JQUERY_URL = getattr( | |
| settings, | |
| "JQUERY_URL", | |
| "https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js", | |
| ) | |
| URL_PREFIX = getattr(settings, "SMART_SELECTS_URL_PREFIX", "") | |
| class JqueryMediaMixin: | |
| @property | |
| def media(self): | |
| """Media defined as a dynamic property instead of an inner class.""" | |
| media = super(JqueryMediaMixin, self).media | |
| js = [] | |
| if JQUERY_URL: | |
| js.append(JQUERY_URL) | |
| elif JQUERY_URL is not False: | |
| vendor = "vendor/jquery/" | |
| extra = "" if settings.DEBUG else ".min" | |
| jquery_paths = [ | |
| "{}jquery{}.js".format(vendor, extra), | |
| "jquery.init.js", | |
| ] | |
| if USE_DJANGO_JQUERY: | |
| jquery_paths = ["admin/js/{}".format(path) for path in jquery_paths] | |
| js.extend(jquery_paths) | |
| media += Media(js=js) | |
| return media |
Also there are no tests which ensures this beaviour.
The issues are:
- If
JQUERY_URL=Trueit results in having aTrueas JS source, which ends up in an errore on frontend - It is not documented that to use
USE_DJANGO_JQUERYtheJQUERY_URLshould be a falsy value different fromFalse(ie: empty string, 0,None, etc...) - It is wrongly documented that using
JQUERY_URL=Truewill make this package use the Google CDN, while this works if the settings is not defined
Some tests over this feature are required and also the docs should be more accurate.
Metadata
Metadata
Assignees
Labels
No labels