claroline / Claroline

Learning management system
https://www.claroline.com
Other
315 stars 188 forks source link

[Themes] Building a theme does not copy the custom fonts folder #1954

Closed maieutiquer closed 2 years ago

maieutiquer commented 2 years ago
Q A
Bug report? no
Version? 13.0,13.1

Our theme has a /fonts folder, just like an /images folder: https://github.com/konsept-ch/cep-theme

We're building the theme and the images get copied, but not the fonts

image

which causes our fonts to not load on the UI:

image

Ideally, building a theme should copy over the /fonts folder, if it exists, just like the /images folder

maieutiquer commented 2 years ago

This seems to be caused by the copyStatic() method deleting the destination every time in:

https://github.com/claroline/Claroline/blob/3224ce3f7420029713f6c05e1c2c118b0d40ed67/src/main/core/Resources/server/themes/build.js#L148

It actually deletes the theme for every static folder that it finds, so it first copies the /fonts, but then deletes the entire custom theme folder, and then copies the /images

I tried removing that line and then /fonts is copied correctly as well as /images

Not sure what the correct solution would be though, I guess we should delete not the entire theme folder, but only the target subfolder, maybe something like:

function copyStatic(src, destination, assetDir) {
  console.log(src)
  console.log(destination)
  console.log(path.join(destination, assetDir))
  shell.rm('-rf', path.join(destination, assetDir))
  shell.cp('-R', src, destination)
}

and we call it like:

      theme.getStaticAssets().map(assetDir =>
        copyStatic(
          // src
          path.resolve(theme.location, theme.name, assetDir),
          // destination
          path.resolve(themeDir),
          // asset dir
          assetDir)
      )

(this seems to work correctly as per my tests)