GoogleChrome / samples

A repo containing samples tied to new functionality in each release of Google Chrome.
https://www.chromestatus.com/samples
Apache License 2.0
5.79k stars 2.38k forks source link

Is calc w/multiple css custom properties broken? #705

Closed roblevintennis closed 4 years ago

roblevintennis commented 4 years ago

Hello my Google friends :)

Noticed this in both Brave / Chrome -- version info:

Brave is up to date Version 1.14.84 Chromium: 85.0.4183.121 (Official Build) (64-bit)

Google Chrome is up to date Version 85.0.4183.121 (Official Build) (64-bit)


So, I noticed that in a framework I'm building, that if I used calc with two custom properties, the rule essentially got ignored/dumped.

I'm using CSS Modules but I noticed these weren't working:

.src-stories-Input__input-large--2M6ZX {
    font-size: calc(var(--agnosticui-input-font-size) + var(--Space-4));
    line-height: calc(var(--agnosticui-input-line-height) + var(--Space-4));
}

So to do a sanity check I did:

element.style {
    height: calc(var(--Space-10) + var(--Space-80));
}

as I "knew" my spacing variables were definitely set, and sure enough I did NOT get my 90px height. In computed: image

So then I googled around and found Google's own CSS Custom Properties page at: https://googlechrome.github.io/samples/css-custom-properties/

And noticed that it's not working there either! Looking specifically at the .grid class you'll see:

padding: calc(var(--grid-margin) - var(--cell-margin));

but looking at the computed properties there's no padding taking affect: image

I also looked at .cell which I'm not as certain about but, it's originally defined with: calc(100% / var(--grid-columns) - var(--grid-gutter)) (I suspect it's using the 100% and throwing out one of the custom properties but that's just a theory).

If I manually create a style: width: calc(var(--grid-columns) - var(--grid-gutter));:

image

and look at the computed it remains with the 100% part: image

so it seems again the simple addition of two custom properties within calc is getting thrown out.

This seems like a pretty bad bug and it completely breaks the ui component framework I'm writing fwiw. Also, it's easy to miss it because of course the fallback is fault tolerant and falls back to another evaluated value. This feels like it must have worked in the passed and this is a regression ¯_( ͡❛ ͜ʖ ͡❛)_/¯ -- meantime I have this in my code:

.input-small {
  /* TODO -- use + var(--Space-4) if they fix the bug I logged: https://github.com/GoogleChrome/samples/issues/705 */
  font-size: calc(var(--agnosticui-input-font-size) - 4px);
  line-height: calc(var(--agnosticui-input-line-height) - 4px);
}

UPDATE: I tried to look at this same .grid element in Firefox 80.0.1 (64-bit) (and 81.0 (64-bit) after an update), and the exact same thing is happening where the padding rule is not taking affect:

image

roblevintennis commented 4 years ago

User error!

Ok, I had a bug and so I had line-height as a ratio not px and that must confuse calc if you ultimately are trying to do 1.5 + 4px (understandably so). When I switched to px it in fact works fine.

Fwiw, I do find it a bit confusing for the .grid thing to not be working in the example page I mentioned previously so it might be worth examining that example page. But, no bug I think. Closing.

argyleink commented 4 years ago

Mixing units in calc() does have some tricky edges! Glad you resolved it 🙂

Here's some interesting use cases on css-tricks that can help illuminate the edges

roblevintennis commented 4 years ago

Thanks for the resource @argyleink 🙏