Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use rem units for font-size, normalise font sizes between headers and body text #630

Open
bencomp opened this issue Nov 6, 2021 · 3 comments

Comments

@bencomp
Copy link

bencomp commented Nov 6, 2021

I found that when I teach a lesson online and have the lesson materials on a second laptop or tablet, I need to zoom in a lot to be able to read the body text from a slightly bigger distance. Zooming in works, but then the headers are huge. Looking at Firefox's inspector for the demo lesson, I notice that the font-size for body text is less than half the pixels (14px) of <h2> header font size (30px). Could we rethink that ratio? It may not help that the headers are bold while the body text is not.

My understanding of CSS good practice is that font sizes are better not specified in pixels (except for the root font size), but using rem ('relative to root em size'). I think this is because some people have their root font size set to a larger value, for example, to compensate for low vision.
https://github.com/carpentries/varnish/blob/main/inst/pkgdown/assets/css/lesson.scss still specifies most font sizes using pixels.

@zkamvar
Copy link
Contributor

zkamvar commented Nov 8, 2021

Thank you for bringing this up! I agree that these need to be adjusted.

Just a note: the {varnish} repository is still in a testing phase for the new infrastructure and is not used in any live lessons, so I'm going to transfer this issue to the styles repository, so that it can be addressed there.

That being said, we will be rolling out an update to the lesson infrastructure next year that will completely revamp the lesson styling that explicitly considers a11y.

@zkamvar zkamvar transferred this issue from carpentries/varnish Nov 8, 2021
@zkamvar
Copy link
Contributor

zkamvar commented Nov 8, 2021

For reference, here is an example of the heading elements being defined with px:

h2 {
padding-top: $codeblock-padding;
padding-bottom: $codeblock-padding;
font-size: 20px;
background: linear-gradient(to bottom, $gradientcolor1, $gradientcolor2);
border-color: $color;
margin-top: 0px;
margin-left: -$codeblock-padding; // to move back to the left margin of the enclosing blockquote

@zkamvar
Copy link
Contributor

zkamvar commented Nov 8, 2021

Additionally, W3schools has a px to em converter so that we can adjust this now:

https://www.w3schools.com/TAGS/ref_pxtoemconversion.asp

zkamvar added a commit to zkamvar/lesson-example that referenced this issue Nov 8, 2021
I used the conversion for 1px = 0.0625em based on a default 16pt font

This will address part of carpentries/styles#630
zkamvar added a commit to zkamvar/styles that referenced this issue Nov 8, 2021
I used the conversion for 1px = 0.0625em based on a default 16pt font

This will address part of carpentries#630
@zkamvar zkamvar mentioned this issue Nov 8, 2021
fmichonneau pushed a commit that referenced this issue Jun 30, 2022
I used the conversion for 1px = 0.0625em based on a default 16pt font

This will address part of #630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants