Installation of the jsTree library requires additional steps, downloading, renaming, etc. This is not very convenient especially for projects that are managed by the composer and don't store any contribs under VCS. The solution with composer.libraries.json is fine but still requires additional actions.
According to the official docs jsTree can be installed with CDNJS, example:
https://cdnjs.cloudflare.com/ajax/libs/jstree/3.3.10/themes/default/style.min.css
https://cdnjs.cloudflare.com/ajax/libs/jstree/3.3.10/jstree.min.js
So we can just update the media_directories_ui.libraries.yml file and connect library files directly from CDN. media_directories_ui_requirements() have to be removed because the library will be always available by default. The solution doesn't require any additional actions for an upgrade path, it will work for all with the new version of the module.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | fix_regression--3179504-10.patch | 1.3 KB | dom. |
| #10 | after_patch.png | 87.02 KB | dom. |
| #10 | before_patch.png | 87.22 KB | dom. |
| #6 | 3179504-use_cdn_if_library_not_installed-6.patch | 6.51 KB | ytsurk |
Comments
Comment #2
rang501 commentedHi!
Thanks for the idea. I do agree that initial setup should be as easy as possible.
CDN could be used as default, but the option to use local files should remain, there may be use cases where content security policy settings doesn't allow external sources, the site is a intranet instance (with no access to outside network), security requirements and so on.
Most likely there should be a setting to control this.
Comment #3
ytsurkAgree with rang501 - we can provide a CDN library which can be enabled in settings.
Comment #4
ytsurkComment #5
ytsurkThere you go
Comment #6
ytsurkThinking twice made me just use the CDN if the library is not installed.
So the module needs one step less after installation.
A warning is shown on the status report if served from CDN. It's kind of my personal preference, maybe information would be enough?
Comment #7
ytsurkComment #9
ytsurkI hate reviewing myself, and even more talking to myself.
Comment #10
dom. commentedSorry, the patch #6 introduces a regression: try moving a media to a folder, you will get a red cross while the operation is actually permitted.
The reason is because jsTree library is added AFTER this module JS files. But media_directories.jstree.js introduces global bindings because of #3176332: Drag and Drop Parent Folder into Subfolder see comment #13. Theses global bindings runs the binded methods in the order of bindings, thus we must run our code AFTER the library default code.
The patch is simple: it just changes the order of library inclusions.
after patch:
Comment #12
ytsurk