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

Remove header fixed font size in overview boxes #109

Merged
merged 9 commits into from
Nov 30, 2023
Merged

Conversation

froggleston
Copy link
Contributor

@froggleston froggleston commented Nov 30, 2023

This change should alleviate some of the font sizing and underlining issues seen in carpentries/workbench#57 and carpentries/workbench#64, and remedied somewhat by @zkamvar in #55.

By changing the font-size property in the overview.scss stylesheet from a fixed px value to a relative calculation, the font size of the overview header text and the two internal headers for Questions and Objectives will scale with the viewport width and not result in nasty underlining shenanigans.

Note: this will fix #83

Copy link
Contributor

@zkamvar zkamvar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good! I'm curious about how the implementation works :)

@@ -6,7 +6,7 @@
border: none;
height: 53px;
padding-top: 7px;
font-size: 18px;
font-size: calc(22px + 0.3vw);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what's going on here?

Copy link
Contributor Author

@froggleston froggleston Nov 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can! The font-size is being calculated (calc()) rather than a fixed single value. There are lots of other examples of this kind of calculation in other varnish stylesheets so I'm not sure why this wasn't done here. In any case, the calc function uses a pixel-based value and a viewport-based ratio to set the font size. The viewport width should be relative to the immediate parent container's width IIRC.

@froggleston
Copy link
Contributor Author

It looks good! I'm curious about how the implementation works :)

I'm glad you asked! 😄

We needed to change the initial font value pre-viewport width modifier to be fixed so that with an shrinking window size, the font doesn't become so small it's unreadable. The smaller viewport modifier values should ensure that the text doesn't get scaled too much from its initial 22px value. It seems to be good on a 1920x1080 monitor full screen and down to 300px wide for mobile view.

@zkamvar
Copy link
Contributor

zkamvar commented Nov 30, 2023

I've gone ahead and fixed a couple of things to keep in line with the original wireframe. One thing I found is that the original design had a font weight of 600 for the card titles.

For reference, here is the old version:

overview card with spindly questions and objectives

The new version looks like this and responds much better:

screenshot of overview card where questions and objectives are bolder

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

Successfully merging this pull request may close these issues.

Heading word wrap inconsistency
3 participants