Laravel-Backpack / FileManager

Admin interface for files & folders, using elFinder.
Other
90 stars 21 forks source link

Update basset paths #44

Closed arb362 closed 8 months ago

arb362 commented 8 months ago

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

karandatwani92 commented 8 months ago

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. https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme/Material/icons/material.eot https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme/Material/icons/material.svg https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme/Material/icons/material.woff https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme/Material/icons/material.ttf https://cdn.jsdelivr.net/gh/RobiNN1/elFinder-Material-Theme/Material/icons/material.woff2

Let me know if I missing something.

pxpm commented 8 months ago

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 v2.x where the icons are inside the icons folder. Other people are getting the v3, which has the icons inside the font folder.

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.svg

If you don't have time/desire to do it, I will do the changes myself and merge it.

Thanks again 🙏

arb362 commented 8 months ago

Sure thing @pxpm! Thanks for checking into it. I have added the version to the URLs.

pxpm commented 8 months ago

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!