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

Use local vendor directory as consistent source for elFinder assets #40

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jnoordsij
Copy link
Contributor

WHY

BEFORE - What was wrong? What was happening before this PR?

ElFinder assets were collected from multiple places, i.e. the vendor directory, a GitHub archive and a CDN.

This should be no issue, but could cause inconsitencies as versions are not necessarily matching.

AFTER - What is happening after this PR?

All ElFinder assets are retrieved from the vendor directory. This ensures consistency, removes external dependencies and allows updating/pinning the version by the user.

HOW

How did you achieve that, in technical terms?

Add a @bassetDirectory(base_path('vendor/studio-42/elfinder/'), 'elfinder-vendor') directive to have a "single source of truth" for elFinder assets. Note that the entire directory has to be copied, as some part of the elfinder script uses relative urls to fetch assets, which causes issues if only individual bassets are used.

Is it a breaking change or non-breaking change?

No.

How can we test the before & after?

Check the various implemented views to assert assets are loaded and correct.

@karandatwani92
Copy link

Hey @jnoordsij Thanks for the PR.

@pxpm could you please review this? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

4 participants