Problem/Motivation

Heading font-sizes are still using values inherited from Seven which doesn't match values in described in the design system.

Proposed resolution

Update heading font-sizes and line-heights to match with the design system.

Remaining tasks

  1. Document heading line-heights in the design system
  2. Implement font-size and line-height here
CommentFileSizeAuthor
#14 Screen Shot 2019-09-25 at 16.32.03.png192.7 KBlauriii
#12 Screen Shot 2019-09-25 at 15.32.44.png221.19 KBlauriii
#11 Claro-headings-with-text.png784.65 KBhuzooka
#10 interdiff.3083231.6-10.txt591 bytesfhaeberle
#10 3083231-10.claro_.Update-heading-fontsizes-and-lineheights.patch1.88 KBfhaeberle
#2 headings.png139.63 KBckrina
#4 claro-heading-sizes-3083231-4.patch2.29 KBant1
#6 interdiff_4-6.txt422 bytesant1
#6 claro-heading-sizes-3083231-6.patch1.88 KBant1

Comments

lauriii created an issue. See original summary.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
StatusFileSize
new headings.png139.63 KB

Here are the full specs on Figma: https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Drupal-Design-system...

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Issue tags: +Novice
ant1’s picture

ant1
Primary language Dutch
commented
Status: Active » Needs review
StatusFileSize
new claro-heading-sizes-3083231-4.patch2.29 KB

Patch provided with updates to the heading's line-heights & font-sizes.

Remark:
Changing the line-height value of .page-title changes the distance between the title and the breadcrumbs above it.
The Content Header specs haven't been updated with new line-height value, unless of course it is meant to remain the same (1.5).
In the patch I declared its line-height to be 1.15.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Needs work 
+++ b/css/src/components/page-title.css
@@ -9,7 +9,7 @@
-  line-height: var(--line-height);
+  line-height: var(--line-height-heading);

I think we should revert this change since the content header design hasn't been changed.

ant1’s picture

ant1
Primary language Dutch
commented
Status: Needs work » Needs review
StatusFileSize
new claro-heading-sizes-3083231-6.patch1.88 KB
new interdiff_4-6.txt422 bytes

.page-title's line-height has been reverted.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs review » Needs work
  1. Design is still unclear (at least for me). I cannot decide which one to prefer: 130% or 1.15? They aren't equal!
  2. I'd be happy if we would have a multi-line example as well, and I'm sure that it would be helpful for everyone not just me.

Note: the design (I mean the look) of the Heading1 tag suggests something else than its own documentation.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented 
+++ b/css/src/base/variables.css
@@ -43,6 +43,7 @@
+  --line-height-heading: 1.15;

It seems that the design system standardized on 1.3, so we should update this.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented

#7.1: Sorry, I copy/pasted too fast. It should be 130%/1.3. Specs updated on Figma.
#7.2: Also examples added in the specs.

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Status: Needs work » Needs review
StatusFileSize
new 3083231-10.claro_.Update-heading-fontsizes-and-lineheights.patch1.88 KB
new interdiff.3083231.6-10.txt591 bytes

Updated the --line-height-heading variable! :) Hopefully there's nothing left.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Claro-headings-with-text.png784.65 KB

This is RTBC.

Screenshot attached (markup provided by Style Guide.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue tags: +Needs followup
StatusFileSize
new Screen Shot 2019-09-25 at 15.32.44.png221.19 KB

This causes some regressions to the status report page. Let's open a follow-up to fix that.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

Re #12:
How does that page look without the patch?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue summary: View changes
StatusFileSize
new Screen Shot 2019-09-25 at 16.32.03.png192.7 KB

Here's how this looks like in alpha5:

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue tags: -Needs followup
Related issues: +#3084008: Fix regressions on status report page

Opened #3084008: Fix regressions on status report page.

  • lauriii committed 119b8c9 on 8.x-1.x 
    Issue #3083231 by AntoineH, fhaeberle, lauriii, huzooka, ckrina: Update...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Patch looks good and we have a follow-up! Thank you all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.