Issue with baseFontSize using units and pxTo

[mrmartineau]

Issue with baseFontSize using units and pxTo

[mrmartineau]

Hi @oyeanuj, the readme is actually a mistake, it should be unitless.

Option 1, while seemingly cleaner, would not be my preferred option because I would have to add a special condition just for this key (type.baseFontSize). My preferred option would be to just get the value like this: font-size: ${ds.get('type.baseFontSize')}px; (used with emotion/styled-components).

Option 2: this sounds good, but if the user specifies an em value of 1.2 for example, it then becomes hard to determine what they really want. I could implement a variation of this option: baseFontSize is always a px value ('20px') and the unit is stripped when using the fontSize function. This one actually seems like it might be better and more clear to users. What do you think?

[mrmartineau]

@oyeanuj As an interim fix, I have updated the docs so that they are correct and I should have the improved version of this soon.

[oyeanuj]

@mrmartineau I see your points, and think your solution sounds good.

On a sidenote, FWIW, is that when using it like font-size: ${ds.get('type.baseFontSize')}px;, I noticed myself forgetting the unit at the end everytime. While that is my fault, I think in general, all the font size functions should return the values in the same way (either always with units, or never with units, or ideally, user could specify how they want it?).

Thinking more about that in conjunction with #4, I think we could have a standard API of fontSize (and maybe even get(type.baseFontSize)) which always returns just the numeric value (using the default unit, but also takes in a parameter to get it in a particular unit?

[mrmartineau]

Both v0.5.0 & v0.6.0 address this issue