Update basset paths #44
Conversation
Change reference from icons to new font folder
|
Hey @arb362 , Thanks for the PR. In my network, both(/font and /icons) are working fine, which doesn't push me to merge the changes. Please check on your side by opening these links in the browser. Let me know if I missing something. |
|
Hey @arb362 thanks for the report and the PR. When I saw some people having the issue, others don't I figured out this had to be something sketchy. What's happening is that some people pull version So my suggestion is to pin the version on this URLs, so that we all get the same version. Can you change your PR's to include the version in the urls ? - https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme/Material/font/material.svg
+ https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme@3.0.0/Material/font/material.svgIf you don't have time/desire to do it, I will do the changes myself and merge it. Thanks again 🙏 |
|
Sure thing @pxpm! Thanks for checking into it. |
|
Thanks @arb362 I will merge this as is, and subsequently merge a new PR to add this new urls to the regex. 🙏 Thanks again for your help here! |
Change reference from icons to new font folder
WHY
BEFORE - What was wrong? What was happening before this PR?
Wasn't caching the correct folder path due to updates in the elFinder Material Theme
AFTER - What is happening after this PR?
It will cache the correct files in the correct folders
Closes #43
HOW
How did you achieve that, in technical terms?
Updated the folder Path
Is it a breaking change or non-breaking change?
Not a breaking change.
How can we test the before & after?
If you run it as is, you will get 404 error and no icons when opening the file manager. If you run this pull request the icons will display and no 404 errors
Additional Notes
Ideally you would lock the version via jsdelivr but I wasn't going to propose that without a conversation.
i.e. https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme@3.0.0/Material/css/theme.min.css