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

Issue with Minify CSS Files option and CSS Custom Properties #371

Closed
stoyan0v opened this issue Feb 1, 2021 · 7 comments
Closed

Issue with Minify CSS Files option and CSS Custom Properties #371

stoyan0v opened this issue Feb 1, 2021 · 7 comments

Comments

@stoyan0v
Copy link

stoyan0v commented Feb 1, 2021

Hey @tdewolff

In some cases where we make calculations with calc() – this breaks everything because the calc function doesn’t know what units to take into consideration

It should be like this margin: var(--margin, 0px) but the minify makes it like this margin: var(--margin, 0) – without unit.

Is there a flag or something we can use to leave the units for calc values only?

Regards,
Stanimir

@tdewolff
Copy link
Owner

tdewolff commented Feb 5, 2021

Thanks for the report. I will look into fixing this!

@tdewolff
Copy link
Owner

tdewolff commented Feb 7, 2021

Are you using the latest version? When I minify

span {
   margin:var(--a,5px)
}

I get span{margin:var(--a,5px)}, so the px stays. In what way does calc come into play here? Can you give an example that is being minified wrong?

@stoyan0v
Copy link
Author

stoyan0v commented Mar 2, 2021

Hey @tdewolff

If the value is not 0, everything works as expected, but if you have the following property:
margin: var(--margin, 0px) the minify removes the px from the code, which causes issues.

Regards,
Stanimir

@tdewolff
Copy link
Owner

tdewolff commented Mar 2, 2021

Ok, pushed out a fix. It's a quick-fix, functionality around functions should definitively be improved.

@stoyan0v
Copy link
Author

Hey @tdewolff

Do you have any plans to release a new CLI version with this patch included?

Regards,
Stanimir

@tdewolff
Copy link
Owner

Yes! See 2.9.14

@stoyan0v
Copy link
Author

Wow, that was fast. Thanks a lot :)

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