Add util method remToPx.

mzbeetnoff commented 2 years ago

What Add util method remToPx to convert a whole or decimal rem value to a pixel value. The motivation being that the global font-size for the Avail app is controlled by the theme in this repository.

https://moveinc.atlassian.net/browse/AT-5900

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

pkrawc commented 2 years ago

Sorry this sat stale. @mzbeetnoff What's the ultimate goal here?

pkrawc commented 2 years ago

I would potentially pull this away from assuming the theme has final say. You can get the computed style for the document font size and use that to calc the correct value.

function remToPx(rem) {    
    return rem * parseFloat(getComputedStyle(document.documentElement).fontSize);
}

mzbeetnoff commented 2 years ago

Sorry this sat stale. @mzbeetnoff What's the ultimate goal here?

Hey @pkrawc no worries at all, sorry for not providing enough background here. We noticed in a few places in Avail we are converting rem values to px either by multiplying by the constant 12 or using the result from getComputedStyle(document.documentElement).fontSize and we wanted a single source for this incase the default rem size changes in the future, and also to keep things DRY.

I would potentially pull this away from assuming the theme has final say. You can get the computed style for the document font size and use that to calc the correct value.

The method you suggest we actually tried initially:

function remToPx(rem) {    
    return rem * parseFloat(getComputedStyle(document.documentElement).fontSize);
}

But we noticed it wasn't reliable as the value returned from getComputedStyle(document.documentElement).fontSize is sometimes 16 (the default for Chrome) depending on whether or not the styled component in the theme had been loaded yet.

pkrawc commented 2 years ago

Cool, that all makes sense. If we're wanting to use the theme directly, we should be pulling it in through context and this should likely be a hook in that case. There's no guarantee that the theme object is what makes it's way to the provider eventually since the consumer can override it.

mzbeetnoff commented 2 years ago

Really really good point and thanks for flushing that out. I made the suggested change and check the value set in the context 👍

rentalutions / elements

Add util method remToPx. #203