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

[4.0] Unused minicolors override #30120

Closed
SharkyKZ opened this issue Jul 17, 2020 · 2 comments
Closed

[4.0] Unused minicolors override #30120

SharkyKZ opened this issue Jul 17, 2020 · 2 comments

Comments

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Jul 17, 2020

There is a currently unused override of minicolors (administrator/templates/atum/scss/vendor/minicolors/minicolors.scss). The file should be called jquery.minicolors, not minicolors. But if we make it usable, the styling breaks because of missing image file used by imported vendor's CSS file.

The build script would need to either copy the images to override directory or update the path.

Or we manually make CSS rule overrides but that's not a good long term solution, considering other vendor files could have the same issue.

@brianteeman
Copy link
Contributor

To make it work is much easier than that

  1. remove the import from the override
    @import "../../../../../../node_modules/@claviska/jquery-minicolors/jquery.minicolors";

  2. add the override to the template.scss
    @import "vendor/minicolors/minicolors";

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Jul 17, 2020

remove the import from the override

This is wrong, it destroys any caching advantages, eg the code will be always invalidated per template scss updates even if it didn't change. That's not how assets should be treated.

The build script would need to either copy the images to override directory or update the path.

There are 2 options:

  • add a resolver for the path in the scss (you can copy it from Bootstrap, etc)
  • Inline as base64 the image, which is better for perf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants