Vertical rhythm in Foundation 6

[karland]

@gakimball @rafibomb A lot of people love vertical rhythms in their sites, me included. There are external tools to achieve this, but I think it would be nice if F6 would make it a little easier to actually make the appropriate settings in _settings.scss without external dependencies.

The basic idea for achieving vertical rhythm in F6 would be to have all all top/bottom margins and paddings and all line-heights to be a multiple of the $global-lineheight. This is almost already possible now, but here are some necessary changes:

• add $header-line-heights similar to $header-sizes. This allows to choose the line-height, when header-sizes change responsively
• form-elements are not behaving very vertical ryhthm like. They would require to be explicitey styled with $form-element-padding and $form-element-margin (the $form-spacing would then play a less important role)
• change docs

I am happy to make a PR, but before I would like to check with ZURB if this is actually desired.

The old default values for the current line-height-settings and margins or paddings would remain.

[erutan]

+1 for that.

I'd also suggest a consistent naming scheme for line-height variables $global-lineheight and $form-label-line-height bug me. :)

[phifa]

+1

[tomByrer]

I had to remove some of the vertical padding & line-heights in some of of my page sections. A vertical .collapse would be nice also.

[andycochran]

I like the idea of a $header-line-heights map. But forms and buttons are where nailing down baseline grid gets tricky. See #8507.

Here's a recent article that might help alleviate any concerns about pixel-perfect baseline grids:
http://zellwk.com/blog/web-typography-broken/

[abarnwell]

+1

[brettsmason]

I'd quite like to pick this up unless anyone else from the original discussion wants to submit a PR?

[abarnwell]

@brettsmason happy to help with this if you want to kick it off.

[karland]

Initially I was checking with ZURB if this is desired. I still think Vertical Rhythm would be a real nice and useful addition. I have not taken the time to work on a complete solution. At the moment I am just using a quick and dirty approach with header-line-heights and some settings.

@mixin myfoundation-vertical-rhythm {

  // Heading line-heights
  @each $size, $headerlhs in $header-lineheights {
    @include breakpoint($size) {
      @each $headerlh, $lheight in $headerlhs {
        #{$headerlh} {
          line-height: $lheight;
        }
      }
    }
  }

}

My settings.scss looks like this:

$header-lineheights: (
  small: (
    'h1': 1.5 * $base-line-height,
    'h2': 1.5 * $base-line-height,
    'h3': 1 * $base-line-height,
    'h4': 1 * $base-line-height,
    'h5': 1 * $base-line-height,
    'h6': 1 * $base-line-height,
  ),
  medium: (
    'h1': 3 * $base-line-height,
    'h2': 2 * $base-line-height,
    'h3': 1.5 * $base-line-height,
    'h4': 1 * $base-line-height,
    'h5': 1 * $base-line-height,
    'h6': 1 * $base-line-height,
  ),
);

// Vertical Rhythm
$global-lineheight: $base-line-height;
$header-lineheight: $base-line-height;
$header-margin-bottom: $base-line-height;
$lead-lineheight: $base-line-height;
$list-lineheight: $base-line-height;
$list-margin-bottom: $base-line-height;
$paragraph-lineheight: $base-line-height;
$paragraph-margin-bottom: $base-line-height;
$subheader-lineheight: $base-line-height;
$subheader-margin-bottom: $base-line-height;
$subheader-margin-top: 0;

Great, if you want to take this up. I am happy to add/support. I also looked at a couple of VR libraries for ideas out of which I particularly liked https://github.com/Pushplaybang/knife/.

[brettsmason]

@karland Great thanks for that and the link, very interesting. I'd love to work through this with you and get this implemented.

My initial thought was to have a separate map, exactly like you have done for line heights, along with maybe margins. Then have the option of assigning this map to the default $header-lineheight variable, as well as $header-margin-bottom. Then in the current code run a check to see if the variable is a map and then output the values in the code if it is, otherwise treat it as a static value as is currently works now.

What do you think? Do you think we also need to account for top margins or assume this is set for all headers globally?

[karland]

@brettsmason Great. At the moment is not a good time for me to work on this, so just push ahead. However, here are some comments:

1. The first thing is probably to add some test so that we know if it behaves like expected. The test case can later also be used in the kitchen sink.
2. I personally like Gridlover, so this could be the underlying concept. People who later put settings into F6 could then try these settings in there.
3. If Gridlover would be the underlying logic this would answer the questions whether to use header-margin-top. I think yes.
4. You are addressing a principal question with respect to checking for a map and I do not have an answer to this. My hunch would be to start with the settings.scss and introduce all necessary new parameters first, before actually starting with SASS-coding. With this it becomes clear how a solution would look like from the user point of view and if it is clear enough. In general I think the default settings.scss should not come with uncommented parameters.
5. I guess one question is if vertical rhythm is becoming a core/base feature and therefore the parameter map such as header-lineheights is moving under

//   4. Base Typography

or if all vertical ryhthm parameters go to

//  ??. Vertical Rhythm

In any case your solution with checking for the map sounds good to me in order to not introduce breaking changes.

Having said the before, also the form elements need some re-work, because they are particularly unvertical-rhythm-like. Especially the form-elements strongly rely on a parameter $form-spacing which is heavily used for vertical and(!) horizontal spacing. I have started to experiment with something like this but this is only food for thought and needs finalization first:

$form-label-line-height: $base-line-height;
$form-spacing: 0;
$form-element-height: $base-line-height;
$form-element-margin: 0 0 $base-line-height 0;
$form-element-padding: 0 0 0 $base-line-height/4;
$form-error-margin: -$base-line-height 0 $base-line-height 0;

What do you think?

[brettsmason]

@karland I'll reply in greater depth later (and thanks for a great reply!) but my first idea was something like this: http://codepen.io/brettsmason/pen/WGXKdE

Basically extend the $header-sizing map. Then some checks would need to check if each key has more than one value, if so then the user is using vertical rhythm. Alternatively we could introduce a new variable $vertical-rhythm which is a Boolean and run some stuff based off this.

I'm happy to do all the work but value your input on it to take it in the right direction! I'll have a chat with some of the Zurb guys and see what they think too.

Also I agree forms needs tackling too, but I'd like to concentrate on headings first and do forms after.

[brettsmason]

Just a quick update. I have this: http://codepen.io/brettsmason/pen/rrYkBo as an evolution of the above Codepen. See what you think of this idea. I feel like it would be quite flexible, as you can set a value to null to not include it in the output - eg to inherit the styling from a smaller breakpoint. However the first option is probably easier for the user I guess?

[karland]

@brettsmason Yeah, extending the $header-sizing-map is a much better approach compared to introducing a new $header-lineheights. I like both of your versions, but I would probably prefer your second (evolved) version. The Boolean variable $vertical-rhythm I do not like so much because it may contradict the settings in the $header-sizing-map, i.e. set to false but map suggests VR. Such an error may be difficult to spot and would introduce complexity in the code and documentation.

[andycochran]

The simplest change would be to simply add a responsive $header-line-height map. But it's still going to be complicated to calculate values by which to multiply the $global-lineheight for each header size so that they snap to a vertical grid. Especially since they're unitless.

Here's an idea… 

What about creating an optional line-height-rhythm function that returns a unitless value, based on the element's font-size in pixels, rounded to the line-height-rhythm value?

You could give elements whatever line-height you want, as you currently do. And with line-height-rhythm:false everything works as-is. But with line-height-rhythm:6, every element's line-height would round to the nearest 6px. Or, an element with font-size:1.5625rem (25px) could return values like 1.12 (28px), 1.28 (32px), etc… 

[andycochran]

Heh. Or even simpler, the $header-line-height map could be set in px instead of unitless (as is the $header-sizes map). Easily calculate 'em yourself.

[brettsmason]

@andycochran that's an interesting idea. I guess we need to establish exactly how to approach this.
Should we just allow users the freedom to change the line height/margin values or provide a way for them to output values that work with a particular rhythm (a bit like modular scale I guess)?

I don't mind either approach, but it would need to work with other items too (eg forms that @karland mentioned) if we expand what this is applicable to.

[andycochran]

We shouldn't enforce vertical rhythm (or over-engineer it). But we should make it easier to implement.

For headers, it'd be enough to provide a $header-line-height map (with values in px that convert to unitless) and let folks set the line heights to whatever they like, snapping to a rhythm or not. Do the calculations yourself. The defaults could (should?) follow a rhythm.

We should also do this for other elements—e.g. allow the line height of $paragraph-lineheight to be set in px and converted to unitless with the same function. Note: paragraphs are currently on a subpixel line height (25.6px).

[brettsmason]

I think we need more than just line heights, at least for typography. We also need control over top/bottom margins if we are going to do it right.

I still think something in this direction might be better: http://codepen.io/brettsmason/pen/WGXKdE
That or expanding the $header-lineheight and $header-margin variables to work as per the $header-sizes map.

The biggest problem I see at the moment is we cant control margins/line heights individually - dont we just need a way to access and change these if required?

[brettsmason]

Having said that I still like your idea of a function and would love to explore how that could work if you have time to discuss further?

[andycochran]

Agreed, we'll need more than line heights. I like the idea of expanding $header-lineheight and $header-margin more than expanding $header-sizes, as we'll be dealing with more than sizes (semantics).

Might it be easier to create a new $header-styles variable map and deprecate the old maps/vars in a future version?

As far as organizing the variables, what you suggest in your codepen is nice and compact. But it might be confusing to have to reference the order of the properties. I'm not sure if either of the following suggestions are better; I just wanted to throw 'em out there to spark discussion.

We could group by styles instead of by element:

$header-styles: (
  small: (
    'font-sizes': (24, 20, 19, 18, 17, 16),
    'line-heights': (28, 24, 20, 20, 20, 16),
    'top-margins': (0, 0, 0, 0, 0, 0),
    'bottom-margins': (24, 20, 20, 16, 16, 16),
  ),
  medium: ( …

^ But that requires you know that the six values are for h1-h6. (We could handle being provided fewer than six values — e.g. h6 is the same size as h5 if you only provide five sizes.)

Or we could be more explicit:

$header-styles: (
  small: (
    'h1': (
      'size': 24,
      'line-height': 28,
      'top-margin': 12,
      'bottom-margin': 24,
    ),
    'h2': (
      'size': 20,
      'line-height': 24,
      'top-margin': 8,
      'bottom-margin': 20,
    ),
    'h3': ( …

^ This would be very long, but it's very clear.

[brettsmason]

@andycochran I agree about $header-sizes not being the ideal variable to use - I just thought it might be easier, but I agree a new map would probably be better and deprecate the other variables (and remove in 6.4 for example). One map would also be easier to output the styles from than multiple.

I'm trying to also think ahead of how we would treat other components with similar functionality once headers are done - do you think ${component}-styles is right, or do you think we should be mentioning rhythm? I guess I'm not sure if everyone will understand rhythm, so styles is probably right.

Out of your examples, I like the simplicity of number 1 (similar to my first example). However number 2 (same as mine here: http://codepen.io/brettsmason/pen/rrYkBo) is a lot more readable. However I think its going to end up being far too long going forward, but happy to discuss that further.

On the one hand your first example would work well if we were to expand to forms in terms of the labels being clear, but I dont know how you would associate the values with anything in that scenario, so we end up with the same issue as my first example.

With comments and good documentation do you think we can adapt one of first examples to work? Like you also mentioned, inheritance is taken care of by not entering a value.

I'm wondering if a vertical rythm function would also be useful. So instead of having to look at modularscale.com etc, we can work it out for them (not sure how needed this is or not).

What about a $vertical-rhythm map, with breakpoint sizes in and a scale in each size? Not sure how exactly this could work yet though.

[phifa]

So sth. like this

$header-styles: (
  small: (
    'h1': (
      'size': 24,
      'line-height': 28,
      'top-margin': 12,
      'bottom-margin': 24,
    ),
    'h2': (
      'size': 20,
      'line-height': 24,
      'top-margin': 8,
      'bottom-margin': 20,
    ),
    'h3': ( …

is basically just plain variables, right? Don't get me wrong, I am just trying to understand, how this is helping with the rythm issue? Also since we are having px/rem and not em's, how does it support the idea of easy and scaled changes in font-size for example? Say for example I have to change h1 from 50 to 70px. Now I also have to change the other values, since they are fix and don't scale proportionaly (as em's do).

Maybe I just don't get vertical rythms and all that, then just ignore my comment :) I am just trying to follow along. Thanks!

[brettsmason]

@phifa Thanks for chipping in, the more the merrier 😄

So the first problem is the user doesn't currently have access to alter the line heights and margins of header elements, hence the need for a new map. That's the underlying issue in my opinion.

I guess this is kind of a vertical rhythm/flexibility change really. Someone may just want to have access to those values, whilst some users may want to go fully into setting a vertical rhythm.

Some users may want to input their own values from http://www.gridlover.net/try or http://www.modularscale.com, but it would be a nice bonus if we could provide a function that could do the maths for them, they just enter a value in the scale, eg rhythm(1). This could be based off the global font size, plus a scale/ratio map.

[phifa]

Saw this today, codepen included: https://twitter.com/trentwalton/status/788810698008637440?refsrc=email&s=11

[andycochran]

@brettsmason Yeah, my first suggestion is definitely more compact. The second one is 38 lines of code for each breakpoint, and you might as well just write raw CSS (which it almost is).

I don't like the idea of a $vertical-rhythm map. We should give folks control over line-heights and margins, allow them to achieve vertical rhythm. But we shouldn't enforce a particular design method (I'm still not convinced pixel-perfect vertical rhythm is crucial).

It would be cool to be able to plug in e.g. rhythm(1) and get a particular scale. But that's not easy to do programmatically, as fonts have various cap heights (which determine where the type is vertically centered in its box). You get into the weeds real quick trying to make sure type's baseline sits exactly on the grid, having to calculate vertical offsets with negative margins… $offset: ($line-height - $cap-height * $font-size) / 2; …which are different for each typeface, not just each font size. And it's going to get even more complicated when we consider elements other than headers and paragraphs.

Maybe we could eventually get there (plugging in rhythms). But there are several things that need to happen first. I say we tackle it like this:

• add a $header-styles map with size, line-height, and margin settings (Is padding necessary?)
• @function unitless-line-height() calculates unitless line-height: px line-height / px font-size

@phifa, maybe the $header-styles map should accept either units or unitless and unitless-line-height() returns them as unitless. It could just pass through unitless values or calculate them if a unit is provided. This way, if your h1's line-height is 1.2, you could change h1 from 50px to 70px without adjusting the other h1 values.

With these two things in place, we're then set up to get more exact with vertical rhythm. Maybe we write a separate pluggable module for vertical rhythm. Something you could @include or not.

[brettsmason]

@karland @andycochran I had a spare hour this afternoon to start looking at this. I thought I'd create the unitless-line-height function first, seeing as @karland 's code above is going to be such a big part of.

So far I have this, what do you think?

/// Converts a pixel, percentage, rem or em value to a unitless value based on a given font size.
///
/// @param {Number} $value - Value to convert to a unitless line height
/// @param {Number} $base - The font size to use to work out the line height - defaults to $global-font-size
///
/// @return {Number} - Unitless number
@function unitless-line-height($value, $base: null) {

  // If no base is defined, defer to the global font size
  @if $base == null {
    $base: $global-font-size;
  }

  // First, lets convert our $base to pixels
  // If the base font size is a %, then multiply it by 16px
  @if unit($base) == '%' {
    $base: ($base / 100%) * 16px;
  }

  // Using rem as base allows correct scaling
  @if unit($base) == 'rem' {
    $base: strip-unit($base) * 16px;
  }

  @if unit($base) == 'em' {
    $base: strip-unit($base) * 16px;
  }

  // Now lets convert our value to pixels too
  @if unit($value) == '%' {
    $value: ($value / 100%) * 16px;
  }

  @if unit($value) == 'rem' {
    $value: strip-unit($value) * 16px;
  }

  @if unit($value) == 'em' {
    $value: strip-unit($value) * 16px;
  }

  @if type-of($value) == 'number' and not unitless($value) {
    @return strip-unit($value) / strip-unit($base);
  }

  @return $value;
}

It allows you to pass in a value to convert, as well as an optional font size to use. I also wonder if we could set the $global-font-size in pixels, and convert it to a percentage when its used for the html elements font size? This would avoid introducing a new variable.

[andycochran]

This is a great start, @brettsmason. Do you think it could be beneficial to abstract this function for purposes other than line height? Not sure what it might be used for. Might there might be a future case where we need to get a number based on a font-size? Maybe call it unitless-value instead of unitless-line-height. Maybe not; just a thought.

[brettsmason]

@andycochran i actually thought about calling it 'unitless-calc' to tie in with rem-calc. What do you think? If you reckon this is good to go I'll start putting everything together. What do you think about the base/global font size issue - introduce a new variable?

[andycochran]

👍 unitless-calc()

[andycochran]

Doesn't this assume that the default font size of every browser is 16px?

[brettsmason]

@andycochran So most of that code was taken from the rem-calc function which does the same. Seeing as thats how its already handled it I assumed this would be OK too. Also wondered if some of the checks should be extracted into their own smaller functions, but its probably OK as is.

[karland]

@brettsmason @andycochran Great stuff.

To test the end result, I have started to think about how to produce a visual test for vertical rhythm to see if all F6 elements eventually align to a vertical rhythm. However, I am not really sure how to deal with this in the current test environment. Currently none of the foundation visual test cases is generated on the fly. So, for the moment I have just come up with a little jsfiddle. Any ideas?

[brettsmason]

@karland Sorry for the delay!

So firstly I have finally started work on this: https://github.com/brettsmason/foundation-sites/tree/vertical-rhythm

So far I have added the new map, and decided to add it in a format that matches the current setup. I also kept pretty much all your code as it was spot on - I just wrapped the values in rem-calc / unitless-calc where appropriate.

What I'm not sure of is how we depreciate the old $header-sizes map, as its no longer needed.

I'd appreciate if you could take a look at the work so far and see what you think 😄

As far as visual tests go, what do you mean by generated dynamically?
That fiddle looks like a great starting point though.

[karland]

@brettsmason I have been working on the code, but cannot PR to your brettsmason/foundation-sites/vertical-rhythm-branch as this conflicts with my own fork karland/foundation-sites. @brettsmason @andycochran is it possible to make this a zurb/foundation-sites/vertical-rhythm branch?

[brettsmason]

@karland yes of course give me an hour to get home and I'll do that!

[brettsmason]

@karland I've added the branch and pushed the changes I've made so far: https://github.com/zurb/foundation-sites/tree/vertical-rhythm

[karland]

@brettsmason Excellent. Thank you. I have submitted a PR #9386.

Now for the unitless-calc(), I am not sure, why your unitless-calc() would calculate unitless line-heights. This would mean that the line-height is relative to the font-size. I'd rather suggest to make line-heights in rems. Or am I on the wrong track?

[brettsmason]

@karland Great I'll take a look later today at your PR.

The unitless-calc function was designed to return a unitless line height, as I believe this is the recommended way of giving a line height.

The function takes a base value (eg $base-font-size) plus a value you give it, and it returns the correct line height value based off those values. At the moment its set to $global-font-size as I didnt know if we were going to introduce another variable or not.

Is that not what you were thinking?

[karland]

@brettsmason Thanks. I understood the logic of the unitless-calc(), however, I struggled with the concept of how to apply it. This is why I believe we need the common test environment. Please let me know, if you get it to run.

My understanding of vertical-rhythm is that line-heights are always an integer multiple of a base-line-height across all elements. So an elements line-height does not depend on the elements font-size. As a result line-heights usually get an absolute number (with 'rem's or 'px's). At least, this is also how gridlover and others (e.g. knife) are implementing it.

I changed your unitless-calc() accordingly, however, now the name does not fit any longer, since it returns a 'rem'-value. Have a look at my PR and tell me what you think. If you want we can move the more code oriented discussion into the thread of my PR.

[brettsmason]

@karland Sorry I'm struggling for time as I'm going away for 3 days after today. However I'd still love to get this moving along and included in 6.3, though the deadline is next Thursday so we are pushing it a bit!
Do you have the available time to spend on it to get it into a good place? I can merge your PR into the vertical-rhythm branch if that will help the process.

I'm not sold on having line heights with values yet, as everything I'm used to and have read suggests unitless is the best way to go. I probably need some more info on why we can't use unitless I guess...

Maybe @andycochran you could chime in with your opinion too?

[andycochran]

I'd stick with unitless line heights because of unforeseen inheritance issues. For now, we're just talking about adding line-height and margin to certain elements, we're not setting pixel-perfect vertical rhythm on every element. Right? I don't want to set the precedent of units on line heights. Child elements should compute their line heights based on their font size, as inherited values from parents are more likely to need overriding. If we introduce units to line heights, we'll have to set line height on every single element (yuck).

[karland]

@brettsmason I am not sure about unitless-lineheights in the context of vertical-ryhthms. Maybe it just has not yet crossed my path. I have to think this over and play around with it.

In any case, I think we have made some good progress moving from $header-sizes to $header-styles and keeping the line-heights relative for the moment is good enough for me. In this case you can forget about my PR on your unitless-calc() function.

I can see the point of @andycochran. In any way, I am not suggesting to introduce fixed line-heights to every element. On the other hand, the settings.scss should eventually and in the future allow the users to style all elements such that they adhere to a pixel-perfect vertical ryhthm.

I am happy to work on this a bit, but I cannot promise to have a working solution by Thursday. One open question is still how to depreciate $header-sizes. I have thought about different solutions, but in the end I believe a boolean variable is unavoidable. In this case you need to give some guidance on a variable name. Or maybe you have an idea for an alternative solution. Cheers.

[karland]

@brettmason @andycochran I have played around with unitless lineheights and I am happy with this approach. I closed my previous PR and worked out an according PR #9397.

While I was working on this PR #9397, I discovered some other issues that I have also put in separate PRs: PR #9396 and PR #9395

[stereobooster]

There is related work on typographic baseline https://github.com/michaeltaranto/basekick, maybe it will be helpful

[brettsmason]

This is something I would personally very much like to include in V7 but we aren't adding any new features for V6 going forward.
@karland Thanks again for your input on this - I'm hoping we can do a lot more with V7 on this 👍

[Anke]

I haven't been satisfied with foundations's vertical rhythm either, and I've implemented this example of Mike's: https://codepen.io/MadeByMike/pen/bEEGvv?editors%3D0100. See his matching article here: https://www.smashingmagazine.com/2016/05/fluid-typography/. Ideally you could mix his appriach of fluid typography and vertical rhythm, however this approch seems to take up a lot of calculating power.