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

Better header line height #418

Merged
merged 2 commits into from
Jan 9, 2019
Merged

Conversation

dfeyer
Copy link
Contributor

@dfeyer dfeyer commented Jan 8, 2019

This change also remote all units for line-height to have more consistent styles and better cascading support. This solve the too small line height for multi line header in article and also add styling for h1 to h6 (previously h3 to h6 was not styled at all)

dfeyer added 2 commits January 8, 2019 23:58
Thie change also remote all units for line-height to have more consistent
styles and better cascading support
Copy link
Contributor

@igalic igalic left a comment

Choose a reason for hiding this comment

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

@@ -16,7 +16,7 @@ main article {
margin: 2.5em auto;
font-family: $lora;
font-size: 1.2em;
line-height: 1.7em;
line-height: 1.7;
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the measure without a postfix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

line-height without postfix is based on the current font size (so a bit like em), but with the em postfix, cascading is based on the current font size, so the h1-6 get a line height of 1.7em based on the font size of the main articleselector, so way too small. Without the postfix, element inside the article will have a correct line height, based on their own font size.

Try to insert a really long headline in an article (to have a multi line rendering), with and without this PR to see that the headline are not readable.

See for more details about the unitless value for line-height: https://css-tricks.com/almanac/properties/l/line-height/#article-header-id-0

@elegaanz elegaanz self-requested a review January 9, 2019 15:55
Copy link
Member

@elegaanz elegaanz left a comment

Choose a reason for hiding this comment

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

Thanks!

@elegaanz elegaanz merged commit 671c340 into Plume-org:master Jan 9, 2019
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.

3 participants