Vertical rhythm for headers

[karland]

Here is the second attempt of a PR to the vertical-ryhthm branch:

1. run the test with npm run test: visual and you will find a file vertical-rhythm/vertical-rhythm.htm
2. all line-heights will be relative now
3. the PR contains the docs. Read the docs first, to see what I have done.
4. $header-styles is replacing $header-sizes. If $header-styles is not present in the settings.scss it is automatically built from $header-sizes. This approch is downward compatible and avoids any boolean variables.
5. I have extensively tested this, but I would love to see this tested by somebody else.

[brettsmason]

@karland I'm away until tomorrow but from your description this sounds fantastic! I'll take a look properly tomorrow but thank you. A great first step into doing similar with everything hopefully.

[karland]

@brettsmason Any progress? Questions?

[brettsmason]

@karland sorry I haven't had as much time as I wanted but I will make sure I fully review it tomorrow. I did quickly pull it down to try yesterday and from my brief tests it seems to work great. Will get back to you properly tomorrow.

6.3 release has been delayed a week because of thanksgiving so we have plenty of time.

[brettsmason]

Hi @karland so had another look (will look more throughout the day).

A few things so far:

• Line height is declared within the h1, h2, h3 etc block first with line-height: $header-lineheight; - I think we can loose this
• Can you think of any way that if a user doesn't declare a value in a breakpoint above the small (default) that a new value isn't output, so it just inherits the small breakpoints values? I think this would be a nice enhancement and save on some code that isnt potentially needed

I love the map conversion you have done, really brilliant idea!

I'll continue testing, as well as reading through the docs with any other points I might find.

[karland]

@brettsmason

• Agree. I have just put $header-lineheight in, in order for the user to see what the default is without looking at the docs. From a coding point of view I don't like it at all.

• What you are suggesting is to simply change the default values for all sizes above 'small' to the values from 'small'. That should not be too difficult to code, but wold probably require a set of temporary variables. Logically, this would be much better than the current solution.

[brettsmason]

@karland sorry just to clear the points up:

• I meant this line - I think that can be removed.

• What I mean is say a user doesn't enter margins for their medium breakpoints - at the moment we are outputting the default values. Wouldn't it be better to not output any values at all on breakpoints above small? So its truly mobile first.

Does that make sense?

[karland]

Inheriting the default values is actually more tricky than I thought. I am working on something. I'll PR later.

[karland]

I think, this was not tricky at all.

[brettsmason]

@karland thanks for looking at this some more. However I think I might of misled you with what I meant by mobile first on this occasion - apologies.

I think the ideal output would be something like this (just an example):

h1,
h2,
h3,
h4,
h5,
h6 {
  font-family: "Helvetica Neue", Helvetica, Roboto, Arial, sans-serif;
  font-style: normal;
  font-weight: normal;
  color: inherit;
  text-rendering: optimizeLegibility; }
  h1 small,
  h2 small,
  h3 small,
  h4 small,
  h5 small,
  h6 small {
    line-height: 0;
    color: #cacaca; }

h1 {
  margin-top: 0;
  margin-bottom: 0.5rem;
  font-size: 1.5rem;
  line-height: 1.4; }

h2 {
  margin-top: 0;
  margin-bottom: 0.5rem;
  font-size: 1.25rem;
  line-height: 1.4; }

h3 {
  margin-top: 0;
  margin-bottom: 0.5rem;
  font-size: 1.1875rem;
  line-height: 1.4; }

h4 {
  margin-top: 0;
  margin-bottom: 0.5rem;
  font-size: 1.125rem;
  line-height: 1.4; }

h5 {
  margin-top: 0;
  margin-bottom: 0.5rem;
  font-size: 1.0625rem;
  line-height: 1.4; }

h6 {
  margin-top: 0;
  margin-bottom: 0.5rem;
  font-size: 1rem;
  line-height: 1.4; }

@media print, screen and (min-width: 40em) {
  h1 {
    margin-bottom: 1rem;
    font-size: 3rem; }
  h2 {
    font-size: 2.5rem; }
  h3 {
    font-size: 1.9375rem; }
  h4 {
    font-size: 1.5625rem; }
  h5 {
    font-size: 1.25rem; }
  h6 {
    line-height: 1.2;
    font-size: 1rem; } }

So what I mean is the fallback values are only used on the small breakpoint, and not output again if the user hasn't entered a value.

The above was generated by making a simple change to your original PR, so the final code would be:

  // Headings
  h1,
  h2,
  h3,
  h4,
  h5,
  h6 {
    font-family: $header-font-family;
    font-style: $header-font-style;
    font-weight: $header-font-weight;
    color: $header-color;
    text-rendering: $header-text-rendering;

    small {
      line-height: 0;
      color: $header-small-font-color;
    }
  }

  // Heading styles
   @each $size, $headers in $header-styles {
    @include breakpoint($size) {
      @each $header, $header-defs in $headers {
        $font-size-temp: 1rem;
        #{$header} {
          @if map-has-key($header-defs, margin-top) {
            margin-top: rem-calc(map-get($header-defs, margin-top));
          } @else if map-has-key($header-defs, mt) {
            margin-top: rem-calc(map-get($header-defs, mt));
          } @else if $size == $-zf-zero-breakpoint {
            margin-top: 0;
          }
          @if map-has-key($header-defs, margin-bottom) {
            margin-bottom: rem-calc(map-get($header-defs, margin-bottom));
          } @else if map-has-key($header-defs, mb) {
            margin-bottom: rem-calc(map-get($header-defs, mb));
          } @else if $size == $-zf-zero-breakpoint {
            margin-bottom: rem-calc($header-margin-bottom);
          }

          @if map-has-key($header-defs, font-size) {
            $font-size-temp: rem-calc(map-get($header-defs, font-size));
            font-size: $font-size-temp;
          } @else if map-has-key($header-defs, fs) {
            $font-size-temp: rem-calc(map-get($header-defs, fs));
            font-size: $font-size-temp;
          } @else if $size == $-zf-zero-breakpoint {
            font-size: $font-size-temp;
          }
          @if map-has-key($header-defs, line-height) {
            line-height: unitless-calc(map-get($header-defs, line-height), $font-size-temp);
          } @else if map-has-key($header-defs, lh) {
            line-height: unitless-calc(map-get($header-defs, lh), $font-size-temp);
          } @else if $size == $-zf-zero-breakpoint {
            line-height: unitless-calc($header-lineheight, $font-size-temp);
          }
        }
      }
    }
  }

The key point being the @else if $size == $-zf-zero-breakpoint so that the fallback is only added if its the small breakpoint. Also no sizes, line heights and margins are first block

  h1,
  h2,
  h3,
  h4,
  h5,
  h6

as they aren't needed. Does that make sense and look clearer/cleaner output? Really sorry if I caused any confusion! Also feel free to disagree with me...

[karland]

@brettsmason No worries. I think our solutions are very similar. Basically, we both do not make use of the @else branch any longer. However, you keep the @else branch to set the default values and I moved the default values back into the 'first block'. I have no problem / disagreement with your suggestion and I am happy to take it over.

[brettsmason]

@andycochran If you get a minute could you take a quick look at this? It now looks OK to me. I'll then get this branch together and sort out a PR into develop.

[DaSchTour]

Just a short question, as I can't find anything about it. This change will break existing old configuration? If that's the case I think the new configuration should have another name and there should be a mapping from old to new, to ensure backward compatibility for a certain time. As this change is an extension to the old functionality it shouldn't be that hard to create a mapping that works with old configuration.

[karland]

@DaSchTour This PR should not break your old settings as described in the accompanying docs.

"Warning. The $header-styles map has replaced $header-sizes map in version 6.3. $header-styles map is a more general map than $header-sizes. $header-sizes map is still working and is used to initialize the $header-styles map. In a future version $header-sizes is going to be depreciated."

So, the $header-sizes map is still working unless you are using the $header-styles map in your settings.

[DaSchTour]

Oh, okay. Somehow I haven't seen it at first sight. Thanks for clarification and great work. 👍

[brettsmason]

@karland going to merge this in so I can put together the main PR into develop. Thanks so much for your work on this, its excellent! 👍

[karland]

@brettsmason Thanks to you. It has been fun working with you. I think one of my next PRs will be to improve on other elements to get proper vertical rhythm.