h5bp / html5-boilerplate

A professional front-end template for building fast, robust, and adaptable web apps or sites.
https://html5boilerplate.com/
MIT License
56.27k stars 12.2k forks source link

line-height issue #656

Closed fmal closed 12 years ago

fmal commented 12 years ago

I've run into an issue where some inline elements are breaking baseline grid. <code> / <small> inherits line-height from body, but the baseline grid gets out of sync. The fix is to set line-height to '1' on phrase elements like kbd,dfn,code and small. They still align well and the problem goes away. I've made a demo to show the difference. Just scroll down to <pre> block and click on ‘change <code> line-height to 1’ http://nmstudio.pl/pliki/demo/lineheight/ - you can see how text jumps and the <pre> block aligns to the grid.

line-height:normal doesn't work when you put <code> inside <h1>, so line-height:1 is the best solution

My recommended changes to h5bp are:

pre, code, kbd, samp {
  font-family: monospace, monospace; 
  _font-family: 'courier new', monospace;
  font-size: 1em; 
}
small {font-size: 85%; }
code,kbd,samp,small{  line-height:1; }
necolas commented 12 years ago

Not seeing what the issue is. Please can you provide a reduced test case that uses the relevant parts of the current H5BP CSS. Also the browser and OS environments in which the alleged issue occurs. Thanks

fmal commented 12 years ago

ok - i've updated demo: http://nmstudio.pl/pliki/demo/lineheight/ - just click on "change <code> line-height to 1" and see what happens - text aligns to the grid. When you use <small> or <code> and want to maintain consistent line-height, then line-height:1 is necessary on those elements.

the issue seems to appear in ff 5, in Opera and Chrome it's fine. Funny - when i turn off hardware acceleration in Firefox the issue disappears.

screenshot: http://dl.dropbox.com/u/20952020/sc/test.png

necolas commented 12 years ago

OK I see it in Firefox. Also in IE8/9 but none of the values give you the consistent vertical rhythm. Also IE7 has a different problem; doesn't seem to have any effect on the inline code but the line-height:1 completely changes the line-height of code when inside pre.

Would you be able to improve the test case, using a proper repeating bg image to show the baseline, so that it shows up in old IE and I can do a few more tests.

Thanks

fmal commented 12 years ago

test case updated. It's using bg image now. It's fine in IE9 as long as you use pixels (i used em values which have been computed differently in IE and Firefox) - changed margins etc to pixels and the result is same in ie9/ff5. line-height:1 for code in pre shouldn't matter because pre inherits proper line-height. I don't have a possibility to test it in IE7 right now, but i suspect that defining line-height on pre might work.. added pre{line-height:21px;} in my demo

necolas commented 12 years ago

Using code {line-height:1} effects the line height of code in pre in IE6/7, irrespective of what line height has been applied to pre.

This also seems to be effected by the primary font that is used, and happens for any inline element (e.g. span) that has a smaller font-size than the default.

So there isn't any quick fix that I can see, that will work in all situations without causing problems in other browsers or introducing unexpected line heights at times when elements are set to display:block.

fmal commented 12 years ago

setting line-height:0 or line-height:1 on span,code,kbd,samp and then reintroducing line-height: 1.231; (body line-height) on code in pre fixes the issue across all browsers.


body{
  margin:0;
  font-size:13px;
  line-height: 1.231;
}
small {font-size: 85%;}
pre, code, kbd, samp {
  font-family: monospace, monospace; 
  _font-family: 'courier new', monospace;
  font-size: 1em;
 }
code,kbd,samp,small{line-height:0;}
pre code{line-height:1.231;} /* <body> line-height (ie 6,7 fix) */

unexpected line-heights when elements are set to display:block may be a problem, but this fix applies to elements which are used mostly inline.. i think that normalized line-height is a nice benefit.

You may check out this css for example: http://natbat.net/static/css/main.css - both big and small have line-height:0 to avoid inconsistency

also found this in spec:

When an element contains text that is rendered in more than one font, user agents may determine the 'normal' 'line-height' value according to the largest font size.

When there is only one value of 'line-height' for all inline boxes in a block container box and they are all in the same font (and there are no replaced elements, inline-block elements, etc.), the above will ensure that baselines of successive lines are exactly 'line-height' apart.

necolas commented 12 years ago

Yeah I thought about including pre code in a shared rule for setting line-height:

body { margin: 0; font-size: 13px; }
body, pre code { line-height: 1.231; }

...

code, kbd, samp, small { line-height: 1; }

But it's all getting a bit hacky, still doesn't fix the problem when there are other inline elements with a different font-family or font-size, and don't think it is uncommon for small to be set to display: block.

For the moment, I'm on the side of leaving browsers to do their own thing where the only drawback is not-perfect vertical rhythm in some browsers...which is notoriously difficult to achieve anyway.

fmal commented 12 years ago

Ok, I understand your point. Maybe leaving browsers to do their own thing is a better compromise here. Thanks for your insight on this issue anyway.

necolas commented 12 years ago

Thanks for looking into all this. Although these fixes might not be ideal for inclusion in the default boilerplate, I think your fixes would be fine for individual developers who know the specifics of a project.