Skip to content
This repository has been archived by the owner on Mar 27, 2023. It is now read-only.

Change root font-size from 24px to 125% #1721

Closed
georgiee opened this issue Nov 27, 2017 · 4 comments
Closed

Change root font-size from 24px to 125% #1721

georgiee opened this issue Nov 27, 2017 · 4 comments

Comments

@georgiee
Copy link

georgiee commented Nov 27, 2017

[ ] bug
[ ] feature request
[x] enhancement

Expected behavior

REM values should be easier to calculate.
If I look into the nav link in the sidenav for example, I see the following values:

.sidenav .nav-group .nav-list .nav-link {
    line-height: .666667rem;
    padding: .16667rem 0 .16667rem .5rem;
}

With your base font-size of 24px you end up with those calculated values:

.sidenav .nav-group .nav-list .nav-link {
    line-height: 16px;
    padding: 4px 0 4px 12px;
}

This wasn't obvious from reading the rem values. This makes it hard for you as the maintainer of this project to calculate rem values, you always end up with floating values and anybody wanting to extend your css ends up in the same conversion chaos.

Actual behavior

By changing the base font-size to 62.5% which calculates to 10px in all browsers, as the defacto default base size in all browsers is 16px (found in the settings).

You should not set this value to a fixed 10px as this would have an impact on the accessibility as the font-size could not be changed in the browser settings anymore.

So by setting this value to a calculated value of 10px, you can use rem nearly intuitively, as rem values are now back in the base 10 system only shifted by one digit

REM = PX / 10

where your current formula is:

REM = PX / 24

The earlier mentioned example would look like this with the new base.

.sidenav .nav-group .nav-list .nav-link {
    line-height: 1.6rem;
    padding: 0.4rem 0 0.4rem 1.2rem;
}

I came up with this problem as I'm using your amazing library to build the chrome for my own component library. I don't have sidebars, headers, boxes for sourcode in this library so it is amazing that I can just grab your framework to display my work.
This works because I use css prefixes through my whole project so I don't have any collissions. But this rem thing is not fixable without changing my or your rem system.

If you think it's worth to give it a try I would love to help you with a PR.
Or maybe you discussed this rem thing already and came to a different conclusion yielding to the current implementation?

Reproduction of behavior

n/a

Environment details

Applies to all environments and the master version of clarity

@mathisscott
Copy link
Contributor

@georgiee
Thanks for the suggestion. It's a great idea. Our only concern is that products that need a 20px baseline would struggle because math is apparently hard.

We think a SASS function to which people could pass the baseline they want (20, 24, 18, 28) would be useful.

I don't know whether it would be better to set the font-size to 10px or go forward with 62.5% as you suggest. So I'd like to hear the pros/cons on that.

Beyond that, we are all onboard with this change.

It will be a breaking change for us, unfortunately. So we would need it to go into 0.12. If you want to take it on, that would be great. If not, I will take it and try to address this after 0.11 enters alpha.

@georgiee
Copy link
Author

Hello @mathisscott,
thanks for your response and I'm glad to hear that you consider this a useful change.

Regarding the root font-size
You can read that issue on the bootstrap repository for clarification why it's always better to have a relative font-size on the html element. Putting hard pixels will eliminate user preferences and will harm accessibility.

You can also find a nice article on the topic linking to https://nicolas-hoizey.com/2016/03/people-don-t-change-the-default-16px-font-size-in-their-browser.html.

It's simple as this: There is no risk and only advantages. I also haven't seen any problems with this in the past in my own projects.

Baseline Math
I must admit I didn't work very much with vertical rhythm and I don't see yet where this will surface to the user. if there is a mixin to change the baseline it should be almost invisible to the user when we change the underlying rem base.

Changing itself should be easy. Set the new base font-size and transform every rem value by factor 2.4. I see a good opportunity to script this as it is almost search and replace. As this must be done any way, would it make sense to do it first and then figure out how to handle the baseline?

@mathisscott mathisscott modified the milestones: Post 2.0, 3.0 Aug 6, 2019
@mathisscott mathisscott self-assigned this Aug 6, 2019
@mathisscott
Copy link
Contributor

Per discussion, we won't be changing the default font size to 10px (62.5%) because our investigations into broader effects this has vs. edge cases were not good.

We will change to a base size of 20px instead and try using the percentage of (125%). This will mitigate edge cases in products where they are using fixed font sizes and not rems or where they are using custom rems that will wind up slightly smaller based on our change in base font size.

@mathisscott mathisscott changed the title Change root font-size from 24px to 62.5% Change root font-size from 24px to 125% Aug 7, 2019
@github-actions
Copy link

github-actions bot commented Sep 3, 2020

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants