Laravel-Backpack / theme-coreuiv4

UI for Backpack v6 that uses CoreUI v4 and Bootstrap v5.
MIT License
7 stars 4 forks source link

[Bug] Coreuiv4 and v2 are not usable without zip extension #41

Closed pxpm closed 1 year ago

pxpm commented 1 year ago

Recently we had a bug report regarding the usage of .zip files in our core code, that would make php zip extension a project dependency. https://github.com/Laravel-Backpack/FileManager/pull/31

Fortunately for us that package we wanted to include had .tar.gz distribution files that we could use, so we avoided having php zip as a project dependency by including the tar.gz distributables.

CoreUIv2 and CoreUIv4 use @bassetArchive('https://github.com/adobe-fonts/source-sans/releases/download/3.046R/WOFF2-source-sans-3.046R.zip', 'source-sans-pro')

This forces the php zip extension to be a theme dependency since this font don't provide the tar.gz distributables.

We have three ways out of this:

What do you think @tabacitu ?

tabacitu commented 1 year ago

add php zip as a theme dependency

Ugh… as a last resort. I guess it wouldn’t be the worst thing in the world.

find a font that provides .tar.gz files

We shouldn’t replace the font itself. The theme comes with a font that looks good with it. On this case it’s Source Sans Pro, so we should use source sans pro. But we can maybe find a different source for this font… right?

convert the zips to .tar.gz and serve them

No no. No!

promatik commented 1 year ago

Hey guys, I just find out Core UI v4 doesn't use Source Sans 🙃

PR to remove it here; https://github.com/Laravel-Backpack/theme-coreuiv4/pull/42

So, by saying this, since only Core UI v2 (deprecated theme) relies on a zip file, and Tabler and Core UI v4 do not, I think we can close this one 👌

Let me know if I'm wrong.

pxpm commented 1 year ago

There is the option to download the source file:

image

But it contains all font variants and it's 14MB instead of 3MB.

I tried to search for some cdn or something providing .tar.gz files but wasn't able to find one.

I think there is only two reasonable solutions, one more reasonable than the other I think:

Another thing I like in removing this zip is that the overall file size will be smaller, and I can clear my caches in development and don't have to wait for a zip to always download on the next page request 🤷

Let me know what you guys think @tabacitu @promatik

Cheers

tabacitu commented 1 year ago

Ok then. Let's bundle the font with the theme 👍

That way we don't have to add ZIP-EXT as a dependency.

promatik commented 1 year ago

Done here https://github.com/Laravel-Backpack/theme-coreuiv2/pull/28 👌