NativeScript / theme

@nativescript/theme
https://v7.docs.nativescript.org/ui/theme
Apache License 2.0
128 stars 45 forks source link

Fix rem-to-dip #278

Closed jamescodesthings closed 3 years ago

jamescodesthings commented 4 years ago

Started trying to use this mixin then noticed a logic error.

The unit of $size will always be (!em || !rem) because if it's one it's not the other. Changed to and because I think that's the desired functionality? It looks like the fallback case is to strip the units off of the input value, seems a little strange, might need further discussion about what's actually going on here?

PR Checklist

What is the current behavior?

rem-to-dip always returns the input size without units, for example:

// $font-size: 12;
// Functon call
font-size: rem-to-dip(1.7rem);
// Desired output
font-size: 20;
// Actual Output
font-size: 1.7;

What is the new behavior?

rem-to-dip will return $size * $font-size for em or rem units, and a fraction for other units. Not certain em should be included because the result is not accurate. Not fixed: rem/em is not stripped from rem/em units. so the actual output is font-size: 20rem; which is interpreted as DIPs.

BREAKING CHANGES: N/A

Migration steps: N/A

NathanaelA commented 3 years ago

Thank you @jamescodesthings - I wasn't watching this repo; so I missed a lot of what was going on. I've merged your PR's. We will get a new version out that fixes all the issues.